CSP: Deduplicate violation reports before sending.

Source/core/page/ContentSecurityPolicy.cpp

 /*
  * Copyright (C) 2011 Google, Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1705 matching lines @@
     const ScriptCallFrame& callFrame = stack->at(0);
 
     if (callFrame.lineNumber()) {
         KURL source = KURL(ParsedURLString, callFrame.sourceURL());
         init.sourceFile = stripURLForUseInReport(document, source);
         init.lineNumber = callFrame.lineNumber();
         init.columnNumber = callFrame.columnNumber();
     }
 }
 
-void ContentSecurityPolicy::reportViolation(const String& directiveText, const String& effectiveDirective, const String& consoleMessage, const KURL& blockedURL, const Vector<KURL>& reportURIs, const String& header, const String& contextURL, const WTF::OrdinalNumber& contextLine, ScriptState* state) const
+void ContentSecurityPolicy::reportViolation(const String& directiveText, const String& effectiveDirective, const String& consoleMessage, const KURL& blockedURL, const Vector<KURL>& reportURIs, const String& header, const String& contextURL, const WTF::OrdinalNumber& contextLine, ScriptState* state)
 {
     logToConsole(consoleMessage, contextURL, contextLine, state);
 
     // FIXME: Support sending reports from worker.
     if (!m_scriptExecutionContext->isDocument())
         return;
 
     Document* document = toDocument(m_scriptExecutionContext);
     Frame* frame = document->frame();
     if (!frame)
@@ skipping 29 matching lines @@
     if (!violationData.sourceFile.isEmpty() && violationData.lineNumber) {
         cspReport->setString("source-file", violationData.sourceFile);
         cspReport->setNumber("line-number", violationData.lineNumber);
         cspReport->setNumber("column-number", violationData.columnNumber);
     }
     cspReport->setNumber("status-code", violationData.statusCode);
 
     RefPtr<JSONObject> reportObject = JSONObject::create();
     reportObject->setObject("csp-report", cspReport.release());
 
+    if (!shouldSendViolationReport(reportObject))
+        return;

[Tom Sepez, 2013/08/02 18:03:22]

Maybe count number of times we are suppressed here, with console.log() on n == 1
with a message along the lines of "Duplicate violation reports being
suppressed".  That might solve the test case implementation without being too
noisy on the developers looking at the console (though slightly noisy and the
purists will still be annoyed). 

[Mike West, 2013/08/05 08:21:29]

I think I'd be annoyed. :)

What do you think about adding a "didDispatchPingLoader" callback to the
FrameLoaderClient and piping it down into the test results? That's a bunch of
busywork, but it would give us something visible in the test expectations
without writing warnings to the console that developers won't reasonably be
expected to do anything with.

+
     RefPtr<FormData> report = FormData::create(reportObject->toJSONString().utf8());

[abarth-chromium, 2013/08/02 18:04:28]

Rather than calling toJSONString() three times (once here, in
shouldSendViolationReport, and in didSendViolationReport), we should create the
string once around line 1775 and use it in all three places.

[Mike West, 2013/08/05 08:21:29]

Done.

 
     for (size_t i = 0; i < reportURIs.size(); ++i)
         PingLoader::sendViolationReport(frame, reportURIs[i], report, PingLoader::ContentSecurityPolicyViolationReport);
+
+    didSendViolationReport(reportObject);

[Tom Sepez, 2013/08/02 18:03:22]

Seems like a shame to have to hash the same string twice, once in shouldSend()
and again in didSend().

[Mike West, 2013/08/05 08:21:29]

Done.

 }
 
 void ContentSecurityPolicy::reportUnsupportedDirective(const String& name) const
 {
     DEFINE_STATIC_LOCAL(String, allow, (ASCIILiteral("allow")));
     DEFINE_STATIC_LOCAL(String, options, (ASCIILiteral("options")));
     DEFINE_STATIC_LOCAL(String, policyURI, (ASCIILiteral("policy-uri")));
     DEFINE_STATIC_LOCAL(String, allowMessage, (ASCIILiteral("The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.")));
     DEFINE_STATIC_LOCAL(String, optionsMessage, (ASCIILiteral("The 'options' directive has been replaced with 'unsafe-inline' and 'unsafe-eval' source expressions for the 'script-src' and 'style-src' directives. Please use those directives instead, as 'options' has no effect.")));
     DEFINE_STATIC_LOCAL(String, policyURIMessage, (ASCIILiteral("The 'policy-uri' directive has been removed from the specification. Please specify a complete policy via the Content-Security-Policy header.")));
@@ skipping 95 matching lines @@
 bool ContentSecurityPolicy::shouldBypassMainWorld(ScriptExecutionContext* context)
 {
     if (context && context->isDocument()) {
         Document* document = toDocument(context);
         if (document->frame())
             return document->frame()->script()->shouldBypassMainWorldContentSecurityPolicy();
     }
     return false;
 }
 
+bool ContentSecurityPolicy::shouldSendViolationReport(PassRefPtr<JSONObject> report) const

[Tom Sepez, 2013/08/02 18:11:40]

Maybe add a comment that we don't care about collisions to prevent some eager
person down the road from pointing this out in some bug.

[Mike West, 2013/08/05 08:21:29]

Done.

+{
+    return !m_violationReportsSent.contains(report->toJSONString().impl()->hash());
 }
+
+void ContentSecurityPolicy::didSendViolationReport(PassRefPtr<JSONObject> report)
+{
+    m_violationReportsSent.add(report->toJSONString().impl()->hash());
+}
+
+} // namespace WebCore