You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2007/09/08 22:48:52 UTC

svn commit: r573908 - in /tapestry/tapestry5/trunk/tapestry-core/src: main/java/org/apache/tapestry/dom/ main/java/org/apache/tapestry/dom/util/ main/java/org/apache/tapestry/internal/util/ test/java/org/apache/tapestry/dom/

Author: hlship
Date: Sat Sep  8 13:48:51 2007
New Revision: 573908

URL: http://svn.apache.org/viewvc?rev=573908&view=rev
Log:
TAPESTRY-1604: Attributes of elements do not have entity values quoted (including the " character itself) resulting in invalid markup

Added:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/util/PrintOutCollector.java
      - copied, changed from r573901, tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/util/PrintOutCollector.java
Removed:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/util/PrintOutCollector.java
Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/DefaultMarkupModel.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Document.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Element.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/MarkupModel.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Node.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/dom/DOMTest.java

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/DefaultMarkupModel.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/DefaultMarkupModel.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/DefaultMarkupModel.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/DefaultMarkupModel.java Sat Sep  8 13:48:51 2007
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006, 2007 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,64 +12,81 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry.dom;
-
+package org.apache.tapestry.dom;
+
 import static org.apache.tapestry.ioc.internal.util.CollectionFactory.newSet;
-
-import java.util.Set;
-
-/**
- * Default implementation of {@link org.apache.tapestry.dom.MarkupModel} that is appropriate for
- * traditional HTML markup. This conforms to the SGML HTML definition, including some things that
- * are not well formed XML-style markup. Assumes that all tags are lowercase.
- */
-public class DefaultMarkupModel implements MarkupModel
-{
-    private final Set<String> EMPTY_ELEMENTS = newSet(
-            "base",
-            "br",
-            "col",
-            "frame",
-            "hr",
-            "img",
-            "input",
-            "link",
-            "meta",
-            "option",
-            "param");
-
-    /** Passes all characters but '&lt;', '&gt;' and '&amp;' through unchanged. */
-    public void encode(String content, StringBuilder buffer)
-    {
-        char[] array = content.toCharArray();
-
-        for (char ch : array)
-        {
-            switch (ch)
-            {
-                case '<':
-                    buffer.append("&lt;");
-                    continue;
-
-                case '>':
-                    buffer.append("&gt;");
-                    continue;
-
-                case '&':
-                    buffer.append("&amp;");
-                    continue;
-
-                default:
-                    buffer.append(ch);
-            }
-        }
-    }
-
-    public EndTagStyle getEndTagStyle(String element)
-    {
-        boolean isEmpty = EMPTY_ELEMENTS.contains(element);
-
-        return isEmpty ? EndTagStyle.OMIT : EndTagStyle.REQUIRE;
-    }
-
-}
+
+import java.util.Set;
+
+/**
+ * Default implementation of {@link org.apache.tapestry.dom.MarkupModel} that is appropriate for
+ * traditional HTML markup. This conforms to the SGML HTML definition, including some things that
+ * are not well formed XML-style markup. Assumes that all tags are lower-case.
+ */
+public class DefaultMarkupModel implements MarkupModel
+{
+    private final Set<String> EMPTY_ELEMENTS = newSet(
+            "base",
+            "br",
+            "col",
+            "frame",
+            "hr",
+            "img",
+            "input",
+            "link",
+            "meta",
+            "option",
+            "param");
+
+    /** Passes all characters but '&lt;', '&gt;' and '&amp;' through unchanged. */
+    public void encode(String content, StringBuilder buffer)
+    {
+        encode(content, false, buffer);
+    }
+
+    public void encodeQuoted(String content, StringBuilder buffer)
+    {
+        encode(content, true, buffer);
+    }
+
+    private void encode(String content, boolean encodeQuotes, StringBuilder buffer)
+    {
+        char[] array = content.toCharArray();
+
+        for (char ch : array)
+        {
+            switch (ch)
+            {
+                case '<':
+                    buffer.append("&lt;");
+                    continue;
+
+                case '>':
+                    buffer.append("&gt;");
+                    continue;
+
+                case '&':
+                    buffer.append("&amp;");
+                    continue;
+
+                case '"':
+                    if (encodeQuotes)
+                    {
+                        buffer.append("&quot;");
+                        continue;
+                    }
+
+                default:
+                    buffer.append(ch);
+            }
+        }
+    }
+
+    public EndTagStyle getEndTagStyle(String element)
+    {
+        boolean isEmpty = EMPTY_ELEMENTS.contains(element);
+
+        return isEmpty ? EndTagStyle.OMIT : EndTagStyle.REQUIRE;
+    }
+
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Document.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Document.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Document.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Document.java Sat Sep  8 13:48:51 2007
@@ -88,11 +88,12 @@
         if (_rootElement == null)
             throw new IllegalStateException("No root element has been defined.");
 
-        // TODO: XML declaration, plus lead-in comments, directives.
+        // TODO: lead-in comments, directives.
         if (_dtd != null)
         {
             _dtd.toMarkup(writer);
         }
+        
         _rootElement.toMarkup(writer);
     }
 

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Element.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Element.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Element.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Element.java Sat Sep  8 13:48:51 2007
@@ -21,6 +21,7 @@
 
 import java.io.PrintWriter;
 import java.util.Collections;
+import java.util.Formatter;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -30,8 +31,9 @@
 
 /**
  * An element that will render with a begin tag and attributes, a body, and an end tag. Also acts as
- * a factory for enclosed Element, Text and Comment nodes. TODO: Support for CDATA nodes. Do we need
- * Entity nodes?
+ * a factory for enclosed Element, Text and Comment nodes.
+ * <p>
+ * TODO: Support for CDATA nodes. Do we need Entity nodes?
  */
 public final class Element extends Node
 {
@@ -88,18 +90,15 @@
     {
         notBlank(name, "name");
 
-        if (value == null)
-            return;
+        if (value == null) return;
 
-        if (_attributes == null)
-            _attributes = newMap();
+        if (_attributes == null) _attributes = newMap();
 
-        if (!_attributes.containsKey(name))
-            _attributes.put(name, value);
+        if (!_attributes.containsKey(name)) _attributes.put(name, value);
     }
 
     /**
-     * Convienience for invoking {@link #attribute(String, String)} multiple times.
+     * Convenience for invoking {@link #attribute(String, String)} multiple times.
      * 
      * @param namesAndValues
      *            alternating attribute names and attribute values
@@ -122,8 +121,7 @@
      */
     public void forceAttributes(String... namesAndValues)
     {
-        if (_attributes == null)
-            _attributes = newMap();
+        if (_attributes == null) _attributes = newMap();
 
         int i = 0;
 
@@ -206,7 +204,13 @@
     @Override
     public void toMarkup(PrintWriter writer)
     {
-        writer.printf("<%s", _name);
+        StringBuilder buffer = new StringBuilder();
+
+        Formatter formatter = new Formatter(buffer);
+
+        formatter.format("<%s", _name);
+
+        MarkupModel markupModel = _document.getMarkupModel();
 
         if (_attributes != null)
         {
@@ -217,31 +221,32 @@
             {
                 String value = _attributes.get(key);
 
-                // TODO: URL encoding of attributes!
+                formatter.format(" %s=\"", key);
+
+                markupModel.encodeQuoted(value, buffer);
 
-                writer.printf(" %s=\"%s\"", key, value);
+                buffer.append('"');
             }
         }
 
-        EndTagStyle style = _document.getMarkupModel().getEndTagStyle(_name);
+        EndTagStyle style = markupModel.getEndTagStyle(_name);
 
         boolean hasChildren = hasChildren();
 
         String close = (!hasChildren && style == EndTagStyle.ABBREVIATE) ? "/>" : ">";
 
-        writer.print(close);
+        formatter.format(close);
+
+        writer.print(buffer.toString());
 
-        if (hasChildren)
-            writeChildMarkup(writer);
+        if (hasChildren) writeChildMarkup(writer);
 
         // Dangerous -- perhaps it should be an error for a tag of type OMIT to even have children!
         // We'll certainly be writing out unbalanced markup in that case.
 
-        if (style == EndTagStyle.OMIT)
-            return;
+        if (style == EndTagStyle.OMIT) return;
 
-        if (hasChildren || style == EndTagStyle.REQUIRE)
-            writer.printf("</%s>", _name);
+        if (hasChildren || style == EndTagStyle.REQUIRE) writer.printf("</%s>", _name);
     }
 
     /**
@@ -266,15 +271,13 @@
 
             String elementId = e.getAttribute("id");
 
-            if (id.equals(elementId))
-                return e;
+            if (id.equals(elementId)) return e;
 
             for (Node n : e.getChildren())
             {
                 Element child = n.asElement();
 
-                if (child != null)
-                    queue.addLast(child);
+                if (child != null) queue.addLast(child);
             }
         }
 
@@ -300,8 +303,7 @@
         {
             search = search.findChildWithElementName(name);
 
-            if (search == null)
-                break;
+            if (search == null) break;
         }
 
         return search;
@@ -313,8 +315,7 @@
         {
             Element child = node.asElement();
 
-            if (child != null && child.getName().equals(name))
-                return child;
+            if (child != null && child.getName().equals(name)) return child;
         }
 
         // Not found.

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/MarkupModel.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/MarkupModel.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/MarkupModel.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/MarkupModel.java Sat Sep  8 13:48:51 2007
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006, 2007 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,27 +12,39 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry.dom;
-
-/**
- * Used by a the DOM to determine how to produce markup. Delegates details about converted entities
- * and some formatting details.
- */
-public interface MarkupModel
-{
-    /**
-     * Encodes the characters into the buffer, converting control characters (such as '&lt;') into
-     * corresponding entities (such as &amp;lt;).
-     * 
-     * @param content
-     *            to be filtered
-     * @param buffer
-     *            to recieve the filtered content
-     */
-    void encode(String content, StringBuilder buffer);
-
-    /**
-     * For a given element, determines how the end tag for the element should be rendered.
-     */
-    EndTagStyle getEndTagStyle(String element);
-}
+package org.apache.tapestry.dom;
+
+/**
+ * Used by a the DOM to determine how to produce markup. Delegates details about converted entities
+ * and some formatting details.
+ */
+public interface MarkupModel
+{
+    /**
+     * Encodes the characters into the buffer, converting control characters (such as '&lt;') into
+     * corresponding entities (such as &amp;lt;).
+     * 
+     * @param content
+     *            to be filtered
+     * @param buffer
+     *            to receive the filtered content
+     */
+    void encode(String content, StringBuilder buffer);
+
+    /**
+     * Encodes the characters into the buffer for use in a quoted value (that is, an attribute
+     * value), converting control characters (such as '&lt;') into corresponding entities (such as
+     * &amp;lt;). In addition, double quotes must be quoted or otherwise escaped.
+     * 
+     * @param content
+     *            to be filtered
+     * @param buffer
+     *            to receive the filtered content
+     */
+    void encodeQuoted(String content, StringBuilder buffer);
+
+    /**
+     * For a given element, determines how the end tag for the element should be rendered.
+     */
+    EndTagStyle getEndTagStyle(String element);
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Node.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Node.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Node.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/Node.java Sat Sep  8 13:48:51 2007
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006, 2007 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -20,7 +20,7 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.apache.tapestry.dom.util.PrintOutCollector;
+import org.apache.tapestry.internal.util.PrintOutCollector;
 
 /**
  * A node within the DOM.

Copied: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/util/PrintOutCollector.java (from r573901, tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/util/PrintOutCollector.java)
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/util/PrintOutCollector.java?p2=tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/util/PrintOutCollector.java&p1=tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/util/PrintOutCollector.java&r1=573901&r2=573908&rev=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/dom/util/PrintOutCollector.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/util/PrintOutCollector.java Sat Sep  8 13:48:51 2007
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006, 2007 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,33 +12,39 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry.dom.util;
-
-import java.io.PrintWriter;
-import java.io.StringWriter;
-
-public class PrintOutCollector
-{
-    private StringWriter _stringWriter;
-
-    private PrintWriter _printWriter;
-
-    public PrintOutCollector()
-    {
-        _stringWriter = new StringWriter();
-        _printWriter = new PrintWriter(_stringWriter);
-    }
-
-    public PrintWriter getPrintWriter()
-    {
-        return _printWriter;
-    }
-
-    public String getPrintOut()
-    {
-        _printWriter.close();
-        return _stringWriter.toString();
-
-    }
-
-}
+package org.apache.tapestry.internal.util;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+/**
+ * Utility for collecting the output of a {@link PrintWriter}.
+ */
+public class PrintOutCollector
+{
+    private StringWriter _stringWriter;
+
+    private PrintWriter _printWriter;
+
+    public PrintOutCollector()
+    {
+        _stringWriter = new StringWriter();
+        _printWriter = new PrintWriter(_stringWriter);
+    }
+
+    public PrintWriter getPrintWriter()
+    {
+        return _printWriter;
+    }
+
+    /**
+     * Closes the {@link PrintWriter} and returns the accumulated text output.
+     */
+    public String getPrintOut()
+    {
+        _printWriter.close();
+        return _stringWriter.toString();
+
+    }
+
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/dom/DOMTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/dom/DOMTest.java?rev=573908&r1=573907&r2=573908&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/dom/DOMTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/dom/DOMTest.java Sat Sep  8 13:48:51 2007
@@ -14,7 +14,6 @@
 
 package org.apache.tapestry.dom;
 
-
 import org.apache.tapestry.internal.test.InternalBaseTestCase;
 import org.testng.annotations.Test;
 
@@ -326,39 +325,44 @@
 
         assertEquals(root.toString(), "<fred><em>&lt;&nbsp;&gt;</em></fred>");
     }
-    
+
     @Test
-    public void dtd_with_markup() 
+    public void dtd_with_markup()
     {
         Document d = new Document(new XMLMarkupModel());
         Element root = d.newRootElement("prime");
         root.element("slag");
         d.dtd("prime", "-//TF", "tf");
         String expected = "<!DOCTYPE prime PUBLIC \"-//TF\" \"tf\"><prime><slag/></prime>";
-        assertEquals(d.toString(),expected);
+        assertEquals(d.toString(), expected);
     }
-    
+
     @Test
-    public void dtd_with_nullids() {
+    public void dtd_with_nullids()
+    {
         Document d = new Document(new XMLMarkupModel());
         d.newRootElement("prime");
         d.dtd("prime", null, null);
-        assertEquals
-        (
-            d.toString(),
-            "<prime/>"
-        );
+        assertEquals(d.toString(), "<prime/>");
         d.dtd("prime", "-//TF", null);
-        assertEquals(
-            d.toString(),
-            "<!DOCTYPE prime PUBLIC \"-//TF\"><prime/>"
-        );
-        
+        assertEquals(d.toString(), "<!DOCTYPE prime PUBLIC \"-//TF\"><prime/>");
+
         d.dtd("prime", null, "tf");
-        assertEquals
-        (
-            d.toString(),
-            "<!DOCTYPE prime SYSTEM \"tf\"><prime/>"
-        );
+        assertEquals(d.toString(), "<!DOCTYPE prime SYSTEM \"tf\"><prime/>");
+    }
+
+    @Test
+    public void markup_characters_inside_attributes_are_escaped()
+    {
+        Document d = new Document(new XMLMarkupModel());
+
+        Element root = d.newRootElement("prime");
+
+        root.attribute("alpha-only", "abcdef");
+        root.attribute("entities", "\"<>&");
+
+        assertEquals(
+                root.toString(),
+                "<prime alpha-only=\"abcdef\" entities=\"&quot;&lt;&gt;&amp;\"/>");
     }
 }