Changeset 141938 in webkit


Ignore:
Timestamp:
Feb 5, 2013 2:39:00 PM (11 years ago)
Author:
tonyg@chromium.org
Message:

Continue making XSSAuditor thread safe: Remove dependency on the parser's tokenizer
https://bugs.webkit.org/show_bug.cgi?id=108666

Reviewed by Adam Barth.

This is the final dependency on the parser, so we remove that as well. Yay!

No new tests because no new functionality.

  • html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::HTMLDocumentParser):
(WebCore::HTMLDocumentParser::pumpTokenizer): Pass m_tokenizer->shouldAllowCDATA()

  • html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::XSSAuditor): Remove isMainThread() check because we have one in init() anyway.
Move m_isEnabled and m_documentURL initialization to init() because we have a Document* there.
(WebCore::XSSAuditor::init):
(WebCore::XSSAuditor::filterToken):
(WebCore::XSSAuditor::filterStartToken):
(WebCore::XSSAuditor::filterEndToken):
(WebCore::XSSAuditor::filterScriptToken):
(WebCore::XSSAuditor::decodedSnippetForJavaScript):

  • html/parser/XSSAuditor.h:

(WebCore::FilterTokenRequest::FilterTokenRequest):
(FilterTokenRequest):
(XSSAuditor):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r141937 r141938  
     12013-02-05  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Continue making XSSAuditor thread safe: Remove dependency on the parser's tokenizer
     4        https://bugs.webkit.org/show_bug.cgi?id=108666
     5
     6        Reviewed by Adam Barth.
     7
     8        This is the final dependency on the parser, so we remove that as well. Yay!
     9
     10        No new tests because no new functionality.
     11
     12        * html/parser/HTMLDocumentParser.cpp:
     13        (WebCore::HTMLDocumentParser::HTMLDocumentParser):
     14        (WebCore::HTMLDocumentParser::pumpTokenizer): Pass m_tokenizer->shouldAllowCDATA()
     15        * html/parser/XSSAuditor.cpp:
     16        (WebCore::XSSAuditor::XSSAuditor): Remove isMainThread() check because we have one in init() anyway.
     17        Move m_isEnabled and m_documentURL initialization to init() because we have a Document* there.
     18        (WebCore::XSSAuditor::init):
     19        (WebCore::XSSAuditor::filterToken):
     20        (WebCore::XSSAuditor::filterStartToken):
     21        (WebCore::XSSAuditor::filterEndToken):
     22        (WebCore::XSSAuditor::filterScriptToken):
     23        (WebCore::XSSAuditor::decodedSnippetForJavaScript):
     24        * html/parser/XSSAuditor.h:
     25        (WebCore::FilterTokenRequest::FilterTokenRequest):
     26        (FilterTokenRequest):
     27        (XSSAuditor):
     28
    1292013-02-05  Enrica Casucci  <enrica@apple.com>
    230
  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

    r141909 r141938  
    8383    , m_treeBuilder(HTMLTreeBuilder::create(this, document, reportErrors, m_options))
    8484    , m_parserScheduler(HTMLParserScheduler::create(this))
    85     , m_xssAuditor(this)
    8685    , m_xssAuditorDelegate(document)
    8786#if ENABLE(THREADED_HTML_PARSER)
     
    103102    , m_tokenizer(HTMLTokenizer::create(m_options))
    104103    , m_treeBuilder(HTMLTreeBuilder::create(this, fragment, contextElement, scriptingPermission, m_options))
    105     , m_xssAuditor(this)
    106104    , m_xssAuditorDelegate(fragment->document())
    107105#if ENABLE(THREADED_HTML_PARSER)
     
    379377            // We do not XSS filter innerHTML, which means we (intentionally) fail
    380378            // http/tests/security/xssAuditor/dom-write-innerHTML.html
    381             OwnPtr<DidBlockScriptRequest> request = m_xssAuditor.filterToken(FilterTokenRequest(token(), m_sourceTracker, document()->decoder()));
    382             if (request)
     379            if (OwnPtr<DidBlockScriptRequest> request = m_xssAuditor.filterToken(FilterTokenRequest(token(), m_sourceTracker, document()->decoder(), m_tokenizer->shouldAllowCDATA())))
    383380                m_xssAuditorDelegate.didBlockScript(request.release());
    384381        }
  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r141909 r141938  
    175175}
    176176
    177 XSSAuditor::XSSAuditor(HTMLDocumentParser* parser)
    178     : m_parser(parser)
    179     , m_documentURL(parser->document()->url())
    180     , m_isEnabled(false)
     177XSSAuditor::XSSAuditor()
     178    : m_isEnabled(false)
    181179    , m_xssProtection(XSSProtectionEnabled)
    182180    , m_state(Uninitialized)
    183     , m_shouldAllowCDATA(false)
    184181    , m_scriptTagNestingLevel(0)
    185182{
    186     ASSERT(isMainThread());
    187     ASSERT(m_parser);
    188     if (Frame* frame = parser->document()->frame()) {
    189         if (Settings* settings = frame->settings())
    190             m_isEnabled = settings->xssAuditorEnabled();
    191     }
    192183    // Although tempting to call init() at this point, the various objects
    193184    // we want to reference might not all have been constructed yet.
     
    205196    m_state = Initialized;
    206197
     198    if (Frame* frame = document->frame())
     199        if (Settings* settings = frame->settings())
     200            m_isEnabled = settings->xssAuditorEnabled();
     201
    207202    if (!m_isEnabled)
    208203        return;
     204
     205    m_documentURL = document->url();
    209206
    210207    // In theory, the Document could have detached from the Frame after the
     
    292289            didBlockScript = filterCharacterToken(request);
    293290        else if (request.token.type() == HTMLTokenTypes::EndTag)
    294             filterEndToken(request.token);
     291            filterEndToken(request);
    295292    }
    296293
     
    314311    if (hasName(request.token, scriptTag)) {
    315312        didBlockScript |= filterScriptToken(request);
    316         ASSERT(m_shouldAllowCDATA || !m_scriptTagNestingLevel);
     313        ASSERT(request.shouldAllowCDATA || !m_scriptTagNestingLevel);
    317314        m_scriptTagNestingLevel++;
    318315    } else if (hasName(request.token, objectTag))
     
    336333}
    337334
    338 void XSSAuditor::filterEndToken(HTMLToken& token)
     335void XSSAuditor::filterEndToken(const FilterTokenRequest& request)
    339336{
    340337    ASSERT(m_scriptTagNestingLevel);
    341     if (hasName(token, scriptTag)) {
     338    if (hasName(request.token, scriptTag)) {
    342339        m_scriptTagNestingLevel--;
    343         ASSERT(m_shouldAllowCDATA || !m_scriptTagNestingLevel);
     340        ASSERT(request.shouldAllowCDATA || !m_scriptTagNestingLevel);
    344341    }
    345342}
     
    362359
    363360    m_cachedDecodedSnippet = decodedSnippetForName(request);
    364     m_shouldAllowCDATA = m_parser->tokenizer()->shouldAllowCDATA();
    365361
    366362    bool didBlockScript = false;
     
    589585        // these as a separate comment tokens. Having consumed whitespace, we need not look
    590586        // further for these.
    591         if (m_shouldAllowCDATA)
     587        if (request.shouldAllowCDATA)
    592588            break;
    593589
     
    616612        // whitespace only. We should have enough text in these cases to avoid false positives.
    617613        for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
    618             if (!m_shouldAllowCDATA) {
     614            if (!request.shouldAllowCDATA) {
    619615                if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
    620616                    foundPosition += 2;
  • trunk/Source/WebCore/html/parser/XSSAuditor.h

    r141909 r141938  
    4141
    4242struct FilterTokenRequest {
    43     FilterTokenRequest(HTMLToken& token, HTMLSourceTracker& sourceTracker, const TextResourceDecoder* decoder)
     43    FilterTokenRequest(HTMLToken& token, HTMLSourceTracker& sourceTracker, const TextResourceDecoder* decoder, bool shouldAllowCDATA)
    4444        : token(token)
    4545        , sourceTracker(sourceTracker)
    4646        , decoder(decoder)
     47        , shouldAllowCDATA(shouldAllowCDATA)
    4748    { }
    4849
     
    5051    HTMLSourceTracker& sourceTracker;
    5152    const TextResourceDecoder* decoder;
     53    bool shouldAllowCDATA;
    5254};
    5355
     
    5557    WTF_MAKE_NONCOPYABLE(XSSAuditor);
    5658public:
    57     explicit XSSAuditor(HTMLDocumentParser*);
     59    XSSAuditor();
    5860
    5961    void init(Document*);
     
    7577
    7678    bool filterStartToken(const FilterTokenRequest&);
    77     void filterEndToken(HTMLToken&);
     79    void filterEndToken(const FilterTokenRequest&);
    7880    bool filterCharacterToken(const FilterTokenRequest&);
    7981    bool filterScriptToken(const FilterTokenRequest&);
     
    98100    bool isLikelySafeResource(const String& url);
    99101
    100     // FIXME: Remove this dependency.
    101     HTMLDocumentParser* m_parser;
    102102    KURL m_documentURL;
    103103    bool m_isEnabled;
     
    112112    State m_state;
    113113    String m_cachedDecodedSnippet;
    114     bool m_shouldAllowCDATA;
    115114    unsigned m_scriptTagNestingLevel;
    116115    KURL m_reportURL;
Note: See TracChangeset for help on using the changeset viewer.