Changeset 143415 in webkit


Ignore:
Timestamp:
Feb 19, 2013, 5:20:48 PM (12 years ago)
Author:
tonyg@chromium.org
Message:

Fix crash in preloading scanning base tags with no href attribute for background parser
​https://bugs.webkit.org/show_bug.cgi?id=110276

Reviewed by Eric Seidel.

Previously a <base> tag without an href attribute (like the one in fast/dom/HTMLAnchorElement/set-href-attribute-rebase.html)
would crash the background parser's preload scanner.

To fix that, we only call stripLeadingAndTrailingHTMLSpaces() if the href attribute is non-null. This matches the main thread parser.

Along with this, I decided to templatize updatePredictedBaseURL() so that the main and background parser can share the same impl.

This required making CompactHTMLToken and HTMLToken a little more similar:

  1. Give HTMLToken a getAttributeItem() method.
  2. Move CompactAttribute to CompactHTMLToken::Attribute and make it a struct.

No new tests because covered by existing tests.

  • html/parser/AtomicHTMLToken.h:

(WebCore::AtomicHTMLToken::AtomicHTMLToken):

  • html/parser/CompactHTMLToken.cpp:

(SameSizeAsCompactHTMLToken):
(WebCore::CompactHTMLToken::CompactHTMLToken):
(WebCore::CompactHTMLToken::getAttributeItem):
(WebCore::CompactHTMLToken::isSafeToSendToAnotherThread):

  • html/parser/CompactHTMLToken.h:

(WebCore::CompactHTMLToken::Attribute::Attribute):
(Attribute):
(WebCore::CompactHTMLToken::attributes):
(CompactHTMLToken):
(WebCore::CompactHTMLToken::publicIdentifier):
(WebCore::CompactHTMLToken::systemIdentifier):

  • html/parser/HTMLParserIdioms.h:

(WebCore):
(WebCore::stripLeadingAndTrailingHTMLSpaces):

  • html/parser/HTMLPreloadScanner.cpp:

(WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
(WebCore):
(WebCore::TokenPreloadScanner::updatePredictedBaseURL):

  • html/parser/HTMLPreloadScanner.h:
  • html/parser/HTMLToken.h:

(WebCore::HTMLToken::getAttributeItem):
(HTMLToken):

Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r143412 r143415  
     12013-02-19  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Fix crash in preloading scanning base tags with no href attribute for background parser
     4        https://bugs.webkit.org/show_bug.cgi?id=110276
     5
     6        Reviewed by Eric Seidel.
     7
     8        Previously a <base> tag without an href attribute (like the one in fast/dom/HTMLAnchorElement/set-href-attribute-rebase.html)
     9        would crash the background parser's preload scanner.
     10
     11        To fix that, we only call stripLeadingAndTrailingHTMLSpaces() if the href attribute is non-null. This matches the main thread parser.
     12
     13        Along with this, I decided to templatize updatePredictedBaseURL() so that the main and background parser can share the same impl.
     14
     15        This required making CompactHTMLToken and HTMLToken a little more similar:
     16        1. Give HTMLToken a getAttributeItem() method.
     17        2. Move CompactAttribute to CompactHTMLToken::Attribute and make it a struct.
     18
     19        No new tests because covered by existing tests.
     20
     21        * html/parser/AtomicHTMLToken.h:
     22        (WebCore::AtomicHTMLToken::AtomicHTMLToken):
     23        * html/parser/CompactHTMLToken.cpp:
     24        (SameSizeAsCompactHTMLToken):
     25        (WebCore::CompactHTMLToken::CompactHTMLToken):
     26        (WebCore::CompactHTMLToken::getAttributeItem):
     27        (WebCore::CompactHTMLToken::isSafeToSendToAnotherThread):
     28        * html/parser/CompactHTMLToken.h:
     29        (WebCore::CompactHTMLToken::Attribute::Attribute):
     30        (Attribute):
     31        (WebCore::CompactHTMLToken::attributes):
     32        (CompactHTMLToken):
     33        (WebCore::CompactHTMLToken::publicIdentifier):
     34        (WebCore::CompactHTMLToken::systemIdentifier):
     35        * html/parser/HTMLParserIdioms.h:
     36        (WebCore):
     37        (WebCore::stripLeadingAndTrailingHTMLSpaces):
     38        * html/parser/HTMLPreloadScanner.cpp:
     39        (WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
     40        (WebCore):
     41        (WebCore::TokenPreloadScanner::updatePredictedBaseURL):
     42        * html/parser/HTMLPreloadScanner.h:
     43        * html/parser/HTMLToken.h:
     44        (WebCore::HTMLToken::getAttributeItem):
     45        (HTMLToken):
     46
    1472013-02-19  Mark Lam  <mark.lam@apple.com>
    248

  • trunk/Source/WebCore/html/parser/AtomicHTMLToken.h

    r142712 r143415  
    202202        case HTMLToken::StartTag:
    203203            m_attributes.reserveInitialCapacity(token.attributes().size());
    204             for (Vector<CompactAttribute>::const_iterator it = token.attributes().begin(); it != token.attributes().end(); ++it)
    205                 m_attributes.append(Attribute(QualifiedName(nullAtom, it->name(), nullAtom), it->value()));
     204            for (Vector<CompactHTMLToken::Attribute>::const_iterator it = token.attributes().begin(); it != token.attributes().end(); ++it)
     205                m_attributes.append(Attribute(QualifiedName(nullAtom, it->name, nullAtom), it->value));
    206206            // Fall through!
    207207        case HTMLToken::EndTag:

  • trunk/Source/WebCore/html/parser/CompactHTMLToken.cpp

    r142829 r143415  
    4040    unsigned bitfields;
    4141    String name;
    42     Vector<CompactAttribute> vector;
     42    Vector<Attribute> vector;
    4343    TextPosition textPosition;
    4444    OwnPtr<XSSInfo> xssInfo;
    … …  
    6161        // There is only 1 DOCTYPE token per document, so to avoid increasing the
    6262        // size of CompactHTMLToken, we just use the m_attributes vector.
    63         m_attributes.append(CompactAttribute(String(token->publicIdentifier()), String(token->systemIdentifier())));
     63        m_attributes.append(Attribute(String(token->publicIdentifier()), String(token->systemIdentifier())));
    6464        m_doctypeForcesQuirks = token->forceQuirks();
    6565        break;
    … …  
    7171        // FIXME: Attribute names and values should be 8bit when possible.
    7272        for (Vector<HTMLToken::Attribute>::const_iterator it = token->attributes().begin(); it != token->attributes().end(); ++it)
    73             m_attributes.append(CompactAttribute(String(it->name), String(it->value)));
     73            m_attributes.append(Attribute(String(it->name), String(it->value)));
    7474        // Fall through!
    7575    case HTMLToken::EndTag:
    … …  
    103103}
    104104
    105 const CompactAttribute* CompactHTMLToken::getAttributeItem(const QualifiedName& name) const
     105const CompactHTMLToken::Attribute* CompactHTMLToken::getAttributeItem(const QualifiedName& name) const
    106106{
    107107    for (unsigned i = 0; i < m_attributes.size(); ++i) {
    108         if (threadSafeMatch(m_attributes.at(i).name(), name))
     108        if (threadSafeMatch(m_attributes.at(i).name, name))
    109109            return &m_attributes.at(i);
    110110    }
    … …  
    114114bool CompactHTMLToken::isSafeToSendToAnotherThread() const
    115115{
    116     for (Vector<CompactAttribute>::const_iterator it = m_attributes.begin(); it != m_attributes.end(); ++it) {
    117         if (!it->name().isSafeToSendToAnotherThread())
     116    for (Vector<Attribute>::const_iterator it = m_attributes.begin(); it != m_attributes.end(); ++it) {
     117        if (!it->name.isSafeToSendToAnotherThread())
    118118            return false;
    119         if (!it->value().isSafeToSendToAnotherThread())
     119        if (!it->value.isSafeToSendToAnotherThread())
    120120            return false;
    121121    }

  • trunk/Source/WebCore/html/parser/CompactHTMLToken.h

    r142829 r143415  
    4343class XSSInfo;
    4444
    45 class CompactAttribute {
    46 public:
    47     CompactAttribute(const String& name, const String& value)
    48         : m_name(name)
    49         , m_value(value)
    50     {
    51     }
    52 
    53     const String& name() const { return m_name; }
    54     const String& value() const { return m_value; }
    55 
    56 private:
    57     String m_name;
    58     String m_value;
    59 };
    60 
    6145class CompactHTMLToken {
    6246public:
     47    struct Attribute {
     48        Attribute(const String& name, const String& value)
     49            : name(name)
     50            , value(value)
     51        {
     52        }
     53
     54        String name;
     55        String value;
     56    };
     57
    6358    CompactHTMLToken(const HTMLToken*, const TextPosition&);
    6459    CompactHTMLToken(const CompactHTMLToken&);
    … …  
    7065    bool selfClosing() const { return m_selfClosing; }
    7166    bool isAll8BitData() const { return m_isAll8BitData; }
    72     const Vector<CompactAttribute>& attributes() const { return m_attributes; }
    73     const CompactAttribute* getAttributeItem(const QualifiedName&) const;
     67    const Vector<Attribute>& attributes() const { return m_attributes; }
     68    const Attribute* getAttributeItem(const QualifiedName&) const;
    7469    const TextPosition& textPosition() const { return m_textPosition; }
    7570
    7671    // There is only 1 DOCTYPE token per document, so to avoid increasing the
    7772    // size of CompactHTMLToken, we just use the m_attributes vector.
    78     const String& publicIdentifier() const { return m_attributes[0].name(); }
    79     const String& systemIdentifier() const { return m_attributes[0].value(); }
     73    const String& publicIdentifier() const { return m_attributes[0].name; }
     74    const String& systemIdentifier() const { return m_attributes[0].value; }
    8075    bool doctypeForcesQuirks() const { return m_doctypeForcesQuirks; }
    8176    XSSInfo* xssInfo() const;
    … …  
    8984
    9085    String m_data; // "name", "characters", or "data" depending on m_type
    91     Vector<CompactAttribute> m_attributes;
     86    Vector<Attribute> m_attributes;
    9287    TextPosition m_textPosition;
    9388    OwnPtr<XSSInfo> m_xssInfo;

  • trunk/Source/WebCore/html/parser/HTMLParserIdioms.h

    r141686 r143415  
    2727
    2828#include <wtf/Forward.h>
     29#include <wtf/text/WTFString.h>
    2930#include <wtf/unicode/Unicode.h>
    3031
    … …  
    4142// Strip leading and trailing whitespace as defined by the HTML specification.
    4243String stripLeadingAndTrailingHTMLSpaces(const String&);
     44template<size_t inlineCapacity>
     45String stripLeadingAndTrailingHTMLSpaces(const Vector<UChar, inlineCapacity>& vector)
     46{
     47    return stripLeadingAndTrailingHTMLSpaces(StringImpl::create8BitIfPossible(vector));
     48}
    4349
    4450// An implementation of the HTML specification's algorithm to convert a number to a string for number and range types.

  • trunk/Source/WebCore/html/parser/HTMLPreloadScanner.cpp

    r143399 r143415  
    128128
    129129#if ENABLE(THREADED_HTML_PARSER)
    130     void processAttributes(const Vector<CompactAttribute>& attributes)
     130    void processAttributes(const Vector<CompactHTMLToken::Attribute>& attributes)
    131131    {
    132132        if (m_tagId >= UnknownTagId)
    133133            return;
    134         for (Vector<CompactAttribute>::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter)
    135             processAttribute(iter->name(), iter->value());
     134        for (Vector<CompactHTMLToken::Attribute>::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter)
     135            processAttribute(iter->name, iter->value);
    136136    }
    137137#endif
    … …  
    338338}
    339339
    340 void TokenPreloadScanner::updatePredictedBaseURL(const HTMLToken& token)
     340template<typename Token>
     341void TokenPreloadScanner::updatePredictedBaseURL(const Token& token)
    341342{
    342343    ASSERT(m_predictedBaseElementURL.isEmpty());
    343     for (HTMLToken::AttributeList::const_iterator iter = token.attributes().begin(); iter != token.attributes().end(); ++iter) {
    344         AtomicString attributeName(iter->name);
    345         if (attributeName == hrefAttr) {
    346             String hrefValue = StringImpl::create8BitIfPossible(iter->value);
    347             m_predictedBaseElementURL = KURL(m_documentURL, stripLeadingAndTrailingHTMLSpaces(hrefValue));
    348             break;
    349         }
    350     }
    351 }
    352 
    353 #if ENABLE(THREADED_HTML_PARSER)
    354 void TokenPreloadScanner::updatePredictedBaseURL(const CompactHTMLToken& token)
    355 {
    356     ASSERT(m_predictedBaseElementURL.isEmpty());
    357     m_predictedBaseElementURL = KURL(m_documentURL, stripLeadingAndTrailingHTMLSpaces(token.getAttributeItem(hrefAttr)->value())).copy();
    358 }
    359 #endif
     344    if (const typename Token::Attribute* hrefAttribute = token.getAttributeItem(hrefAttr))
     345        m_predictedBaseElementURL = KURL(m_documentURL, stripLeadingAndTrailingHTMLSpaces(hrefAttribute->value)).copy();
     346}
    360347
    361348HTMLPreloadScanner::HTMLPreloadScanner(const HTMLParserOptions& options, const KURL& documentURL)

  • trunk/Source/WebCore/html/parser/HTMLPreloadScanner.h

    r143399 r143415  
    8383    static String initiatorFor(TagId);
    8484
    85     void updatePredictedBaseURL(const HTMLToken&);
    86 #if ENABLE(THREADED_HTML_PARSER)
    87     void updatePredictedBaseURL(const CompactHTMLToken&);
    88 #endif
     85    template<typename Token>
     86    void updatePredictedBaseURL(const Token&);
    8987
    9088    CSSPreloadScanner m_cssScanner;

  • trunk/Source/WebCore/html/parser/HTMLToken.h

    r142712 r143415  
    359359    }
    360360
     361    const Attribute* getAttributeItem(const QualifiedName& name) const
     362    {
     363        for (unsigned i = 0; i < m_attributes.size(); ++i) {
     364            if (AtomicString(m_attributes.at(i).name) == name.localName())
     365                return &m_attributes.at(i);
     366        }
     367        return 0;
     368    }
     369
    361370    // Used by the XSSAuditor to nuke XSS-laden attributes.
    362371    void eraseValueOfAttribute(size_t i)
Note: See TracChangeset for help on using the changeset viewer.