⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 259773 in webkit


Ignore:
Timestamp:
Apr 8, 2020, 5:43:49 PM (6 years ago)
Author:
Chris Dumez
Message:

querySelector("#\u0000") should match an element with ID U+FFFD
​https://bugs.webkit.org/show_bug.cgi?id=210119

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Import test coverage from upstream WPT.

  • web-platform-tests/dom/nodes/ParentNode-querySelector-escapes-expected.txt: Added.
  • web-platform-tests/dom/nodes/ParentNode-querySelector-escapes.html: Added.

Source/WebCore:

As per the specification [1][2], we should preprocess the input string before performing
CSS tokenization. The preprocessing step replaces certain characters in the input string.

However, our code did not have this preprocessing step and instead was trying to deal
with those characters during tokenization. This is however not working as expected for
the '\0' character (which is supposed to be replaced with U+FFFD REPLACEMENT CHARACTER)
because our code deals with StringViews of the input String and just converts part of
the input stream to Strings / AtomStrings.

To address the issue, this patch adds a preprocessing step that replaces the '\0'
character with the U+FFFD REPLACEMENT CHARACTER). I opted not to replace '\r' or '\f'
characters since our tokenizer seems to be dealing fine with those.

[1] ​https://drafts.csswg.org/css-syntax/#input-preprocessing
[2] ​https://drafts.csswg.org/css-syntax/#parser-entry-points

Test: imported/w3c/web-platform-tests/dom/nodes/ParentNode-querySelector-escapes.html

  • css/parser/CSSTokenizer.cpp:

(WebCore::preprocessString):
(WebCore::CSSTokenizer::CSSTokenizer):
(WebCore::CSSTokenizer::lessThan):
(WebCore::CSSTokenizer::hyphenMinus):
(WebCore::CSSTokenizer::hash):
(WebCore::CSSTokenizer::reverseSolidus):
(WebCore::CSSTokenizer::letterU):
(WebCore::CSSTokenizer::consumeNumber):
(WebCore::CSSTokenizer::consumeIdentLikeToken):
(WebCore::CSSTokenizer::consumeStringTokenUntil):
(WebCore::CSSTokenizer::consumeUnicodeRange):
(WebCore::CSSTokenizer::consumeUrlToken):
(WebCore::CSSTokenizer::consumeBadUrlRemnants):
(WebCore::CSSTokenizer::consumeSingleWhitespaceIfNext):
(WebCore::CSSTokenizer::consumeIfNext):
(WebCore::CSSTokenizer::consumeName):
(WebCore::CSSTokenizer::consumeEscape):
(WebCore::CSSTokenizer::nextTwoCharsAreValidEscape):
(WebCore::CSSTokenizer::nextCharsAreNumber):
(WebCore::CSSTokenizer::nextCharsAreIdentifier):

  • css/parser/CSSTokenizer.h:
  • css/parser/CSSTokenizerInputStream.h:

(WebCore::CSSTokenizerInputStream::nextInputChar const):
(WebCore::CSSTokenizerInputStream::peek const):
(WebCore::CSSTokenizerInputStream::peekWithoutReplacement const): Deleted.

Source/WTF:

  • wtf/text/StringImpl.cpp:

