Change TokenQueue.consumeCssIdentifier() to support hex digit unescaping (#2297)

To meet the https://www.w3.org/TR/css-syntax-3/#consume-name spec.

---------

Co-authored-by: Jonathan Hedley <jonathan@hedley.net>

Author: cketti

src/main/java/org/jsoup/parser/TokenQueue.java

@@ -5,14 +5,15 @@
 
 /**
  * A character queue with parsing helpers.
- *
- * @author Jonathan Hedley
  */
 public class TokenQueue {
     private String queue;
     private int pos = 0;
 
-    private static final char ESC = '\\'; // escape char for chomp balanced.
+    private static final char Esc = '\\'; // escape char for chomp balanced.
+    private static final char Hyphen_Minus = '-';
+    private static final char Unicode_Null = '\u0000';
+    private static final char Replacement = '\uFFFD';
 
     /**
      Create a new TokenQueue.
@@ -116,6 +117,10 @@ public void advance() {
         if (!isEmpty()) pos++;
     }
 
+    private char current() {
+        return queue.charAt(pos);
+    }
+
     /**
      * Consume one character off queue.
      * @return first character on queue.
@@ -238,7 +243,7 @@ public String chompBalanced(char open, char close) {
         do {
             if (isEmpty()) break;
             char c = consume();
-            if (last != ESC) {
+            if (last != Esc) {
                 if (c == '\'' && c != open && !inDoubleQuote)
                     inSingleQuote = !inSingleQuote;
                 else if (c == '"' && c != open && !inSingleQuote)
@@ -281,8 +286,8 @@ public static String unescape(String in) {
         StringBuilder out = StringUtil.borrowBuilder();
         char last = 0;
         for (char c : in.toCharArray()) {
-            if (c == ESC) {
-                if (last == ESC) {
+            if (c == Esc) {
+                if (last == Esc) {
                     out.append(c);
                     c = 0;
                 }
@@ -305,7 +310,7 @@ public static String escapeCssIdentifier(String in) {
             if (q.matchesCssIdentifier(CssIdentifierChars)) {
                 out.append(q.consume());
             } else {
-                out.append(ESC).append(q.consume());
+                out.append(Esc).append(q.consume());
             }
         }
         return StringUtil.releaseBuilder(out);
@@ -335,7 +340,6 @@ public String consumeWord() {
         return queue.substring(start, pos);
     }
 
-    
     /**
      * Consume a CSS element selector (tag name, but | instead of : for namespaces (or *| for wildcard namespace), to not conflict with :pseudo selects).
      * 
@@ -347,21 +351,157 @@ public String consumeElementSelector() {
     private static final String[] ElementSelectorChars = {"*", "|", "_", "-"};
 
     /**
-     Consume a CSS identifier (ID or class) off the queue (letter, digit, -, _)
-     http://www.w3.org/TR/CSS2/syndata.html#value-def-identifier
-     @return identifier
+     Consume a CSS identifier (ID or class) off the queue.
+     <p>Note: For backwards compatibility this method supports improperly formatted CSS identifiers, e.g. {@code 1} instead
+     of {@code \31}.</p>
+
+     @return The unescaped identifier.
+     @throws IllegalArgumentException if an invalid escape sequence was found. Afterward, the state of the TokenQueue
+     is undefined.
+     @see <a href="https://www.w3.org/TR/css-syntax-3/#consume-name">CSS Syntax Module Level 3, Consume an ident sequence</a>
+     @see <a href="https://www.w3.org/TR/css-syntax-3/#typedef-ident-token">CSS Syntax Module Level 3, ident-token</a>
      */
     public String consumeCssIdentifier() {
-        return consumeEscapedCssIdentifier(CssIdentifierChars);
+        if (isEmpty()) throw new IllegalArgumentException("CSS identifier expected, but end of input found");
+        int start = pos;
+
+        // Fast path for CSS identifiers that don't contain escape sequences.
+        while (!isEmpty()) {
+            char c = current();
+            if (isIdent(c)) {
+                advance();
+            } else if (c == Esc || c == Unicode_Null) {
+                // Exit fast path when an escape sequence or U+0000 is found.
+                break;
+            } else {
+                // End of identifier reached without encountering a sequence that requires special handling. The CSS
+                // identifier is a substring of the input.
+                return queue.substring(start, pos);
+            }
+        }
+
+        // An escape sequence was found. Use a StringBuilder to store the decoded CSS identifier.
+        StringBuilder out = StringUtil.borrowBuilder();
+
+        if (start < pos) {
+            // Copy the CSS identifier up to the first escape sequence.
+            out.append(queue, start, pos);
+        }
+
+        while (!isEmpty()) {
+            char c = current();
+            if (isIdent(c)) {
+                out.append(consume());
+            } else if (c == Unicode_Null) {
+                // https://www.w3.org/TR/css-syntax-3/#input-preprocessing
+                advance();
+                out.append(Replacement);
+            } else if (c == Esc) {
+                advance();
+                if (!isEmpty() && isNewline(current())) {
+                    // Not a valid escape sequence. This is treated as the end of the CSS identifier.
+                    pos--;
+                    break;
+                } else {
+                    consumeCssEscapeSequenceInto(out);
+                }
+            } else {
+                break;
+            }
+        }
+        return StringUtil.releaseBuilder(out);
     }
+
+    private void consumeCssEscapeSequenceInto(StringBuilder out) {
+        if (isEmpty()) {
+            out.append(Replacement);
+            return;
+        }
+        int start = pos;
+        char firstEscaped = consume();
+        if (!isHexDigit(firstEscaped)) {
+            out.append(firstEscaped);
+        } else {
+            for (int i = 0; i < 5 && !isEmpty(); i++) {
+                char escapedChar = current();
+                if (isHexDigit(escapedChar)) advance();
+                else break;
+            }
+            String hexString = queue.substring(start, pos);
+            int codePoint;
+            try {
+                codePoint = Integer.parseInt(hexString, 16);
+            } catch (NumberFormatException e) { // Won't happen as we confirmed hex above; just mollifying scanners
+                throw new IllegalArgumentException("Invalid escape sequence: " + hexString, e);
+            }
+            if (isValidCodePoint(codePoint)) {
+                out.appendCodePoint(codePoint);
+            } else {
+                out.append(Replacement);
+            }
+
+            if (!isEmpty()) {
+                char c = current();
+                if (c == '\r') {
+                    // Since there's currently no input preprocessing, check for CRLF here.
+                    // https://www.w3.org/TR/css-syntax-3/#input-preprocessing
+                    advance();
+                    if (!isEmpty() && current() == '\n') advance();
+                } else if (c == ' ' || c == '\t' || isNewline(c)) {
+                    advance();
+                }
+            }
+        }
+    }
+
+    // statics below specifically for CSS identifiers:
+
+    private static boolean isLetter(char c) {
+        return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z';
+    }
+
+    private static boolean isDigit(char c) {
+        return c >= '0' && c <= '9';
+    }
+
+    private static boolean isHexDigit(char c) {
+        return isDigit(c) || c >= 'a' && c <= 'f' || c >= 'A' && c <= 'F';
+    }
+
+    // https://www.w3.org/TR/css-syntax-3/#non-ascii-code-point
+    private static boolean isNonAscii(char c) {
+        return c >= '\u0080';
+    }
+
+    // https://www.w3.org/TR/css-syntax-3/#ident-start-code-point
+    private static boolean isIdentStart(char c) {
+        return c == '_' || isLetter(c) || isNonAscii(c);
+    }
+
+    // https://www.w3.org/TR/css-syntax-3/#ident-code-point
+    private static boolean isIdent(char c) {
+        return c == Hyphen_Minus || isDigit(c) || isIdentStart(c);
+    }
+
+    // https://www.w3.org/TR/css-syntax-3/#newline
+    // Note: currently there's no preprocessing happening.
+    private static boolean isNewline(char c) {
+        return c == '\n' || c == '\r' || c == '\f';
+    }
+
+    // https://www.w3.org/TR/css-syntax-3/#consume-an-escaped-code-point
+    private static boolean isValidCodePoint(int codePoint) {
+        return codePoint != 0 && Character.isValidCodePoint(codePoint) && !Character.isSurrogate((char) codePoint);
+    }
+
     private static final String[] CssIdentifierChars = {"-", "_"};
 
 
     private String consumeEscapedCssIdentifier(String... matches) {
         int start = pos;
         boolean escaped = false;
         while (!isEmpty()) {
-            if (queue.charAt(pos) == ESC && remainingLength() >1 ) {
+            if (queue.charAt(pos) == Esc && remainingLength() >1 ) {
                 escaped = true;
                 pos+=2; // skip the escape and the escaped
             } else if (matchesCssIdentifier(matches)) {

src/test/java/org/jsoup/parser/TokenQueueTest.java

@@ -3,8 +3,12 @@
 import org.jsoup.Jsoup;
 import org.jsoup.nodes.Document;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -139,4 +143,126 @@ public void testQuotedPattern() {
         assertEquals("i\\d", q.consumeCssIdentifier());
         assertTrue(q.isEmpty());
     }
+
+    @ParameterizedTest
+    @MethodSource("cssIdentifiers")
+    @MethodSource("cssAdditionalIdentifiers")
+    void consumeCssIdentifier_WebPlatformTests(String expected, String cssIdentifier) {
+        assertParsedCssIdentifierEquals(expected, cssIdentifier);
+    }
+
+    private static Stream<Arguments> cssIdentifiers() {
+        return Stream.of(
+            // https://github.com/web-platform-tests/wpt/blob/36036fb5212a3fc15fc5750cecb1923ba4071668/dom/nodes/ParentNode-querySelector-escapes.html
+            // - escape hex digit
+            Arguments.of("0nextIsWhiteSpace", "\\30 nextIsWhiteSpace"),
+            Arguments.of("0nextIsNotHexLetters", "\\30nextIsNotHexLetters"),
+            Arguments.of("0connectHexMoreThan6Hex", "\\000030connectHexMoreThan6Hex"),
+            Arguments.of("0spaceMoreThan6Hex", "\\000030 spaceMoreThan6Hex"),
+
+            // - hex digit special replacement
+            // 1. zero points
+            Arguments.of("zero\uFFFD", "zero\\0"),
+            Arguments.of("zero\uFFFD", "zero\\000000"),
+            // 2. surrogate points
+            Arguments.of("\uFFFDsurrogateFirst", "\\d83d surrogateFirst"),
+            Arguments.of("surrogateSecond\uFFFd", "surrogateSecond\\dd11"),
+            Arguments.of("surrogatePair\uFFFD\uFFFD", "surrogatePair\\d83d\\dd11"),
+            // 3. out of range points
+            Arguments.of("outOfRange\uFFFD", "outOfRange\\110000"),
+            Arguments.of("outOfRange\uFFFD", "outOfRange\\110030"),
+            Arguments.of("outOfRange\uFFFD", "outOfRange\\555555"),
+            Arguments.of("outOfRange\uFFFD", "outOfRange\\ffffff"),
+
+            // - escape anything else
+            Arguments.of(".comma", "\\.comma"),
+            Arguments.of("-minus", "\\-minus"),
+            Arguments.of("g", "\\g"),
+
+            // non edge cases
+            Arguments.of("aBMPRegular", "\\61 BMPRegular"),
+            Arguments.of("\uD83D\uDD11nonBMP", "\\1f511 nonBMP"),
+            Arguments.of("00continueEscapes", "\\30\\30 continueEscapes"),
+            Arguments.of("00continueEscapes", "\\30 \\30 continueEscapes"),
+            Arguments.of("continueEscapes00", "continueEscapes\\30 \\30 "),
+            Arguments.of("continueEscapes00", "continueEscapes\\30 \\30"),
+            Arguments.of("continueEscapes00", "continueEscapes\\30\\30 "),
+            Arguments.of("continueEscapes00", "continueEscapes\\30\\30"),
+
+            // ident tests case from CSS tests of chromium source: https://goo.gl/3Cxdov
+            Arguments.of("hello", "hel\\6Co"),
+            Arguments.of("&B", "\\26 B"),
+            Arguments.of("hello", "hel\\6C o"),
+            Arguments.of("spaces", "spac\\65\r\ns"),
+            Arguments.of("spaces", "sp\\61\tc\\65\fs"),
+            Arguments.of("test\uD799", "test\\D799"),
+            Arguments.of("\uE000", "\\E000"),
+            Arguments.of("test", "te\\s\\t"),
+            Arguments.of("spaces in\tident", "spaces\\ in\\\tident"),
+            Arguments.of(".,:!", "\\.\\,\\:\\!"),
+            Arguments.of("null\uFFFD", "null\\0"),
+            Arguments.of("null\uFFFD", "null\\0000"),
+            Arguments.of("large\uFFFD", "large\\110000"),
+            Arguments.of("large\uFFFD", "large\\23456a"),
+            Arguments.of("surrogate\uFFFD", "surrogate\\D800"),
+            Arguments.of("surrogate\uFFFD", "surrogate\\0DBAC"),
+            Arguments.of("\uFFFDsurrogate", "\\00DFFFsurrogate"),
+            Arguments.of("\uDBFF\uDFFF", "\\10fFfF"),
+            Arguments.of("\uDBFF\uDFFF0", "\\10fFfF0"),
+            Arguments.of("\uDBC0\uDC0000", "\\10000000"),
+            Arguments.of("eof\uFFFD", "eof\\"),
+
+            Arguments.of("simple-ident", "simple-ident"),
+            Arguments.of("testing123", "testing123"),
+            Arguments.of("_underscore", "_underscore"),
+            Arguments.of("-text", "-text"),
+            Arguments.of("-m", "-\\6d"),
+            Arguments.of("--abc", "--abc"),
+            Arguments.of("--", "--"),
+            Arguments.of("--11", "--11"),
+            Arguments.of("---", "---"),
+            Arguments.of("\u2003", "\u2003"),
+            Arguments.of("\u00A0", "\u00A0"),
+            Arguments.of("\u1234", "\u1234"),
+            Arguments.of("\uD808\uDF45", "\uD808\uDF45"),
+            Arguments.of("\uFFFD", "\u0000"),
+            Arguments.of("ab\uFFFDc", "ab\u0000c")
+        );
+    }
+
+    private static Stream<Arguments> cssAdditionalIdentifiers() {
+        return Stream.of(
+            Arguments.of("1st", "\\31\r\nst"),
+            Arguments.of("1", "\\31\r"),
+            Arguments.of("1a", "\\31\ra"),
+            Arguments.of("1", "\\031"),
+            Arguments.of("1", "\\0031"),
+            Arguments.of("1", "\\00031"),
+            Arguments.of("1", "\\000031"),
+            Arguments.of("1", "\\000031"),
+            Arguments.of("a", "a\\\nb")
+        );
+    }
+
+    @Test void consumeCssIdentifierWithEmptyInput() {
+        TokenQueue emptyQueue = new TokenQueue("");
+        Exception exception = assertThrows(IllegalArgumentException.class, emptyQueue::consumeCssIdentifier);
+        assertEquals("CSS identifier expected, but end of input found", exception.getMessage());
+    }
+
+    // Some of jsoup's tests depend on this behavior
+    @Test public void consumeCssIdentifier_invalidButSupportedForBackwardsCompatibility() {
+        assertParsedCssIdentifierEquals("1", "1");
+        assertParsedCssIdentifierEquals("-", "-");
+        assertParsedCssIdentifierEquals("-1", "-1");
+    }
+
+    private static String parseCssIdentifier(String text) {
+        TokenQueue q = new TokenQueue(text);
+        return q.consumeCssIdentifier();
+    }
+
+    private void assertParsedCssIdentifierEquals(String expected, String cssIdentifier) {
+        assertEquals(expected, parseCssIdentifier(cssIdentifier));
+    }
 }

src/test/java/org/jsoup/select/SelectorTest.java

@@ -1450,4 +1450,15 @@ public void testAncestorChain() {
         assertTrue(b_needs_a.threadMemo.get().isEmpty());
         assertTrue(c_needs_b.threadMemo.get().isEmpty());
     }
+
+    @Test void hexDigitUnescape() {
+        // tests the select component of https://github.com/jhy/jsoup/pull/2297, with per-spec escapes
+        // literal is: #\30 \%\ Platform\ Image
+        String html = "<img id='0% Platform Image'>";
+        String q = "#\\30 \\%\\ Platform\\ Image";
+
+        Document doc = Jsoup.parse(html);
+        Element img = doc.expectFirst(q);
+        assertEquals("img", img.tagName());
+    }
 }