Changeset 57105 in webkit


Ignore:
Timestamp:
Apr 5, 2010 5:08:34 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-05 Yuta Kitamura <​yutak@chromium.org>

Reviewed by Darin Adler.

Escape control characters in CSS string value when it is serialilzed.

When WebKit serializes a CSS string value that contains binary characters
('\0\1\2' for example), it did not escape these characters. As a result,
users got (invisible) control characters through scripts. This change fixes
this issue.

As a side effect, two separate codes for escaping CSS strings are merged, and
become a public function (quoteCSSString).

CSS string value is not correctly serialized when it contains binary characters
​https://bugs.webkit.org/show_bug.cgi?id=28938

  • fast/css/script-tests/string-quote-binary.js: Added.
  • fast/css/string-quote-binary-expected.txt: Added.
  • fast/css/string-quote-binary.html: Added.
  • fast/js/resources/js-test-pre.js: (shouldBeEqualToString): Considering the case when the argument contains binary characters.

2010-04-05 Yuta Kitamura <​yutak@chromium.org>

Reviewed by Darin Adler.

Escape control characters in CSS string value when it is serialilzed.

When WebKit serializes a CSS string value that contains binary characters
('\0\1\2' for example), it did not escape these characters. As a result,
users got (invisible) control characters through scripts. This change fixes
this issue.

As a side effect, two separate codes for escaping CSS strings are merged, and
become a public function (quoteCSSString).

CSS string value is not correctly serialized when it contains binary characters
​https://bugs.webkit.org/show_bug.cgi?id=28938

Test: fast/css/string-quote-binary.html

  • css/CSSParser.cpp: (WebCore::isCSSTokenizerIdentifier): (WebCore::isCSSTokenizerURL): (WebCore::quoteCSSString): (WebCore::quoteCSSStringIfNeeded): (WebCore::quoteCSSURLIfNeeded):
  • css/CSSParser.h:
  • css/CSSPrimitiveValue.cpp: (WebCore::CSSPrimitiveValue::cssText):
  • css/FontFamilyValue.cpp: (WebCore::FontFamilyValue::cssText):
