Bug 1376651 - Pass the nsIScriptElement instead of allocating a string every time r?ckerschb draft
authorKate McKinley <kmckinley@mozilla.com>
Thu, 27 Jul 2017 11:01:24 -0700
changeset 621358 617d328905e1168922b7f12265e01e1c2607ee30
parent 621357 5f31bb248554c4fc6ba9465647368c9f3dbe5391
child 640982 29c9a48bf21b711eeb6982735ce5eba1ff32dde3
push id72344
push userbmo:kmckinley@mozilla.com
push dateFri, 04 Aug 2017 18:09:07 +0000
reviewersckerschb
bugs1376651
milestone57.0a1
Bug 1376651 - Pass the nsIScriptElement instead of allocating a string every time r?ckerschb Change the interface to GetAlowsInline to take an nsISupports* instead of a string, and pass the nsIScriptElement directly. If we don't have an element, then pass nullptr or the mock string created as an nsISupportsString. MozReview-Commit-ID: pgIMxtplsi
dom/base/nsDocument.cpp
dom/events/EventListenerManager.cpp
dom/interfaces/security/nsIContentSecurityPolicy.idl
dom/jsurl/nsJSProtocolHandler.cpp
dom/script/ScriptLoader.cpp
dom/security/nsCSPContext.cpp
dom/security/test/unit/test_csp_reports.js
layout/style/nsStyleUtil.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -12681,17 +12681,17 @@ nsIDocument::InlineScriptAllowedByCSP()
   nsCOMPtr<nsIContentSecurityPolicy> csp;
   nsresult rv = NodePrincipal()->GetCsp(getter_AddRefs(csp));
   NS_ENSURE_SUCCESS(rv, true);
   bool allowsInlineScript = true;
   if (csp) {
     nsresult rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
                                        EmptyString(), // aNonce
                                        true,          // aParserCreated
-                                       EmptyString(), // FIXME get script sample (bug 1314567)
+                                       nullptr, // FIXME get script sample (bug 1314567)
                                        0,             // aLineNumber
                                        &allowsInlineScript);
     NS_ENSURE_SUCCESS(rv, true);
   }
   return allowsInlineScript;
 }
 
 static bool
--- a/dom/events/EventListenerManager.cpp
+++ b/dom/events/EventListenerManager.cpp
@@ -41,16 +41,17 @@
 #include "nsError.h"
 #include "nsGkAtoms.h"
 #include "nsIContent.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsIDocument.h"
 #include "nsIDOMEventListener.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsISupports.h"
+#include "nsISupportsPrimitives.h"
 #include "nsIXPConnect.h"
 #include "nsJSUtils.h"
 #include "nsNameSpaceManager.h"
 #include "nsPIDOMWindow.h"
 #include "nsSandboxFlags.h"
 #include "xpcpublic.h"
 #include "nsIFrame.h"
 #include "nsDisplayList.h"
@@ -871,22 +872,26 @@ EventListenerManager::SetEventHandler(ns
       if (domNode) {
         domNode->GetNodeName(tagName);
       }
       // build a "script sample" based on what we know about this element
       scriptSample.Assign(attr);
       scriptSample.AppendLiteral(" attribute on ");
       scriptSample.Append(tagName);
       scriptSample.AppendLiteral(" element");
+      nsCOMPtr<nsISupportsString> sampleIString(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));
+      if (sampleIString) {
+        sampleIString->SetData(scriptSample);
+      }
 
       bool allowsInlineScript = true;
       rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
                                 EmptyString(), // aNonce
                                 true, // aParserCreated (true because attribute event handler)
-                                scriptSample,
+                                sampleIString,
                                 0,             // aLineNumber
                                 &allowsInlineScript);
       NS_ENSURE_SUCCESS(rv, rv);
 
       // return early if CSP wants us to block inline scripts
       if (!allowsInlineScript) {
         return NS_OK;
       }
--- a/dom/interfaces/security/nsIContentSecurityPolicy.idl
+++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl
@@ -6,16 +6,17 @@
 #include "nsIContentPolicy.idl"
 
 interface nsIURI;
 interface nsIChannel;
 interface nsIDocShell;
 interface nsIDOMDocument;
 interface nsIEventTarget;
 interface nsIPrincipal;
+interface nsIScriptElement;
 interface nsIURI;
 
 /**
  * nsIContentSecurityPolicy
  * Describes an XPCOM component used to model and enforce CSPs.  Instances of
  * this class may have multiple policies within them, but there should only be
  * one of these per document/principal.
  */
