Check attribute name is not empty after trimming before creating attribute

jhy · jhy · commit a623db776696 · 2017-01-05T15:07:29.000-08:00
Fixes jhy#793
diff --git a/CHANGES b/CHANGES
@@ -1,5 +1,10 @@
 jsoup changelog
 
+*** Release 1.10.3 [PENDING]
+  * Bugfix: if an attribute name started or ended with a control character, the parse would fail with a validation
+    exception.
+    <https://github.com/jhy/jsoup/issues/793>
+
 *** Release 1.10.2 [2017-Jan-02]
  * Improved startup time, particularly on Android, by reducing garbage generation and CPU execution time when loading
    the HTML entity files. About 1.72x faster in this area.
diff --git a/src/main/java/org/jsoup/nodes/Attribute.java b/src/main/java/org/jsoup/nodes/Attribute.java
@@ -29,9 +29,10 @@ public class Attribute implements Map.Entry<String, String>, Cloneable  {
      * @see #createFromEncoded
      */
     public Attribute(String key, String value) {
-        Validate.notEmpty(key);
+        Validate.notNull(key);
         Validate.notNull(value);
         this.key = key.trim();
+        Validate.notEmpty(key); // trimming could potentially make empty, so validate here
         this.value = value;
     }
 
diff --git a/src/main/java/org/jsoup/parser/Token.java b/src/main/java/org/jsoup/parser/Token.java
@@ -102,15 +102,19 @@ final void newAttribute() {
                 attributes = new Attributes();
 
             if (pendingAttributeName != null) {
-                Attribute attribute;
-                if (hasPendingAttributeValue)
-                    attribute = new Attribute(pendingAttributeName,
-                        pendingAttributeValue.length() > 0 ? pendingAttributeValue.toString() : pendingAttributeValueS);
-                else if (hasEmptyAttributeValue)
-                    attribute = new Attribute(pendingAttributeName, "");
-                else
-                    attribute = new BooleanAttribute(pendingAttributeName);
-                attributes.put(attribute);
+                // the tokeniser has skipped whitespace control chars, but trimming could collapse to empty for other control codes, so verify here
+                pendingAttributeName = pendingAttributeName.trim();
+                if (pendingAttributeName.length() > 0) {
+                    Attribute attribute;
+                    if (hasPendingAttributeValue)
+                        attribute = new Attribute(pendingAttributeName,
+                            pendingAttributeValue.length() > 0 ? pendingAttributeValue.toString() : pendingAttributeValueS);
+                    else if (hasEmptyAttributeValue)
+                        attribute = new Attribute(pendingAttributeName, "");
+                    else
+                        attribute = new BooleanAttribute(pendingAttributeName);
+                    attributes.put(attribute);
+                }
             }
             pendingAttributeName = null;
             hasEmptyAttributeValue = false;
diff --git a/src/test/java/org/jsoup/parser/HtmlParserTest.java b/src/test/java/org/jsoup/parser/HtmlParserTest.java
@@ -941,4 +941,9 @@ public void testInvalidTableContents() throws IOException {
         Document doc = parser.parseInput("<div id=1><SPAN ID=2>", "");
         assertEquals("<html> <head></head> <body> <div id=\"1\"> <SPAN ID=\"2\"></SPAN> </div> </body> </html>", StringUtil.normaliseWhitespace(doc.outerHtml()));
     }
+
+    @Test public void handlesControlCodeInAttributeName() {
+        Document doc = Jsoup.parse("<p><a \06=foo>One</a><a/\06=bar><a foo\06=bar>Two</a></p>");
+        assertEquals("<p><a>One</a><a></a><a foo=\"bar\">Two</a></p>", doc.body().html());
+    }
 }
diff --git a/src/test/java/org/jsoup/safety/CleanerTest.java b/src/test/java/org/jsoup/safety/CleanerTest.java
@@ -277,4 +277,10 @@ public void bailsIfRemovingProtocolThatsNotSet() {
         w.addAttributes("a", "href");
         w.removeProtocols("a", "href", "javascript"); // with no protocols enforced, this was a noop. Now validates.
     }
+
+    @Test public void handlesControlCharactersAfterTagName() {
+        String html = "<a/\06>";
+        String clean = Jsoup.clean(html, Whitelist.basic());
+        assertEquals("<a rel=\"nofollow\"></a>", clean);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -941,4 +941,9 @@ public void testInvalidTableContents() throws IOException {`
`941`	`941`	`Document doc = parser.parseInput("<div id=1><SPAN ID=2>", "");`
`942`	`942`	`assertEquals("<html> <head></head> <body> <div id=\"1\"> <SPAN ID=\"2\"></SPAN> </div> </body> </html>", StringUtil.normaliseWhitespace(doc.outerHtml()));`
`943`	`943`	`}`
	`944`	`+`
	`945`	`+ @Test public void handlesControlCodeInAttributeName() {`
	`946`	`+ Document doc = Jsoup.parse("<p><a \06=foo>One</a><a/\06=bar><a foo\06=bar>Two</a></p>");`
	`947`	`+ assertEquals("<p><a>One</a><a></a><a foo=\"bar\">Two</a></p>", doc.body().html());`
	`948`	`+ }`
`944`	`949`	`}`
Original file line number	Diff line number	Diff line change
`@@ -277,4 +277,10 @@ public void bailsIfRemovingProtocolThatsNotSet() {`
`277`	`277`	`w.addAttributes("a", "href");`
`278`	`278`	`w.removeProtocols("a", "href", "javascript"); // with no protocols enforced, this was a noop. Now validates.`
`279`	`279`	`}`
	`280`	`+`
	`281`	`+ @Test public void handlesControlCharactersAfterTagName() {`
	`282`	`+ String html = "<a/\06>";`
	`283`	`+ String clean = Jsoup.clean(html, Whitelist.basic());`
	`284`	`+ assertEquals("<a rel=\"nofollow\"></a>", clean);`
	`285`	`+ }`
`280`	`286`	`}`