XSSAuditor doesn't need a copy of the original document URL.
authormkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Mar 2013 19:57:08 +0000 (19:57 +0000)
committermkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Mar 2013 19:57:08 +0000 (19:57 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145331 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/html/parser/XSSAuditor.cpp
Source/WebCore/html/parser/XSSAuditor.h
Source/WebCore/html/parser/XSSAuditorDelegate.cpp
Source/WebCore/html/parser/XSSAuditorDelegate.h

index f7320db..9ac5adc 100644 (file)
@@ -1,3 +1,34 @@
+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>
 
         GlyphMetricsMap should use OwnPtr.
index 1a02644..9f93dec 100644 (file)
@@ -312,11 +312,8 @@ void XSSAuditor::init(Document* document)
         return;
     }
 
-    if (!m_reportURL.isEmpty()) {
-        // May need these for reporting later on.
-        m_originalURL = m_documentURL.string().isolatedCopy();
+    if (!m_reportURL.isEmpty())
         m_originalHTTPBody = httpBodyAsString;
-    }
 }
 
 PassOwnPtr<XSSInfo> XSSAuditor::filterToken(const FilterTokenRequest& request)
@@ -337,10 +334,9 @@ PassOwnPtr<XSSInfo> XSSAuditor::filterToken(const FilterTokenRequest& request)
 
     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();
         }
         return xssInfo.release();
@@ -731,7 +727,6 @@ bool XSSAuditor::isLikelySafeResource(const String& url)
 bool XSSAuditor::isSafeToSendToAnotherThread() const
 {
     return m_documentURL.isSafeToSendToAnotherThread()
-        && m_originalURL.isSafeToSendToAnotherThread()
         && m_originalHTTPBody.isSafeToSendToAnotherThread()
         && m_decodedURL.isSafeToSendToAnotherThread()
         && m_decodedHTTPBody.isSafeToSendToAnotherThread()
index 4cf8301..a66f66e 100644 (file)
@@ -105,7 +105,6 @@ private:
     bool m_isEnabled;
     ContentSecurityPolicy::ReflectedXSSDisposition m_xssProtection;
 
-    String m_originalURL;
     String m_originalHTTPBody;
     String m_decodedURL;
     String m_decodedHTTPBody;
index 4a0fe3d..06f8c6c 100644 (file)
@@ -43,7 +43,6 @@ namespace WebCore {
 bool XSSInfo::isSafeToSendToAnotherThread() const
 {
     return m_reportURL.isSafeToSendToAnotherThread()
-        && m_originalURL.isSafeToSendToAnotherThread()
         && m_originalHTTPBody.isSafeToSendToAnotherThread();
 }
 
@@ -73,7 +72,7 @@ void XSSAuditorDelegate::didBlockScript(const XSSInfo& xssInfo)
 
     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);
 
         RefPtr<InspectorObject> reportObject = InspectorObject::create();
index 7c10bcf..c22b984 100644 (file)
@@ -39,23 +39,21 @@ class Document;
 
 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));
     }
 
     bool isSafeToSendToAnotherThread() const;
 
     KURL m_reportURL;
-    String m_originalURL;
     String m_originalHTTPBody;
     bool m_didBlockEntirePage;
     TextPosition m_textPosition;
 
 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)
     { }