Changeset 145331 in webkit

Timestamp:

Mar 10, 2013 12:57:08 PM (11 years ago)

Author:

mkwst@chromium.org

Message:

XSSAuditor doesn't need a copy of the original document URL.
​https://bugs.webkit.org/show_bug.cgi?id=111944

Reviewed by Adam Barth.

When creating an XSSInfo object in response to detecting reflected XSS
on a page, the Auditor was passing in a copy of the document's
original URL for reporting. It doesn't look like we need this, as
XSSInfo's only consumer, XSSAuditorDelegate, runs on the main thread
with access to the document. We can obtain access to the same
information by reading the URL directly from the delegate's Document
object if and when we need it.

• html/parser/XSSAuditorDelegate.cpp:

(WebCore::XSSAuditorDelegate::didBlockScript):

Read the document's URL directly in order to create a violation
report.

(WebCore::XSSInfo::isSafeToSendToAnotherThread):

• html/parser/XSSAuditorDelegate.h:

(WebCore::XSSInfo::create):
(WebCore::XSSInfo::XSSInfo):

• html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::init):
(WebCore::XSSAuditor::filterToken):
(WebCore::XSSAuditor::isSafeToSendToAnotherThread):

• html/parser/XSSAuditor.h:

Remove the copied original URL from both XSSInfo objects and the
XSSAuditor.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/XSSAuditor.cpp (modified) (3 diffs)
• html/parser/XSSAuditor.h (modified) (1 diff)
• html/parser/XSSAuditorDelegate.cpp (modified) (2 diffs)
• html/parser/XSSAuditorDelegate.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-03-10  Mike West  <mkwst@chromium.org>
+
+        XSSAuditor doesn't need a copy of the original document URL.
+        https://bugs.webkit.org/show_bug.cgi?id=111944
+
+        Reviewed by Adam Barth.
+
+        When creating an XSSInfo object in response to detecting reflected XSS
+        on a page, the Auditor was passing in a copy of the document's
+        original URL for reporting. It doesn't look like we need this, as
+        XSSInfo's only consumer, XSSAuditorDelegate, runs on the main thread
+        with access to the document. We can obtain access to the same
+        information by reading the URL directly from the delegate's Document
+        object if and when we need it.
+
+        * html/parser/XSSAuditorDelegate.cpp:
+        (WebCore::XSSAuditorDelegate::didBlockScript):
+            Read the document's URL directly in order to create a violation
+            report.
+        (WebCore::XSSInfo::isSafeToSendToAnotherThread):
+        * html/parser/XSSAuditorDelegate.h:
+        (WebCore::XSSInfo::create):
+        (WebCore::XSSInfo::XSSInfo):
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::init):
+        (WebCore::XSSAuditor::filterToken):
+        (WebCore::XSSAuditor::isSafeToSendToAnotherThread):
+        * html/parser/XSSAuditor.h:
+            Remove the copied original URL from both XSSInfo objects and the
+            XSSAuditor.
+
 2013-03-10  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/html/parser/XSSAuditor.cpp

@@ -313 +313 @@
     }
 
-    if (!m_reportURL.isEmpty()) {
-        // May need these for reporting later on.
-        m_originalURL = m_documentURL.string().isolatedCopy();
+    if (!m_reportURL.isEmpty())
         m_originalHTTPBody = httpBodyAsString;
-    }
 }
 
@@ -338 +335 @@
     if (didBlockScript) {
         bool didBlockEntirePage = (m_xssProtection == ContentSecurityPolicy::BlockReflectedXSS);
-        OwnPtr<XSSInfo> xssInfo = XSSInfo::create(m_reportURL, m_originalURL, m_originalHTTPBody, didBlockEntirePage);
+        OwnPtr<XSSInfo> xssInfo = XSSInfo::create(m_reportURL, m_originalHTTPBody, didBlockEntirePage);
         if (!m_reportURL.isEmpty()) {
             m_reportURL = KURL();
-            m_originalURL = String();
             m_originalHTTPBody = String();
         }
@@ -732 +728 @@
 {
     return m_documentURL.isSafeToSendToAnotherThread()
-        && m_originalURL.isSafeToSendToAnotherThread()
         && m_originalHTTPBody.isSafeToSendToAnotherThread()
         && m_decodedURL.isSafeToSendToAnotherThread()

trunk/Source/WebCore/html/parser/XSSAuditor.h

@@ -106 +106 @@
     ContentSecurityPolicy::ReflectedXSSDisposition m_xssProtection;
 
-    String m_originalURL;
     String m_originalHTTPBody;
     String m_decodedURL;

trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp

@@ -44 +44 @@
 {
     return m_reportURL.isSafeToSendToAnotherThread()
-        && m_originalURL.isSafeToSendToAnotherThread()
         && m_originalHTTPBody.isSafeToSendToAnotherThread();
 }
@@ -74 +73 @@
     if (!xssInfo.m_reportURL.isEmpty()) {
         RefPtr<InspectorObject> reportDetails = InspectorObject::create();
-        reportDetails->setString("request-url", xssInfo.m_originalURL);
+        reportDetails->setString("request-url", m_document->url().string());
         reportDetails->setString("request-body", xssInfo.m_originalHTTPBody);
 

trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h

@@ -40 +40 @@
 class XSSInfo {
 public:
-    static PassOwnPtr<XSSInfo> create(const KURL& reportURL, const String& originalURL, const String& originalHTTPBody, bool didBlockEntirePage)
+    static PassOwnPtr<XSSInfo> create(const KURL& reportURL, const String& originalHTTPBody, bool didBlockEntirePage)
     {
-        return adoptPtr(new XSSInfo(reportURL, originalURL, originalHTTPBody, didBlockEntirePage));
+        return adoptPtr(new XSSInfo(reportURL, originalHTTPBody, didBlockEntirePage));
     }
 
@@ -48 +48 @@
 
     KURL m_reportURL;
-    String m_originalURL;
     String m_originalHTTPBody;
     bool m_didBlockEntirePage;
@@ -54 +53 @@
 
 private:
-    XSSInfo(const KURL& reportURL, const String& originalURL, const String& originalHTTPBody, bool didBlockEntirePage)
+    XSSInfo(const KURL& reportURL, const String& originalHTTPBody, bool didBlockEntirePage)
         : m_reportURL(reportURL)
-        , m_originalURL(originalURL)
         , m_originalHTTPBody(originalHTTPBody)
         , m_didBlockEntirePage(didBlockEntirePage)