Title: [145695] trunk/Source/WebCore
Revision
145695
Author
mk...@chromium.org
Date
2013-03-13 02:32:25 -0700 (Wed, 13 Mar 2013)

Log Message

Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
https://bugs.webkit.org/show_bug.cgi?id=112179

Reviewed by Adam Barth.

Rather than relying on XSSInfo objects to move the XSSAuditor's report
URL into the XSSAuditorDelegate for reporting, we should be able to grab
the URL directly from XSSAuditor before it moves off the main thread,
and store it on the delegate.

This will enable us to drop the report URL properties from both
XSSAuditor and XSSInfo. Oh, happy day!

* html/parser/BackgroundHTMLParser.cpp:
(WebCore::BackgroundHTMLParser::sendTokensToMainThread):
    We no longer need to check whether XSSInfo objects are thread safe,
    as we've dropped the only problematic bit.
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::pumpTokenizer):
(WebCore::HTMLDocumentParser::startBackgroundParser):
* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::init):
    When initializing the XSSAuditor, pass in an XSSAuditorDelegate*
    and assign the report URL directly onto that object.
(WebCore::XSSAuditor::filterToken):
    Drop the report URL parameter from XSSInfo objects we create in the
    Auditor, as they're now handled directly from the delegate.
(WebCore::XSSAuditor::isSafeToSendToAnotherThread):
    Drop the report URL property from XSSAuditor's threadsafeness check,
    as properties that do not exist are automatically thread-safe.
* html/parser/XSSAuditorDelegate.cpp:
(WebCore::XSSAuditorDelegate::didBlockScript):
    Use the delegate's own report URL rather than the XSSInfo objects'.
* html/parser/XSSAuditorDelegate.h:
(WebCore::XSSInfo::create):
(WebCore::XSSInfo::XSSInfo):
    Drop the report URL property from XSSInfo.
(WebCore::XSSAuditorDelegate::setReportURL):
(XSSAuditorDelegate):
    Provide a public API for setting a delegate's report URL.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (145694 => 145695)


--- trunk/Source/WebCore/ChangeLog	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/ChangeLog	2013-03-13 09:32:25 UTC (rev 145695)
@@ -1,5 +1,48 @@
 2013-03-13  Mike West  <mk...@chromium.org>
 
+        Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
+        https://bugs.webkit.org/show_bug.cgi?id=112179
+
+        Reviewed by Adam Barth.
+
+        Rather than relying on XSSInfo objects to move the XSSAuditor's report
+        URL into the XSSAuditorDelegate for reporting, we should be able to grab
+        the URL directly from XSSAuditor before it moves off the main thread,
+        and store it on the delegate.
+
+        This will enable us to drop the report URL properties from both
+        XSSAuditor and XSSInfo. Oh, happy day!
+
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
+            We no longer need to check whether XSSInfo objects are thread safe,
+            as we've dropped the only problematic bit.
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::pumpTokenizer):
+        (WebCore::HTMLDocumentParser::startBackgroundParser):
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::init):
+            When initializing the XSSAuditor, pass in an XSSAuditorDelegate*
+            and assign the report URL directly onto that object.
+        (WebCore::XSSAuditor::filterToken):
+            Drop the report URL parameter from XSSInfo objects we create in the
+            Auditor, as they're now handled directly from the delegate.
+        (WebCore::XSSAuditor::isSafeToSendToAnotherThread):
+            Drop the report URL property from XSSAuditor's threadsafeness check,
+            as properties that do not exist are automatically thread-safe.
+        * html/parser/XSSAuditorDelegate.cpp:
+        (WebCore::XSSAuditorDelegate::didBlockScript):
+            Use the delegate's own report URL rather than the XSSInfo objects'.
+        * html/parser/XSSAuditorDelegate.h:
+        (WebCore::XSSInfo::create):
+        (WebCore::XSSInfo::XSSInfo):
+            Drop the report URL property from XSSInfo.
+        (WebCore::XSSAuditorDelegate::setReportURL):
+        (XSSAuditorDelegate):
+            Provide a public API for setting a delegate's report URL.
+
+2013-03-13  Mike West  <mk...@chromium.org>
+
         Unsafe _javascript_ attempt errors are ludicrously verbose and annoying
         https://bugs.webkit.org/show_bug.cgi?id=112042
 

Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp (145694 => 145695)


--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp	2013-03-13 09:32:25 UTC (rev 145695)
@@ -53,12 +53,6 @@
         ASSERT(preloads[i]->isSafeToSendToAnotherThread());
 }
 
