- Revision
- 145348
- Author
- [email protected]
- Date
- 2013-03-11 02:48:14 -0700 (Mon, 11 Mar 2013)
Log Message
XSSAuditor doesn't need a copy of the original document's body.
https://bugs.webkit.org/show_bug.cgi?id=111946
Reviewed by Darin Adler.
The XSSAuditor currently copies the original HTTP body of the document
that's being audited in order to include it into a violation report if
reflected XSS is detected. We don't actually need to do this, as we
have access to the original request information from inside the
XSSAuditorDelegate where the report is generated.
XSSAuditorDelegate::didBlockScript ASSERTs that it's running on the
main thread, so it should be safe to reach through the document's
loader to get that information directly, rather than passing it from
thread to thread via XSSInfo object properties.
* html/parser/XSSAuditor.h:
* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::init):
(WebCore::XSSAuditor::filterToken):
(WebCore::XSSAuditor::isSafeToSendToAnotherThread):
* html/parser/XSSAuditorDelegate.h:
(WebCore::XSSInfo::create):
(WebCore::XSSInfo::XSSInfo):
* html/parser/XSSAuditorDelegate.cpp:
(WebCore::XSSInfo::isSafeToSendToAnotherThread):
Drop the XSSInfo and XSSAuditor properties that held an
isolatedCopy of the the original HTTP body. Depending on the
document's size, this could be a significant savings.
(WebCore::XSSAuditorDelegate::didBlockScript):
Reach into the document's loader's original request in order to
grab the body as a String, and feed that into the violation report
object.
As a drive-by, this patch creates a FrameLoader* temporary
variable to minimize repetition in this area of the code. We use
the loader a few times, but should only have to grab it once.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (145347 => 145348)
--- trunk/Source/WebCore/ChangeLog 2013-03-11 09:40:08 UTC (rev 145347)
+++ trunk/Source/WebCore/ChangeLog 2013-03-11 09:48:14 UTC (rev 145348)
@@ -1,3 +1,42 @@
+2013-03-11 Mike West <[email protected]>
+
+ XSSAuditor doesn't need a copy of the original document's body.
+ https://bugs.webkit.org/show_bug.cgi?id=111946
+
+ Reviewed by Darin Adler.
+
+ The XSSAuditor currently copies the original HTTP body of the document
+ that's being audited in order to include it into a violation report if
+ reflected XSS is detected. We don't actually need to do this, as we
+ have access to the original request information from inside the
+ XSSAuditorDelegate where the report is generated.
+ XSSAuditorDelegate::didBlockScript ASSERTs that it's running on the
+ main thread, so it should be safe to reach through the document's
+ loader to get that information directly, rather than passing it from
+ thread to thread via XSSInfo object properties.
+
+ * html/parser/XSSAuditor.h:
+ * html/parser/XSSAuditor.cpp:
+ (WebCore::XSSAuditor::init):
+ (WebCore::XSSAuditor::filterToken):
+ (WebCore::XSSAuditor::isSafeToSendToAnotherThread):
+ * html/parser/XSSAuditorDelegate.h:
+ (WebCore::XSSInfo::create):
+ (WebCore::XSSInfo::XSSInfo):
+ * html/parser/XSSAuditorDelegate.cpp:
+ (WebCore::XSSInfo::isSafeToSendToAnotherThread):
+ Drop the XSSInfo and XSSAuditor properties that held an
+ isolatedCopy of the the original HTTP body. Depending on the
+ document's size, this could be a significant savings.
+ (WebCore::XSSAuditorDelegate::didBlockScript):
+ Reach into the document's loader's original request in order to
+ grab the body as a String, and feed that into the violation report
+ object.
+
+ As a drive-by, this patch creates a FrameLoader* temporary
+ variable to minimize repetition in this area of the code. We use
+ the loader a few times, but should only have to grab it once.
+
2013-03-11 Silvia Pfeiffer <[email protected]>
[Chromium] REGRESSION: Closed Captions button not showing properly
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (145347 => 145348)
--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2013-03-11 09:40:08 UTC (rev 145347)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2013-03-11 09:48:14 UTC (rev 145348)
@@ -311,9 +311,6 @@
m_isEnabled = false;
return;
}
-
- if (!m_reportURL.isEmpty())
- m_originalHTTPBody = httpBodyAsString;
}
PassOwnPtr<XSSInfo> XSSAuditor::filterToken(const FilterTokenRequest& request)
@@ -334,11 +331,8 @@
if (didBlockScript) {
bool didBlockEntirePage = (m_xssProtection == ContentSecurityPolicy::BlockReflectedXSS);
- OwnPtr<XSSInfo> xssInfo = XSSInfo::create(m_reportURL, m_originalHTTPBody, didBlockEntirePage);
- if (!m_reportURL.isEmpty()) {
- m_reportURL = KURL();
- m_originalHTTPBody = String();
- }
+ OwnPtr<XSSInfo> xssInfo = XSSInfo::create(m_reportURL, didBlockEntirePage);
+ m_reportURL = KURL();
return xssInfo.release();
}
return nullptr;
@@ -727,7 +721,6 @@
bool XSSAuditor::isSafeToSendToAnotherThread() const
{
return m_documentURL.isSafeToSendToAnotherThread()
- && m_originalHTTPBody.isSafeToSendToAnotherThread()
&& m_decodedURL.isSafeToSendToAnotherThread()
&& m_decodedHTTPBody.isSafeToSendToAnotherThread()
&& m_cachedDecodedSnippet.isSafeToSendToAnotherThread()
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.h (145347 => 145348)
--- trunk/Source/WebCore/html/parser/XSSAuditor.h 2013-03-11 09:40:08 UTC (rev 145347)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.h 2013-03-11 09:48:14 UTC (rev 145348)
@@ -105,7 +105,6 @@
bool m_isEnabled;
ContentSecurityPolicy::ReflectedXSSDisposition m_xssProtection;
- String m_originalHTTPBody;
String m_decodedURL;
String m_decodedHTTPBody;
OwnPtr<SuffixTree<ASCIICodebook> > m_decodedHTTPBodySuffixTree;
Modified: trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp (145347 => 145348)
--- trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp 2013-03-11 09:40:08 UTC (rev 145347)
+++ trunk/Source/WebCore/html/parser/XSSAuditorDelegate.cpp 2013-03-11 09:48:14 UTC (rev 145348)
@@ -29,6 +29,7 @@
#include "Console.h"
#include "DOMWindow.h"
#include "Document.h"
+#include "DocumentLoader.h"
#include "FormData.h"
#include "Frame.h"
#include "FrameLoader.h"
@@ -42,8 +43,7 @@
bool XSSInfo::isSafeToSendToAnotherThread() const
{
- return m_reportURL.isSafeToSendToAnotherThread()
- && m_originalHTTPBody.isSafeToSendToAnotherThread();
+ return m_reportURL.isSafeToSendToAnotherThread();
}
XSSAuditorDelegate::XSSAuditorDelegate(Document* document)
@@ -62,19 +62,27 @@
DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Refused to execute a _javascript_ script. Source code of script found within request.\n")));
m_document->addConsoleMessage(SecurityMessageSource, ErrorMessageLevel, consoleMessage);
+ FrameLoader* frameLoader = m_document->frame()->loader();
+
if (xssInfo.m_didBlockEntirePage)
- m_document->frame()->loader()->stopAllLoaders();
+ frameLoader->stopAllLoaders();
if (!m_didNotifyClient) {
- m_document->frame()->loader()->client()->didDetectXSS(m_document->url(), xssInfo.m_didBlockEntirePage);
+ frameLoader->client()->didDetectXSS(m_document->url(), xssInfo.m_didBlockEntirePage);
m_didNotifyClient = true;
}
if (!xssInfo.m_reportURL.isEmpty()) {
RefPtr<InspectorObject> reportDetails = InspectorObject::create();
reportDetails->setString("request-url", m_document->url().string());
- reportDetails->setString("request-body", xssInfo.m_originalHTTPBody);
+ String httpBody;
+ if (frameLoader->documentLoader()) {
+ if (FormData* formData = frameLoader->documentLoader()->originalRequest().httpBody())
+ httpBody = formData->flattenToString();
+ }
+ reportDetails->setString("request-body", httpBody);
+
RefPtr<InspectorObject> reportObject = InspectorObject::create();
reportObject->setObject("xss-report", reportDetails.release());
Modified: trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h (145347 => 145348)
--- trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h 2013-03-11 09:40:08 UTC (rev 145347)
+++ trunk/Source/WebCore/html/parser/XSSAuditorDelegate.h 2013-03-11 09:48:14 UTC (rev 145348)
@@ -39,22 +39,20 @@
class XSSInfo {
public:
- static PassOwnPtr<XSSInfo> create(const KURL& reportURL, const String& originalHTTPBody, bool didBlockEntirePage)
+ static PassOwnPtr<XSSInfo> create(const KURL& reportURL, bool didBlockEntirePage)
{
- return adoptPtr(new XSSInfo(reportURL, originalHTTPBody, didBlockEntirePage));
+ return adoptPtr(new XSSInfo(reportURL, didBlockEntirePage));
}
bool isSafeToSendToAnotherThread() const;
KURL m_reportURL;
- String m_originalHTTPBody;
bool m_didBlockEntirePage;
TextPosition m_textPosition;
private:
- XSSInfo(const KURL& reportURL, const String& originalHTTPBody, bool didBlockEntirePage)
+ XSSInfo(const KURL& reportURL, bool didBlockEntirePage)
: m_reportURL(reportURL)
- , m_originalHTTPBody(originalHTTPBody)
, m_didBlockEntirePage(didBlockEntirePage)
{ }
};