@@ -133,28 +134,29 @@ interface nsIContentSecurityPolicy : nsI
                     in boolean reportOnly,
                     in boolean deliveredViaMetaTag);
 
   /*
    * Whether this policy allows inline script or style.
    * @param aContentPolicyType Either TYPE_SCRIPT or TYPE_STYLESHEET
    * @param aNonce The nonce string to check against the policy
    * @param aParserCreated If the script element was created by the HTML Parser
-   * @param aContent  The content of the inline resource to hash
+   * @param aElementOrContent The script element of the inline resource to hash
+   *        or the content of the psuedo-script to compare to hash
    *        (and compare to the hashes listed in the policy)
    * @param aLineNumber The line number of the inline resource
    *        (used for reporting)
    * @return
    *     Whether or not the effects of the inline style should be allowed
    *     (block the rules if false).
    */
   boolean getAllowsInline(in nsContentPolicyType aContentPolicyType,
                           in AString aNonce,
                           in boolean aParserCreated,
-                          in AString aContent,
+                          in nsISupports aElementOrContent,
                           in unsigned long aLineNumber);
 
   /**
    * whether this policy allows eval and eval-like functions
    * such as setTimeout("code string", time).
    * @param shouldReportViolations
    *     Whether or not the use of eval should be reported.
    *     This function returns "true" when violating report-only policies, but
--- a/dom/jsurl/nsJSProtocolHandler.cpp
+++ b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -177,17 +177,17 @@ nsresult nsJSThunk::EvaluateScript(nsICh
     nsCOMPtr<nsIContentSecurityPolicy> csp;
     rv = principal->GetCsp(getter_AddRefs(csp));
     NS_ENSURE_SUCCESS(rv, rv);
     if (csp) {
         bool allowsInlineScript = true;
         rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
                                   EmptyString(), // aNonce
                                   true,         // aParserCreated
-                                  EmptyString(), // aContent
+                                  nullptr, // aContent
                                   0,             // aLineNumber
                                   &allowsInlineScript);
 
         //return early if inline scripts are not allowed
         if (!allowsInlineScript) {
           return NS_ERROR_DOM_RETVAL_UNDEFINED;
         }
     }
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -1105,23 +1105,19 @@ CSPAllowsInlineScript(nsIScriptElement* 
   }
 
   // query the nonce
   nsCOMPtr<nsIContent> scriptContent = do_QueryInterface(aElement);
   nsAutoString nonce;
   scriptContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce);
   bool parserCreated = aElement->GetParserCreated() != mozilla::dom::NOT_FROM_PARSER;
 
-  // query the scripttext
-  nsAutoString scriptText;
-  aElement->GetScriptText(scriptText);
-
   bool allowInlineScript = false;
   rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
-                            nonce, parserCreated, scriptText,
+                            nonce, parserCreated, aElement,
                             aElement->GetScriptLineNumber(),
                             &allowInlineScript);
   return allowInlineScript;
 }
 
 ScriptLoadRequest*
 ScriptLoader::CreateLoadRequest(ScriptKind aKind,
                                 nsIScriptElement* aElement,
--- a/dom/security/nsCSPContext.cpp
+++ b/dom/security/nsCSPContext.cpp
@@ -21,16 +21,17 @@
 #include "nsIHttpChannel.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsIObserver.h"
 #include "nsIObserverService.h"
 #include "nsIStringStream.h"
+#include "nsISupportsPrimitives.h"
 #include "nsIUploadChannel.h"
 #include "nsIScriptError.h"
 #include "nsIWebNavigation.h"
 #include "nsMimeTypes.h"
 #include "nsNetUtil.h"
 #include "nsIContentPolicy.h"
 #include "nsSupportsPrimitives.h"
 #include "nsThreadUtils.h"
@@ -500,49 +501,73 @@ nsCSPContext::reportInlineViolation(nsCo
                        codeSample,                         // aScriptSample
                        aLineNumber);                       // aLineNum
 }
 
 NS_IMETHODIMP
 nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType,
                               const nsAString& aNonce,
                               bool aParserCreated,
-                              const nsAString& aContent,
+                              nsISupports* aElementOrContent,
                               uint32_t aLineNumber,
                               bool* outAllowsInline)
 {
   *outAllowsInline = true;
 
   MOZ_ASSERT(aContentType == nsContentUtils::InternalContentPolicyTypeToExternal(aContentType),
              "We should only see external content policy types here.");
 
   if (aContentType != nsIContentPolicy::TYPE_SCRIPT &&
       aContentType != nsIContentPolicy::TYPE_STYLESHEET) {
     MOZ_ASSERT(false, "can only allow inline for script or style");
     return NS_OK;
   }
 
+   nsAutoString content(EmptyString());
+
   // always iterate all policies, otherwise we might not send out all reports
   for (uint32_t i = 0; i < mPolicies.Length(); i++) {
     bool allowed =
       mPolicies[i]->allows(aContentType, CSP_UNSAFE_INLINE, EmptyString(), aParserCreated) ||
-      mPolicies[i]->allows(aContentType, CSP_NONCE, aNonce, aParserCreated) ||
-      mPolicies[i]->allows(aContentType, CSP_HASH, aContent, aParserCreated);
+      mPolicies[i]->allows(aContentType, CSP_NONCE, aNonce, aParserCreated);
+
+    // If the inlined script or style is allowed by either unsafe-inline or the
+    // nonce, go ahead and shortcut this loop so we can avoid allocating
+    // unecessary strings
+    if (allowed) {
+      continue;
+    }
+
+    // Check the content length to ensure the content is not allocated more than
+    // once. Even though we are in a for loop, it is probable that there is only one
+    // policy, so this check may be unnecessary.
+    if (content.Length() == 0) {
+      // postpone the allocation until absolutely necessary.
+      nsCOMPtr<nsISupportsString> stringContent = do_QueryInterface(aElementOrContent);
+      nsCOMPtr<nsIScriptElement> element = do_QueryInterface(aElementOrContent);
+      if (stringContent) {
+        Unused << stringContent->GetData(content);
+      } else if (element) {
+        element->GetScriptText(content);
+      }
+    }
+
+    allowed = mPolicies[i]->allows(aContentType, CSP_HASH, content, aParserCreated);
 
     if (!allowed) {
       // policy is violoated: deny the load unless policy is report only and
       // report the violation.
       if (!mPolicies[i]->getReportOnlyFlag()) {
         *outAllowsInline = false;
       }
       nsAutoString violatedDirective;
       mPolicies[i]->getDirectiveStringForContentType(aContentType, violatedDirective);
       reportInlineViolation(aContentType,
                             aNonce,
-                            aContent,
+                            content,
                             violatedDirective,
                             i,
                             aLineNumber);
     }
   }
   return NS_OK;
 }
 
--- a/dom/security/test/unit/test_csp_reports.js
+++ b/dom/security/test/unit/test_csp_reports.js
@@ -102,24 +102,27 @@ function makeTest(id, expectedJSON, useR
   callback(csp);
 }
 
 function run_test() {
   var selfuri = NetUtil.newURI(REPORT_SERVER_URI +
                                ":" + REPORT_SERVER_PORT +
                                "/foo/self");
 
+  let content = Cc["@mozilla.org/supports-string;1"].
+                   createInstance(Ci.nsISupportsString);
+  content.data = "";
   // test that inline script violations cause a report.
   makeTest(0, {"blocked-uri": "self"}, false,
       function(csp) {
         let inlineOK = true;
         inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
                                        "", // aNonce
                                        false, // aParserCreated
-                                       "", // aContent
+                                       content, // aContent
                                        0); // aLineNumber
 
         // this is not a report only policy, so it better block inline scripts
         do_check_false(inlineOK);
       });
 
   // test that eval violations cause a report.
   makeTest(1, {"blocked-uri": "self",
@@ -153,20 +156,23 @@ function run_test() {
                       NetUtil.newURI("http://blocked.test/foo.js"),
                       null, null, null, null);
       });
 
   // test that inline script violations cause a report in report-only policy
   makeTest(3, {"blocked-uri": "self"}, true,
       function(csp) {
         let inlineOK = true;
+        let content = Cc["@mozilla.org/supports-string;1"].
+                         createInstance(Ci.nsISupportsString);
+        content.data = "";
         inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
                                        "", // aNonce
                                        false, // aParserCreated
-                                       "", // aContent
+                                       content, // aContent
                                        0); // aLineNumber
 
         // this is a report only policy, so it better allow inline scripts
         do_check_true(inlineOK);
       });
 
   // test that eval violations cause a report in report-only policy
   makeTest(4, {"blocked-uri": "self"}, true,
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -10,16 +10,17 @@
 #include "nsCSSProps.h"
 #include "nsContentUtils.h"
 #include "nsRuleNode.h"
 #include "nsROCSSPrimitiveValue.h"
 #include "nsStyleStruct.h"
 #include "nsIContentPolicy.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsIURI.h"
+#include "nsISupportsPrimitives.h"
 #include "nsPrintfCString.h"
 #include <cctype>
 
 using namespace mozilla;
 
 //------------------------------------------------------------------------------
 // Font Algorithm Code
 //------------------------------------------------------------------------------
@@ -844,18 +845,23 @@ nsStyleUtil::CSPAllowsInlineStyle(nsICon
   }
 
   // query the nonce
   nsAutoString nonce;
   if (aContent) {
     aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce);
   }
 
+  nsCOMPtr<nsISupportsString> styleText(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));
+  if (styleText) {
+    styleText->SetData(aStyleText);
+  }
+
   bool allowInlineStyle = true;
   rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_STYLESHEET,
                             nonce,
                             false, // aParserCreated only applies to scripts
-                            aStyleText, aLineNumber,
+                            styleText, aLineNumber,
                             &allowInlineStyle);
   NS_ENSURE_SUCCESS(rv, false);
 
   return allowInlineStyle;
 }