-static void checkThatXSSInfosAreSafeToSendToAnotherThread(const XSSInfoStream& xssInfos)
-{
-    for (size_t i = 0; i < xssInfos.size(); ++i)
-        ASSERT(xssInfos[i]->isSafeToSendToAnotherThread());
-}
-
 #endif
 
 static const size_t pendingTokenLimit = 1000;
@@ -160,7 +154,6 @@
 #ifndef NDEBUG
     checkThatTokensAreSafeToSendToAnotherThread(m_pendingTokens.get());
     checkThatPreloadsAreSafeToSendToAnotherThread(m_pendingPreloads);
-    checkThatXSSInfosAreSafeToSendToAnotherThread(m_pendingXSSInfos);
 #endif
 
     OwnPtr<HTMLDocumentParser::ParsedChunk> chunk = adoptPtr(new HTMLDocumentParser::ParsedChunk);

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (145694 => 145695)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2013-03-13 09:32:25 UTC (rev 145695)
@@ -521,7 +521,7 @@
     // much we parsed as part of didWriteHTML instead of willWriteHTML.
     InspectorInstrumentationCookie cookie = InspectorInstrumentation::willWriteHTML(document(), m_input.current().currentLine().zeroBasedInt());
 
-    m_xssAuditor.init(document());
+    m_xssAuditor.init(document(), &m_xssAuditorDelegate);
 
     while (canTakeNextToken(mode, session) && !session.needsYield) {
         if (!isParsingFragment())
@@ -662,7 +662,7 @@
     config->options = m_options;
     config->parser = m_weakFactory.createWeakPtr();
     config->xssAuditor = adoptPtr(new XSSAuditor);
-    config->xssAuditor->init(document());
+    config->xssAuditor->init(document(), &m_xssAuditorDelegate);
     config->preloadScanner = adoptPtr(new TokenPreloadScanner(document()->url().copy()));
 
     ASSERT(config->xssAuditor->isSafeToSendToAnotherThread());

Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (145694 => 145695)


--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2013-03-13 09:32:25 UTC (rev 145695)
@@ -51,6 +51,7 @@
 #include "TextEncoding.h"
 #include "TextResourceDecoder.h"
 #include "XLinkNames.h"
+#include "XSSAuditorDelegate.h"
 
 #if ENABLE(SVG)
 #include "SVGNames.h"
@@ -226,7 +227,7 @@
     // we want to reference might not all have been constructed yet.
 }
 