(WTF::StringImpl::replace):
Slightly optimize the 16-bit code path of StringImpl::replace(). Since we know
there is no character match from indexes 0 to i, we can simply use memcpy for
this range.

Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r259725 r259773  
     12020-04-08  Chris Dumez  <cdumez@apple.com>
     2
     3        querySelector("#\u0000") should match an element with ID U+FFFD
     4        https://bugs.webkit.org/show_bug.cgi?id=210119
     5
     6        Reviewed by Darin Adler.
     7
     8        Import test coverage from upstream WPT.
     9
     10        * web-platform-tests/dom/nodes/ParentNode-querySelector-escapes-expected.txt: Added.
     11        * web-platform-tests/dom/nodes/ParentNode-querySelector-escapes.html: Added.
     12
    1132020-04-08  Rob Buis  <rbuis@igalia.com>
    214

  • trunk/Source/WTF/ChangeLog

    r259767 r259773  
     12020-04-08  Chris Dumez  <cdumez@apple.com>
     2
     3        querySelector("#\u0000") should match an element with ID U+FFFD
     4        https://bugs.webkit.org/show_bug.cgi?id=210119
     5
     6        Reviewed by Darin Adler.
     7
     8        * wtf/text/StringImpl.cpp:
     9        (WTF::StringImpl::replace):
     10        Slightly optimize the 16-bit code path of StringImpl::replace(). Since we know
     11        there is no character match from indexes 0 to i, we can simply use memcpy for
     12        this range.
     13
    1142020-04-08  Ross Kirsling  <ross.kirsling@sony.com>
    215

  • trunk/Source/WTF/wtf/text/StringImpl.cpp

    r258828 r259773  
    13031303    auto newImpl = createUninitializedInternalNonEmpty(m_length, data);
    13041304
    1305     for (i = 0; i != m_length; ++i) {
    1306         UChar character = m_data16[i];
     1305    memcpy(data, m_data16, i * sizeof(UChar));
     1306    for (unsigned j = i; j != m_length; ++j) {
     1307        UChar character = m_data16[j];
    13071308        if (character == target)
    13081309            character = replacement;
    1309         data[i] = character;
     1310        data[j] = character;
    13101311    }
    13111312    return newImpl;

  • trunk/Source/WebCore/ChangeLog

    r259772 r259773  
     12020-04-08  Chris Dumez  <cdumez@apple.com>
     2
     3        querySelector("#\u0000") should match an element with ID U+FFFD
     4        https://bugs.webkit.org/show_bug.cgi?id=210119
     5
     6        Reviewed by Darin Adler.
     7
     8        As per the specification [1][2], we should preprocess the input string before performing
     9        CSS tokenization. The preprocessing step replaces certain characters in the input string.
     10
     11        However, our code did not have this preprocessing step and instead was trying to deal
     12        with those characters during tokenization. This is however not working as expected for
     13        the '\0' character (which is supposed to be replaced with U+FFFD REPLACEMENT CHARACTER)
     14        because our code deals with StringViews of the input String and just converts part of
     15        the input stream to Strings / AtomStrings.
     16
     17        To address the issue, this patch adds a preprocessing step that replaces the '\0'
     18        character with the U+FFFD REPLACEMENT CHARACTER). I opted not to replace '\r' or '\f'
     19        characters since our tokenizer seems to be dealing fine with those.
     20
     21        [1] https://drafts.csswg.org/css-syntax/#input-preprocessing
     22        [2] https://drafts.csswg.org/css-syntax/#parser-entry-points
     23
     24        Test: imported/w3c/web-platform-tests/dom/nodes/ParentNode-querySelector-escapes.html
     25
     26        * css/parser/CSSTokenizer.cpp:
     27        (WebCore::preprocessString):
     28        (WebCore::CSSTokenizer::CSSTokenizer):
     29        (WebCore::CSSTokenizer::lessThan):
     30        (WebCore::CSSTokenizer::hyphenMinus):
     31        (WebCore::CSSTokenizer::hash):
     32        (WebCore::CSSTokenizer::reverseSolidus):
     33        (WebCore::CSSTokenizer::letterU):
     34        (WebCore::CSSTokenizer::consumeNumber):
     35        (WebCore::CSSTokenizer::consumeIdentLikeToken):
     36        (WebCore::CSSTokenizer::consumeStringTokenUntil):
     37        (WebCore::CSSTokenizer::consumeUnicodeRange):
     38        (WebCore::CSSTokenizer::consumeUrlToken):
     39        (WebCore::CSSTokenizer::consumeBadUrlRemnants):
     40        (WebCore::CSSTokenizer::consumeSingleWhitespaceIfNext):
     41        (WebCore::CSSTokenizer::consumeIfNext):
     42        (WebCore::CSSTokenizer::consumeName):
     43        (WebCore::CSSTokenizer::consumeEscape):
     44        (WebCore::CSSTokenizer::nextTwoCharsAreValidEscape):
     45        (WebCore::CSSTokenizer::nextCharsAreNumber):
     46        (WebCore::CSSTokenizer::nextCharsAreIdentifier):
     47        * css/parser/CSSTokenizer.h:
     48        * css/parser/CSSTokenizerInputStream.h:
     49        (WebCore::CSSTokenizerInputStream::nextInputChar const):
     50        (WebCore::CSSTokenizerInputStream::peek const):
     51        (WebCore::CSSTokenizerInputStream::peekWithoutReplacement const): Deleted.
     52
    1532020-04-08  Alex Christensen  <achristensen@webkit.org>
    254

  • trunk/Source/WebCore/css/parser/CSSTokenizer.cpp

    r251655 r259773  
    4141namespace WebCore {
    4242
     43// See: http://dev.w3.org/csswg/css-syntax/#input-preprocessing
     44static String preprocessString(String string)
     45{
     46    // According to the specification, we should replace '\r' and '\f' with '\n' but we do not need to
     47    // because our CSSTokenizer treats all of them as new lines.
     48    return string.replace('\0', replacementCharacter);
     49}
     50
    4351CSSTokenizer::CSSTokenizer(const String& string)
     52    : CSSTokenizer(preprocessString(string), nullptr)
     53{
     54}
     55
     56CSSTokenizer::CSSTokenizer(const String& string, CSSParserObserverWrapper& wrapper)
     57    : CSSTokenizer(preprocessString(string), &wrapper)
     58{
     59}
     60
     61inline CSSTokenizer::CSSTokenizer(String&& string, CSSParserObserverWrapper* wrapper)
    4462    : m_input(string)
    4563{
    46     // According to the spec, we should perform preprocessing here.
    47     // See: http://dev.w3.org/csswg/css-syntax/#input-preprocessing
    48     //
    49     // However, we can skip this step since:
    50     // * We're using HTML spaces (which accept \r and \f as a valid white space)
    51     // * Do not count white spaces
    52     // * CSSTokenizerInputStream::nextInputChar() replaces NULLs for replacement characters
    53 
    5464    if (string.isEmpty())
    5565        return;
    … …  
    5868    // Most strings we tokenize have about 3.5 to 5 characters per token.
    5969    m_tokens.reserveInitialCapacity(string.length() / 3);
    60 
    61     while (true) {
    62         CSSParserToken token = nextToken();
    63         if (token.type() == CommentToken)
    64             continue;
    65         if (token.type() == EOFToken)
    66             return;
    67         m_tokens.append(token);
    68     }
    69 }
    70 
    71 CSSTokenizer::CSSTokenizer(const String& string, CSSParserObserverWrapper& wrapper)
    72     : m_input(string)
    73 {
    74     if (string.isEmpty())
    75         return;
    7670
    7771    unsigned offset = 0;
    … …  
    8074        if (token.type() == EOFToken)
    8175            break;
    82         if (token.type() == CommentToken)
    83             wrapper.addComment(offset, m_input.offset(), m_tokens.size());
    84         else {
     76        if (token.type() == CommentToken) {
     77            if (wrapper)
     78                wrapper->addComment(offset, m_input.offset(), m_tokens.size());
     79        } else {
    8580            m_tokens.append(token);
    86             wrapper.addToken(offset);
     81            if (wrapper)
     82                wrapper->addToken(offset);
    8783        }
    8884        offset = m_input.offset();
    8985    }
    9086
    91     wrapper.addToken(offset);
    92     wrapper.finalizeConstruction(m_tokens.begin());
     87    if (wrapper) {
     88        wrapper->addToken(offset);
     89        wrapper->finalizeConstruction(m_tokens.begin());
     90    }
    9391}
    9492
    … …  
    204202{
    205203    ASSERT_UNUSED(cc, cc == '<');
    206     if (m_input.peekWithoutReplacement(0) == '!'
    207         && m_input.peekWithoutReplacement(1) == '-'
    208         && m_input.peekWithoutReplacement(2) == '-') {
     204    if (m_input.peek(0) == '!' && m_input.peek(1) == '-' && m_input.peek(2) == '-') {
    209205        m_input.advance(3);
    210206        return CSSParserToken(CDOToken);
    … …  
    224220        return consumeNumericToken();
    225221    }
    226     if (m_input.peekWithoutReplacement(0) == '-'
    227         && m_input.peekWithoutReplacement(1) == '>') {
     222    if (m_input.peek(0) == '-' && m_input.peek(1) == '>') {
    228223        m_input.advance(2);
    229224        return CSSParserToken(CDCToken);
    … …  
    259254CSSParserToken CSSTokenizer::hash(UChar cc)
    260255{
    261     UChar nextChar = m_input.peekWithoutReplacement(0);
    262     if (isNameCodePoint(nextChar) || twoCharsAreValidEscape(nextChar, m_input.peekWithoutReplacement(1))) {
     256    UChar nextChar = m_input.peek(0);
     257    if (isNameCodePoint(nextChar) || twoCharsAreValidEscape(nextChar, m_input.peek(1))) {
    263258        HashTokenType type = nextCharsAreIdentifier() ? HashTokenId : HashTokenUnrestricted;
    264259        return CSSParserToken(type, consumeName());
    … …  
    312307CSSParserToken CSSTokenizer::reverseSolidus(UChar cc)
    313308{
    314     if (twoCharsAreValidEscape(cc, m_input.peekWithoutReplacement(0))) {
     309    if (twoCharsAreValidEscape(cc, m_input.peek(0))) {
    315310        reconsume(cc);
    316311        return consumeIdentLikeToken();
    … …  
    327322CSSParserToken CSSTokenizer::letterU(UChar cc)
    328323{
    329     if (m_input.peekWithoutReplacement(0) == '+'
    330         && (isASCIIHexDigit(m_input.peekWithoutReplacement(1)) || m_input.peekWithoutReplacement(1) == '?')) {
     324    if (m_input.peek(0) == '+' && (isASCIIHexDigit(m_input.peek(1)) || m_input.peek(1) == '?')) {
    331325        m_input.advance();
    332326        return consumeUnicodeRange();
    … …  
    520514    unsigned numberLength = 0;
    521515
    522     UChar next = m_input.peekWithoutReplacement(0);
     516    UChar next = m_input.peek(0);
    523517    if (next == '+') {
    524518        ++numberLength;
    … …  
    530524
    531525    numberLength = m_input.skipWhilePredicate<isASCIIDigit>(numberLength);
    532     next = m_input.peekWithoutReplacement(numberLength);
    533     if (next == '.' && isASCIIDigit(m_input.peekWithoutReplacement(numberLength + 1))) {
     526    next = m_input.peek(numberLength);
     527    if (next == '.' && isASCIIDigit(m_input.peek(numberLength + 1))) {
    534528        type = NumberValueType;
    535529        numberLength = m_input.skipWhilePredicate<isASCIIDigit>(numberLength + 2);
    536         next = m_input.peekWithoutReplacement(numberLength);
     530        next = m_input.peek(numberLength);
    537531    }
    538532
    539533    if (next == 'E' || next == 'e') {
    540         next = m_input.peekWithoutReplacement(numberLength + 1);
     534        next = m_input.peek(numberLength + 1);
    541535        if (isASCIIDigit(next)) {
    542536            type = NumberValueType;
    543537            numberLength = m_input.skipWhilePredicate<isASCIIDigit>(numberLength + 1);
    544         } else if ((next == '+' || next == '-') && isASCIIDigit(m_input.peekWithoutReplacement(numberLength + 2))) {
     538        } else if ((next == '+' || next == '-') && isASCIIDigit(m_input.peek(numberLength + 2))) {
    545539            type = NumberValueType;
    546540            numberLength = m_input.skipWhilePredicate<isASCIIDigit>(numberLength + 3);
    … …  
    574568            // tokens, but they wouldn't be used and this is easier.
    575569            m_input.advanceUntilNonWhitespace();
    576             UChar next = m_input.peekWithoutReplacement(0);
     570            UChar next = m_input.peek(0);
    577571            if (next != '"' && next != '\'')
    578572                return consumeUrlToken();
    … …  
    588582    // Strings without escapes get handled without allocations
    589583    for (unsigned size = 0; ; size++) {
    590         UChar cc = m_input.peekWithoutReplacement(size);
     584        UChar cc = m_input.peek(size);
    591585        if (cc == endingCodePoint) {
    592586            unsigned startOffset = m_input.offset();
    … …  
    598592            return CSSParserToken(BadStringToken);
    599593        }
    600         if (cc == '\0' || cc == '\\')
     594        if (cc == kEndOfFileMarker || cc == '\\')
    601595            break;
    602596    }
    … …  
    614608            if (m_input.nextInputChar() == kEndOfFileMarker)
    615609                continue;
    616             if (isNewLine(m_input.peekWithoutReplacement(0)))
     610            if (isNewLine(m_input.peek(0)))
    617611                consumeSingleWhitespaceIfNext(); // This handles \r\n for us
    618612            else
    … …  
    625619CSSParserToken CSSTokenizer::consumeUnicodeRange()
    626620{
    627     ASSERT(isASCIIHexDigit(m_input.peekWithoutReplacement(0)) || m_input.peekWithoutReplacement(0) == '?');
     621    ASSERT(isASCIIHexDigit(m_input.peek(0)) || m_input.peek(0) == '?');
    628622    int lengthRemaining = 6;
    629623    UChar32 start = 0;
    630624
    631     while (lengthRemaining && isASCIIHexDigit(m_input.peekWithoutReplacement(0))) {
     625    while (lengthRemaining && isASCIIHexDigit(m_input.peek(0))) {
    632626        start = start * 16 + toASCIIHexValue(consume());
    633627        --lengthRemaining;
    … …  
    641635            --lengthRemaining;
    642636        } while (lengthRemaining && consumeIfNext('?'));
    643     } else if (m_input.peekWithoutReplacement(0) == '-' && isASCIIHexDigit(m_input.peekWithoutReplacement(1))) {
     637    } else if (m_input.peek(0) == '-' && isASCIIHexDigit(m_input.peek(1))) {
    644638        m_input.advance();
    645639        lengthRemaining = 6;
    … …  
    648642            end = end * 16 + toASCIIHexValue(consume());
    649643            --lengthRemaining;
    650         } while (lengthRemaining && isASCIIHexDigit(m_input.peekWithoutReplacement(0)));
     644        } while (lengthRemaining && isASCIIHexDigit(m_input.peek(0)));
    651645    }
    652646
    … …  
    667661    // URL tokens without escapes get handled without allocations
    668662    for (unsigned size = 0; ; size++) {
    669         UChar cc = m_input.peekWithoutReplacement(size);
     663        UChar cc = m_input.peek(size);
    670664        if (cc == ')') {
    671665            unsigned startOffset = m_input.offset();
    … …  
    694688
    695689        if (cc == '\\') {
    696             if (twoCharsAreValidEscape(cc, m_input.peekWithoutReplacement(0))) {
     690            if (twoCharsAreValidEscape(cc, m_input.peek(0))) {
    697691                result.appendCharacter(consumeEscape());
    698692                continue;
    … …  
    715709        if (cc == ')' || cc == kEndOfFileMarker)
    716710            return;
    717         if (twoCharsAreValidEscape(cc, m_input.peekWithoutReplacement(0)))
     711        if (twoCharsAreValidEscape(cc, m_input.peek(0)))
    718712            consumeEscape();
    719713    }
    … …  
    723717{
    724718    // We check for \r\n and HTML spaces since we don't do preprocessing
    725     UChar next = m_input.peekWithoutReplacement(0);
    726     if (next == '\r' && m_input.peekWithoutReplacement(1) == '\n')
     719    UChar next = m_input.peek(0);
     720    if (next == '\r' && m_input.peek(1) == '\n')
    727721        m_input.advance(2);
    728722    else if (isHTMLSpace(next))
    … …  
    752746    // NUL.
    753747    ASSERT(character);
    754     if (m_input.peekWithoutReplacement(0) == character) {
     748    if (m_input.peek(0) == character) {
    755749        m_input.advance();
    756750        return true;
    … …  
    764758    // Names without escapes get handled without allocations
    765759    for (unsigned size = 0; ; ++size) {
    766         UChar cc = m_input.peekWithoutReplacement(size);
     760        UChar cc = m_input.peek(size);
    767761        if (isNameCodePoint(cc))
    768762            continue;
    769         // peekWithoutReplacement will return NUL when we hit the end of the
     763        // peek will return NUL when we hit the end of the
    770764        // input. In that case we want to still use the rangeAt() fast path
    771765        // below.
    772         if (cc == '\0' && m_input.offset() + size < m_input.length())
     766        if (cc == kEndOfFileMarker && m_input.offset() + size < m_input.length())
    773767            break;
    774768        if (cc == '\\')
    … …  
    786780            continue;
    787781        }
    788         if (twoCharsAreValidEscape(cc, m_input.peekWithoutReplacement(0))) {
     782        if (twoCharsAreValidEscape(cc, m_input.peek(0))) {
    789783            result.appendCharacter(consumeEscape());
    790784            continue;
    … …  
    804798        StringBuilder hexChars;
    805799        hexChars.append(cc);
    806         while (consumedHexDigits < 6 && isASCIIHexDigit(m_input.peekWithoutReplacement(0))) {
     800        while (consumedHexDigits < 6 && isASCIIHexDigit(m_input.peek(0))) {
    807801            cc = consume();
    808802            hexChars.append(cc);
    … …  
    825819bool CSSTokenizer::nextTwoCharsAreValidEscape()
    826820{
    827     return twoCharsAreValidEscape(m_input.peekWithoutReplacement(0), m_input.peekWithoutReplacement(1));
     821    return twoCharsAreValidEscape(m_input.peek(0), m_input.peek(1));
    828822}
    829823
    … …  
    831825bool CSSTokenizer::nextCharsAreNumber(UChar first)
    832826{
    833     UChar second = m_input.peekWithoutReplacement(0);
     827    UChar second = m_input.peek(0);
    834828    if (isASCIIDigit(first))
    835829        return true;
    836830    if (first == '+' || first == '-')
    837         return ((isASCIIDigit(second)) || (second == '.' && isASCIIDigit(m_input.peekWithoutReplacement(1))));
     831        return ((isASCIIDigit(second)) || (second == '.' && isASCIIDigit(m_input.peek(1))));
    838832    if (first =='.')
    839833        return (isASCIIDigit(second));
    … …  
    852846bool CSSTokenizer::nextCharsAreIdentifier(UChar first)
    853847{
    854     UChar second = m_input.peekWithoutReplacement(0);
     848    UChar second = m_input.peek(0);
    855849    if (isNameStartCodePoint(first) || twoCharsAreValidEscape(first, second))
    856850        return true;

  • trunk/Source/WebCore/css/parser/CSSTokenizer.h

    r247688 r259773  
    5555
    5656private:
     57    CSSTokenizer(String&&, CSSParserObserverWrapper*);
     58
    5759    CSSParserToken nextToken();
    5860

  • trunk/Source/WebCore/css/parser/CSSTokenizerInputStream.h

    r209466 r259773  
    4848    {
    4949        if (m_offset >= m_stringLength)
    50             return '\0';
    51         UChar result = (*m_string)[m_offset];
    52         return result ? result : 0xFFFD;
     50            return kEndOfFileMarker;
     51        return (*m_string)[m_offset];
    5352    }
    5453
    5554    // Gets the char at lookaheadOffset from the current stream position. Will
    5655    // return NUL (kEndOfFileMarker) if the stream position is at the end.
    57     // NOTE: This may *also* return NUL if there's one in the input! Never
    58     // compare the return value to '\0'.
    59     UChar peekWithoutReplacement(unsigned lookaheadOffset) const
     56    UChar peek(unsigned lookaheadOffset) const
    6057    {
    6158        if ((m_offset + lookaheadOffset) >= m_stringLength)
    62             return '\0';
     59            return kEndOfFileMarker;
    6360        return (*m_string)[m_offset + lookaheadOffset];
    6461    }
Note: See TracChangeset for help on using the changeset viewer.