Location:
trunk
Files:
3 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r57103 r57105  
     12010-04-05  Yuta Kitamura  <yutak@chromium.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Escape control characters in CSS string value when it is serialilzed.
     6
     7        When WebKit serializes a CSS string value that contains binary characters
     8        ('\0\1\2' for example), it did not escape these characters. As a result,
     9        users got (invisible) control characters through scripts. This change fixes
     10        this issue.
     11
     12        As a side effect, two separate codes for escaping CSS strings are merged, and
     13        become a public function (quoteCSSString).
     14
     15        CSS string value is not correctly serialized when it contains binary characters
     16        https://bugs.webkit.org/show_bug.cgi?id=28938
     17
     18        * fast/css/script-tests/string-quote-binary.js: Added.
     19        * fast/css/string-quote-binary-expected.txt: Added.
     20        * fast/css/string-quote-binary.html: Added.
     21        * fast/js/resources/js-test-pre.js:
     22        (shouldBeEqualToString): Considering the case when the argument contains binary characters.
     23
    1242010-04-05  John Gregg  <johnnyg@google.com>
    225

  • trunk/LayoutTests/fast/js/resources/js-test-pre.js

    r55430 r57105  
    123123function shouldBeEqualToString(a, b)
    124124{
    125   var unevaledString = '"' + b.replace(/"/g, "\"") + '"';
     125  var unevaledString = '"' + b.replace(/\\/g, "\\\\").replace(/"/g, "\"") + '"';
    126126  shouldBe(a, unevaledString);
    127127}

  • trunk/WebCore/ChangeLog

    r57103 r57105  
     12010-04-05  Yuta Kitamura  <yutak@chromium.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Escape control characters in CSS string value when it is serialilzed.
     6
     7        When WebKit serializes a CSS string value that contains binary characters
     8        ('\0\1\2' for example), it did not escape these characters. As a result,
     9        users got (invisible) control characters through scripts. This change fixes
     10        this issue.
     11
     12        As a side effect, two separate codes for escaping CSS strings are merged, and
     13        become a public function (quoteCSSString).
     14
     15        CSS string value is not correctly serialized when it contains binary characters
     16        https://bugs.webkit.org/show_bug.cgi?id=28938
     17
     18        Test: fast/css/string-quote-binary.html
     19
     20        * css/CSSParser.cpp:
     21        (WebCore::isCSSTokenizerIdentifier):
     22        (WebCore::isCSSTokenizerURL):
     23        (WebCore::quoteCSSString):
     24        (WebCore::quoteCSSStringIfNeeded):
     25        (WebCore::quoteCSSURLIfNeeded):
     26        * css/CSSParser.h:
     27        * css/CSSPrimitiveValue.cpp:
     28        (WebCore::CSSPrimitiveValue::cssText):
     29        * css/FontFamilyValue.cpp:
     30        (WebCore::FontFamilyValue::cssText):
     31
    1322010-04-05  John Gregg  <johnnyg@google.com>
    233

  • trunk/WebCore/css/CSSParser.cpp

    r56825 r57105  
    6666#include "Rect.h"
    6767#include "ShadowValue.h"
     68#include "StringBuffer.h"
    6869#include "WebKitCSSKeyframeRule.h"
    6970#include "WebKitCSSKeyframesRule.h"
    … …  
    54015402}
    54025403
     5404// "ident" from the CSS tokenizer, minus backslash-escape sequences
     5405static bool isCSSTokenizerIdentifier(const String& string)
     5406{
     5407    const UChar* p = string.characters();
     5408    const UChar* end = p + string.length();
     5409
     5410    // -?
     5411    if (p != end && p[0] == '-')
     5412        ++p;
     5413
     5414    // {nmstart}
     5415    if (p == end || !(p[0] == '_' || p[0] >= 128 || isASCIIAlpha(p[0])))
     5416        return false;
     5417    ++p;
     5418
     5419    // {nmchar}*
     5420    for (; p != end; ++p) {
     5421        if (!(p[0] == '_' || p[0] == '-' || p[0] >= 128 || isASCIIAlphanumeric(p[0])))
     5422            return false;
     5423    }
     5424
     5425    return true;
     5426}
     5427
     5428// "url" from the CSS tokenizer, minus backslash-escape sequences
     5429static bool isCSSTokenizerURL(const String& string)
     5430{
     5431    const UChar* p = string.characters();
     5432    const UChar* end = p + string.length();
     5433
     5434    for (; p != end; ++p) {
     5435        UChar c = p[0];
     5436        switch (c) {
     5437            case '!':
     5438            case '#':
     5439            case '$':
     5440            case '%':
     5441            case '&':
     5442                break;
     5443            default:
     5444                if (c < '*')
     5445                    return false;
     5446                if (c <= '~')
     5447                    break;
     5448                if (c < 128)
     5449                    return false;
     5450        }
     5451    }
     5452
     5453    return true;
     5454}
     5455
     5456// We use single quotes for now because markup.cpp uses double quotes.
     5457String quoteCSSString(const String& string)
     5458{
     5459    // For efficiency, we first pre-calculate the length of the quoted string, then we build the actual one.
     5460    // Please see below for the actual logic.
     5461    unsigned quotedStringSize = 2; // Two quotes surrounding the entire string.
     5462    bool afterEscape = false;
     5463    for (unsigned i = 0; i < string.length(); ++i) {
     5464        UChar ch = string[i];
     5465        if (ch == '\\' || ch == '\'') {
     5466            quotedStringSize += 2;
     5467            afterEscape = false;
     5468        } else if (ch < 0x20 || ch == 0x7F) {
     5469            quotedStringSize += 2 + (ch >= 0x10);
     5470            afterEscape = true;
     5471        } else {
     5472            quotedStringSize += 1 + (afterEscape && (isASCIIHexDigit(ch) || ch == ' '));
     5473            afterEscape = false;
     5474        }
     5475    }
     5476
     5477    StringBuffer buffer(quotedStringSize);
     5478    unsigned index = 0;
     5479    buffer[index++] = '\'';
     5480    afterEscape = false;
     5481    for (unsigned i = 0; i < string.length(); ++i) {
     5482        UChar ch = string[i];
     5483        if (ch == '\\' || ch == '\'') {
     5484            buffer[index++] = '\\';
     5485            buffer[index++] = ch;
     5486            afterEscape = false;
     5487        } else if (ch < 0x20 || ch == 0x7F) { // Control characters.
     5488            static const char hexDigits[17] = "0123456789abcdef";
     5489            buffer[index++] = '\\';
     5490            if (ch >= 0x10)
     5491                buffer[index++] = hexDigits[ch >> 4];
     5492            buffer[index++] = hexDigits[ch & 0xF];
     5493            afterEscape = true;
     5494        } else {
     5495            // Space character may be required to separate backslash-escape sequence and normal characters.
     5496            if (afterEscape && (isASCIIHexDigit(ch) || ch == ' '))
     5497                buffer[index++] = ' ';
     5498            buffer[index++] = ch;
     5499            afterEscape = false;
     5500        }
     5501    }
     5502    buffer[index++] = '\'';
     5503
     5504    ASSERT(quotedStringSize == index);
     5505    return String::adopt(buffer);
     5506}
     5507
     5508String quoteCSSStringIfNeeded(const String& string)
     5509{
     5510    return isCSSTokenizerIdentifier(string) ? string : quoteCSSString(string);
     5511}
     5512
     5513String quoteCSSURLIfNeeded(const String& string)
     5514{
     5515    return isCSSTokenizerURL(string) ? string : quoteCSSString(string);
     5516}
     5517
    54035518#define YY_DECL int CSSParser::lex()
    54045519#define yyconst const

  • trunk/WebCore/css/CSSParser.h

    r56383 r57105  
    321321    };
    322322
     323    String quoteCSSString(const String&);
     324    String quoteCSSStringIfNeeded(const String&);
     325    String quoteCSSURLIfNeeded(const String&);
     326
    323327} // namespace WebCore
    324328

  • trunk/WebCore/css/CSSPrimitiveValue.cpp

    r57101 r57105  
    2323
    2424#include "CSSHelper.h"
     25#include "CSSParser.h"
    2526#include "CSSPropertyNames.h"
    2627#include "CSSStyleSheet.h"
    … …  
    143144}
    144145
    145 // "ident" from the CSS tokenizer, minus backslash-escape sequences
    146 static bool isCSSTokenizerIdentifier(const String& string)
    147 {
    148     const UChar* p = string.characters();
    149     const UChar* end = p + string.length();
    150 
    151     // -?
    152     if (p != end && p[0] == '-')
    153         ++p;
    154 
    155     // {nmstart}
    156     if (p == end || !(p[0] == '_' || p[0] >= 128 || isASCIIAlpha(p[0])))
    157         return false;
    158     ++p;
    159 
    160     // {nmchar}*
    161     for (; p != end; ++p) {
    162         if (!(p[0] == '_' || p[0] == '-' || p[0] >= 128 || isASCIIAlphanumeric(p[0])))
    163             return false;
    164     }
    165 
    166     return true;
    167 }
    168 
    169 // "url" from the CSS tokenizer, minus backslash-escape sequences
    170 static bool isCSSTokenizerURL(const String& string)
    171 {
    172     const UChar* p = string.characters();
    173     const UChar* end = p + string.length();
    174 
    175     for (; p != end; ++p) {
    176         UChar c = p[0];
    177         switch (c) {
    178             case '!':
    179             case '#':
    180             case '$':
    181             case '%':
    182             case '&':
    183                 break;
    184             default:
    185                 if (c < '*')
    186                     return false;
    187                 if (c <= '~')
    188                     break;
    189                 if (c < 128)
    190                     return false;
    191         }
    192     }
    193 
    194     return true;
    195 }
    196 
    197 // We use single quotes for now because markup.cpp uses double quotes.
    198 static String quoteString(const String& string)
    199 {
    200     // FIXME: Also need to escape characters like '\n'.
    201     String s = string;
    202     s.replace('\\', "\\\\");
    203     s.replace('\'', "\\'");
    204     return "'" + s + "'";
    205 }
    206 
    207 static String quoteStringIfNeeded(const String& string)
    208 {
    209     return isCSSTokenizerIdentifier(string) ? string : quoteString(string);
    210 }
    211 
    212 static String quoteURLIfNeeded(const String& string)
    213 {
    214     return isCSSTokenizerURL(string) ? string : quoteString(string);
    215 }
    216 
    217146CSSPrimitiveValue::CSSPrimitiveValue()
    218147    : m_type(0)
    … …  
    775704            break;
    776705        case CSS_STRING:
    777             text = quoteStringIfNeeded(m_value.string);
     706            text = quoteCSSStringIfNeeded(m_value.string);
    778707            break;
    779708        case CSS_URI:
    780             text = "url(" + quoteURLIfNeeded(m_value.string) + ")";
     709            text = "url(" + quoteCSSURLIfNeeded(m_value.string) + ")";
    781710            break;
    782711        case CSS_IDENT:
    … …  
    906835        }
    907836        case CSS_PARSER_IDENTIFIER:
    908             text = quoteStringIfNeeded(m_value.string);
     837            text = quoteCSSStringIfNeeded(m_value.string);
    909838            break;
    910839    }

  • trunk/WebCore/css/FontFamilyValue.cpp

    r34404 r57105  
    2222#include "FontFamilyValue.h"
    2323
     24#include "CSSParser.h"
     25
    2426namespace WebCore {
    25 
    26 // FIXME: This appears identical to isCSSTokenizerIdentifier from CSSPrimitiveValue.cpp, we should use a single function.
    27 static bool isValidCSSIdentifier(const String& string)
    28 {
    29     unsigned length = string.length();
    30     if (!length)
    31         return false;
    32 
    33     const UChar* characters = string.characters();
    34     UChar c = characters[0];
    35     if (!(c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '-' || c >= 0x80))
    36         return false;
    37 
    38     for (unsigned i = 1; i < length; ++i) {
    39         c = characters[i];
    40         if (!(c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c >= 0x80))
    41             return false;
    42     }
    43 
    44     return true;
    45 }
    46 
    47 // Quotes the string if it needs quoting.
    48 // We use single quotes because serialization code uses double quotes, and it's nice to
    49 // avoid having to turn all the quote marks into &quot; as we would have to.
    50 static String quoteStringIfNeeded(const String& string)
    51 {
    52     if (isValidCSSIdentifier(string))
    53         return string;
    54 
    55     // FIXME: Also need to transform control characters (00-1F) into \ sequences.
    56     // FIXME: This is inefficient -- should use a Vector<UChar> instead.
    57     String quotedString = string;
    58     quotedString.replace('\\', "\\\\");
    59     quotedString.replace('\'', "\\'");
    60     return "'" + quotedString + "'";
    61 }
    6227
    6328FontFamilyValue::FontFamilyValue(const String& familyName)
    … …  
    10267String FontFamilyValue::cssText() const
    10368{
    104     return quoteStringIfNeeded(m_familyName);
     69    return quoteCSSStringIfNeeded(m_familyName);
    10570}
    10671
Note: See TracChangeset for help on using the changeset viewer.