-void XSSAuditor::init(Document* document)
+void XSSAuditor::init(Document* document, XSSAuditorDelegate* auditorDelegate)
 {
     const size_t miniumLengthForSuffixTree = 512; // FIXME: Tune this parameter.
     const int suffixTreeDepth = 5;
@@ -298,7 +299,9 @@
         m_didSendValidCSPHeader = cspHeader != ContentSecurityPolicy::ReflectedXSSUnset && cspHeader != ContentSecurityPolicy::ReflectedXSSInvalid;
 
         m_xssProtection = combineXSSProtectionHeaderAndCSP(xssProtectionHeader, cspHeader);
-        m_reportURL = xssProtectionReportURL; // FIXME: Combine the two report URLs in some reasonable way.
+        // FIXME: Combine the two report URLs in some reasonable way.
+        if (auditorDelegate)
+            auditorDelegate->setReportURL(xssProtectionReportURL.copy());
         FormData* httpBody = documentLoader->originalRequest().httpBody();
         if (httpBody && !httpBody->isEmpty()) {
             httpBodyAsString = httpBody->flattenToString();
@@ -336,8 +339,7 @@
 
     if (didBlockScript) {
         bool didBlockEntirePage = (m_xssProtection == ContentSecurityPolicy::BlockReflectedXSS);
-        OwnPtr<XSSInfo> xssInfo = XSSInfo::create(m_reportURL, didBlockEntirePage, m_didSendValidXSSProtectionHeader, m_didSendValidCSPHeader);
-        m_reportURL = KURL();
+        OwnPtr<XSSInfo> xssInfo = XSSInfo::create(didBlockEntirePage, m_didSendValidXSSProtectionHeader, m_didSendValidCSPHeader);
         return xssInfo.release();
     }
     return nullptr;
@@ -728,8 +730,7 @@
     return m_documentURL.isSafeToSendToAnotherThread()
         && m_decodedURL.isSafeToSendToAnotherThread()
         && m_decodedHTTPBody.isSafeToSendToAnotherThread()
-        && m_cachedDecodedSnippet.isSafeToSendToAnotherThread()
-        && m_reportURL.isSafeToSendToAnotherThread();
+        && m_cachedDecodedSnippet.isSafeToSendToAnotherThread();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/html/parser/XSSAuditor.h (145694 => 145695)


--- trunk/Source/WebCore/html/parser/XSSAuditor.h	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.h	2013-03-13 09:32:25 UTC (rev 145695)
@@ -39,6 +39,7 @@
 class HTMLDocumentParser;
 class HTMLSourceTracker;
 class XSSInfo;
+class XSSAuditorDelegate;
 
 struct FilterTokenRequest {
     FilterTokenRequest(HTMLToken& token, HTMLSourceTracker& sourceTracker, bool shouldAllowCDATA)
@@ -57,7 +58,7 @@
 public:
     XSSAuditor();
 
-    void init(Document*);
+    void init(Document*, XSSAuditorDelegate*);
     PassOwnPtr<XSSInfo> filterToken(const FilterTokenRequest&);
     bool isSafeToSendToAnotherThread() const;
 
@@ -115,7 +116,6 @@
     State m_state;
     String m_cachedDecodedSnippet;
     unsigned m_scriptTagNestingLevel;
-    KURL m_reportURL;
     TextEncoding m_encoding;
 };
 

Modified: trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp (145694 => 145695)


--- trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp	2013-03-13 09:32:25 UTC (rev 145695)
@@ -41,11 +41,6 @@
 
 namespace WebCore {
 
-bool XSSInfo::isSafeToSendToAnotherThread() const
-{
-    return m_reportURL.isSafeToSendToAnotherThread();
-}
-
 XSSAuditorDelegate::XSSAuditorDelegate(Document* document)
     : m_document(document)
     , m_didNotifyClient(false)
@@ -91,7 +86,7 @@
         m_didNotifyClient = true;
     }
 
-    if (!xssInfo.m_reportURL.isEmpty()) {
+    if (!m_reportURL.isEmpty()) {
         RefPtr<InspectorObject> reportDetails = InspectorObject::create();
         reportDetails->setString("request-url", m_document->url().string());
 
@@ -106,7 +101,7 @@
         reportObject->setObject("xss-report", reportDetails.release());
 
         RefPtr<FormData> report = FormData::create(reportObject->toJSONString().utf8().data());
-        PingLoader::sendViolationReport(m_document->frame(), xssInfo.m_reportURL, report);
+        PingLoader::sendViolationReport(m_document->frame(), m_reportURL, report);
     }
 
     if (xssInfo.m_didBlockEntirePage)

Modified: trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h (145694 => 145695)


--- trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h	2013-03-13 09:26:39 UTC (rev 145694)
+++ trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h	2013-03-13 09:32:25 UTC (rev 145695)
@@ -39,23 +39,19 @@
 
 class XSSInfo {
 public:
-    static PassOwnPtr<XSSInfo> create(const KURL& reportURL, bool didBlockEntirePage, bool didSendXSSProtectionHeader, bool didSendCSPHeader)
+    static PassOwnPtr<XSSInfo> create(bool didBlockEntirePage, bool didSendXSSProtectionHeader, bool didSendCSPHeader)
     {
-        return adoptPtr(new XSSInfo(reportURL, didBlockEntirePage, didSendXSSProtectionHeader, didSendCSPHeader));
+        return adoptPtr(new XSSInfo(didBlockEntirePage, didSendXSSProtectionHeader, didSendCSPHeader));
     }
 
-    bool isSafeToSendToAnotherThread() const;
-
-    KURL m_reportURL;
     bool m_didBlockEntirePage;
     bool m_didSendXSSProtectionHeader;
     bool m_didSendCSPHeader;
     TextPosition m_textPosition;
 
 private:
-    XSSInfo(const KURL& reportURL, bool didBlockEntirePage, bool didSendXSSProtectionHeader, bool didSendCSPHeader)
-        : m_reportURL(reportURL)
-        , m_didBlockEntirePage(didBlockEntirePage)
+    XSSInfo(bool didBlockEntirePage, bool didSendXSSProtectionHeader, bool didSendCSPHeader)
+        : m_didBlockEntirePage(didBlockEntirePage)
         , m_didSendXSSProtectionHeader(didSendXSSProtectionHeader)
         , m_didSendCSPHeader(didSendCSPHeader)
     { }
@@ -67,10 +63,12 @@
     explicit XSSAuditorDelegate(Document*);
 
     void didBlockScript(const XSSInfo&);
+    void setReportURL(const KURL& url) { m_reportURL = url; }
 
 private:
     Document* m_document;
     bool m_didNotifyClient;
+    KURL m_reportURL;
 };
 
 typedef Vector<OwnPtr<XSSInfo> > XSSInfoStream;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to