Bug 1301251 - Handle GetSpec() failure in nsContentUtils::GetWrapperSafeScriptFilename(), r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 19 Sep 2016 15:50:22 +0200
changeset 314392 553eb2fc3a71308cca3b860aef1ff0753cb518f7
parent 314391 40cb45881d370984a49a54cfb5e272d1730d69ac
child 314393 951db29640de5d15a089e71116ccf75d0c36889a
push id30725
push userkwierso@gmail.com
push dateMon, 19 Sep 2016 22:51:45 +0000
treeherdermozilla-central@80a9c7007243 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1301251
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1301251 - Handle GetSpec() failure in nsContentUtils::GetWrapperSafeScriptFilename(), r=smaug
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/base/nsScriptLoader.cpp
dom/base/nsScriptLoader.h
dom/xbl/nsXBLService.cpp
dom/xul/nsXULElement.cpp
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -3665,23 +3665,27 @@ nsContentUtils::IsPlainTextType(const ns
   return aContentType.EqualsLiteral(TEXT_PLAIN) ||
          aContentType.EqualsLiteral(TEXT_CSS) ||
          aContentType.EqualsLiteral(TEXT_CACHE_MANIFEST) ||
          aContentType.EqualsLiteral(TEXT_VTT) ||
          IsScriptType(aContentType);
 }
 
 bool
-nsContentUtils::GetWrapperSafeScriptFilename(nsIDocument *aDocument,
-                                             nsIURI *aURI,
-                                             nsACString& aScriptURI)
-{
+nsContentUtils::GetWrapperSafeScriptFilename(nsIDocument* aDocument,
+                                             nsIURI* aURI,
+                                             nsACString& aScriptURI,
+                                             nsresult* aRv)
+{
+  MOZ_ASSERT(aRv);
   bool scriptFileNameModified = false;
-  // XXX: should handle GetSpec() failure properly. See bug 1301251.
-  Unused << aURI->GetSpec(aScriptURI);
+  *aRv = NS_OK;
+
+  *aRv = aURI->GetSpec(aScriptURI);
+  NS_ENSURE_SUCCESS(*aRv, false);
 
   if (IsChromeDoc(aDocument)) {
     nsCOMPtr<nsIChromeRegistry> chromeReg =
       mozilla::services::GetChromeRegistryService();
 
     if (!chromeReg) {
       // If we're running w/o a chrome registry we won't modify any
       // script file names.
@@ -3699,18 +3703,21 @@ nsContentUtils::GetWrapperSafeScriptFile
     if (docURI && docWrappersEnabled && !uriWrappersEnabled) {
       // aURI is a script from a URL that doesn't get wrapper
       // automation. aDocument is a chrome document that does get
       // wrapper automation. Prepend the chrome document's URI
       // followed by the string " -> " to the URI of the script we're
       // loading here so that script in that URI gets the same wrapper
       // automation that the chrome document expects.
       nsAutoCString spec;
-      // XXX: should handle GetSpec() failure properly. See bug 1301251.
-      Unused << docURI->GetSpec(spec);
+      *aRv = docURI->GetSpec(spec);
+      if (NS_WARN_IF(NS_FAILED(*aRv))) {
+        return false;
+      }
+
       spec.AppendLiteral(" -> ");
       spec.Append(aScriptURI);
 
       aScriptURI = spec;
 
       scriptFileNameModified = true;
     }
   }
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -1013,17 +1013,18 @@ public:
    * security wrapper automation the script file name that's returned
    * will be the spec in aURI, else it will be the spec in aDocument's
    * URI followed by aURI's spec, separated by " -> ". Returns true
    * if the script file name was modified, false if it's aURI's
    * spec.
    */
   static bool GetWrapperSafeScriptFilename(nsIDocument *aDocument,
                                              nsIURI *aURI,
-                                             nsACString& aScriptURI);
+                                             nsACString& aScriptURI,
+                                             nsresult* aRv);
 
 
   /**
    * Returns true if aDocument belongs to a chrome docshell for
    * display purposes.  Returns false for null documents or documents
    * which do not belong to a docshell.
    */
   static bool IsInChromeDocshell(nsIDocument *aDocument);
--- a/dom/base/nsScriptLoader.cpp
+++ b/dom/base/nsScriptLoader.cpp
@@ -799,21 +799,23 @@ nsScriptLoader::CreateModuleScript(nsMod
     if (aRequest->mWasCompiledOMT) {
       module = JS::FinishOffThreadModule(cx, aRequest->mOffThreadToken);
       aRequest->mOffThreadToken = nullptr;
       rv = module ? NS_OK : NS_ERROR_FAILURE;
     } else {
       JS::Rooted<JSObject*> global(cx, globalObject->GetGlobalJSObject());
 
       JS::CompileOptions options(cx);
-      FillCompileOptionsForRequest(aes, aRequest, global, &options);
-
-      nsAutoString inlineData;
-      SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
-      rv = nsJSUtils::CompileModule(cx, srcBuf, global, options, &module);
+      rv = FillCompileOptionsForRequest(aes, aRequest, global, &options);
+
+      if (NS_SUCCEEDED(rv)) {
+        nsAutoString inlineData;
+        SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
+        rv = nsJSUtils::CompileModule(cx, srcBuf, global, options, &module);
+      }
     }
     MOZ_ASSERT(NS_SUCCEEDED(rv) == (module != nullptr));
     if (module) {
       aRequest->mModuleScript =
         new nsModuleScript(this, aRequest->mBaseURL, module);
     }
   }
 
@@ -1839,17 +1841,21 @@ nsScriptLoader::AttemptAsyncScriptCompil
   AutoJSAPI jsapi;
   if (!jsapi.Init(globalObject)) {
     return NS_ERROR_FAILURE;
   }
 
   JSContext* cx = jsapi.cx();
   JS::Rooted<JSObject*> global(cx, globalObject->GetGlobalJSObject());
   JS::CompileOptions options(cx);
-  FillCompileOptionsForRequest(jsapi, aRequest, global, &options);
+
+  nsresult rv = FillCompileOptionsForRequest(jsapi, aRequest, global, &options);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   if (!JS::CanCompileOffThread(cx, options, aRequest->mScriptTextLength)) {
     return NS_ERROR_FAILURE;
   }
 
   RefPtr<NotifyOffThreadScriptLoadCompletedRunnable> runnable =
     new NotifyOffThreadScriptLoadCompletedRunnable(aRequest, this);
 
@@ -2058,25 +2064,30 @@ nsScriptLoader::GetScriptGlobalObject()
   nsresult rv = globalObject->EnsureScriptEnvironment();
   if (NS_FAILED(rv)) {
     return nullptr;
   }
 
   return globalObject.forget();
 }
 
-void
-nsScriptLoader::FillCompileOptionsForRequest(const AutoJSAPI &jsapi,
-                                             nsScriptLoadRequest *aRequest,
-                                             JS::Handle<JSObject *> aScopeChain,
-                                             JS::CompileOptions *aOptions)
+nsresult
+nsScriptLoader::FillCompileOptionsForRequest(const AutoJSAPI&jsapi,
+                                             nsScriptLoadRequest* aRequest,
+                                             JS::Handle<JSObject*> aScopeChain,
+                                             JS::CompileOptions* aOptions)
 {
   // It's very important to use aRequest->mURI, not the final URI of the channel
   // aRequest ended up getting script data from, as the script filename.
-  nsContentUtils::GetWrapperSafeScriptFilename(mDocument, aRequest->mURI, aRequest->mURL);
+  nsresult rv;
+  nsContentUtils::GetWrapperSafeScriptFilename(mDocument, aRequest->mURI,
+                                               aRequest->mURL, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   bool isScriptElement = !aRequest->IsModuleRequest() ||
                          aRequest->AsModuleRequest()->IsTopLevel();
   aOptions->setIntroductionType(isScriptElement ? "scriptElement"
                                                 : "importedModule");
   aOptions->setFileAndLine(aRequest->mURL.get(), aRequest->mLineNo);
   aOptions->setVersion(JSVersion(aRequest->mJSVersion));
   aOptions->setIsRunOnce(true);
@@ -2096,16 +2107,18 @@ nsScriptLoader::FillCompileOptionsForReq
   JS::Rooted<JS::Value> elementVal(cx);
   MOZ_ASSERT(aRequest->mElement);
   if (NS_SUCCEEDED(nsContentUtils::WrapNative(cx, aRequest->mElement,
                                               &elementVal,
                                               /* aAllowWrapping = */ true))) {
     MOZ_ASSERT(elementVal.isObject());
     aOptions->setElement(&elementVal.toObject());
   }
+
+  return NS_OK;
 }
 
 nsresult
 nsScriptLoader::EvaluateScript(nsScriptLoadRequest* aRequest)
 {
   // We need a document to evaluate scripts.
   if (!mDocument) {
     return NS_ERROR_FAILURE;
@@ -2176,22 +2189,24 @@ nsScriptLoader::EvaluateScript(nsScriptL
         rv = NS_ERROR_FAILURE;
       } else {
         JS::Rooted<JSObject*> module(aes.cx(), ms->ModuleRecord());
         MOZ_ASSERT(module);
         rv = nsJSUtils::ModuleEvaluation(aes.cx(), module);
       }
     } else {
       JS::CompileOptions options(aes.cx());
-      FillCompileOptionsForRequest(aes, aRequest, global, &options);
-
-      nsAutoString inlineData;
-      SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
-      rv = nsJSUtils::EvaluateString(aes.cx(), srcBuf, global, options,
-                                     aRequest->OffThreadTokenPtr());
+      rv = FillCompileOptionsForRequest(aes, aRequest, global, &options);
+
+      if (NS_SUCCEEDED(rv)) {
+        nsAutoString inlineData;
+        SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
+        rv = nsJSUtils::EvaluateString(aes.cx(), srcBuf, global, options,
+                                       aRequest->OffThreadTokenPtr());
+      }
     }
   }
 
   context->SetProcessingScriptTag(oldProcessingScriptTag);
   return rv;
 }
 
 void
--- a/dom/base/nsScriptLoader.h
+++ b/dom/base/nsScriptLoader.h
@@ -544,20 +544,20 @@ private:
   nsresult CompileOffThreadOrProcessRequest(nsScriptLoadRequest* aRequest);
   void FireScriptAvailable(nsresult aResult,
                            nsScriptLoadRequest* aRequest);
   void FireScriptEvaluated(nsresult aResult,
                            nsScriptLoadRequest* aRequest);
   nsresult EvaluateScript(nsScriptLoadRequest* aRequest);
 
   already_AddRefed<nsIScriptGlobalObject> GetScriptGlobalObject();
-  void FillCompileOptionsForRequest(const mozilla::dom::AutoJSAPI &jsapi,
-                                    nsScriptLoadRequest *aRequest,
-                                    JS::Handle<JSObject *> aScopeChain,
-                                    JS::CompileOptions *aOptions);
+  nsresult FillCompileOptionsForRequest(const mozilla::dom::AutoJSAPI& jsapi,
+                                        nsScriptLoadRequest* aRequest,
+                                        JS::Handle<JSObject*> aScopeChain,
+                                        JS::CompileOptions* aOptions);
 
   uint32_t NumberOfProcessors();
   nsresult PrepareLoadedRequest(nsScriptLoadRequest* aRequest,
                                 nsIIncrementalStreamLoader* aLoader,
                                 nsresult aStatus,
                                 mozilla::Vector<char16_t> &aString);
 
   void AddDeferRequest(nsScriptLoadRequest* aRequest);
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -419,20 +419,25 @@ nsXBLService::LoadBindings(nsIContent* a
   *aBinding = nullptr;
   *aResolveStyle = false;
 
   nsresult rv;
 
   nsCOMPtr<nsIDocument> document = aContent->OwnerDoc();
 
   nsAutoCString urlspec;
-  if (nsContentUtils::GetWrapperSafeScriptFilename(document, aURL, urlspec)) {
+  bool ok = nsContentUtils::GetWrapperSafeScriptFilename(document, aURL,
+                                                         urlspec, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  if (ok) {
     // Block an attempt to load a binding that has special wrapper
     // automation needs.
-
     return NS_OK;
   }
 
   nsXBLBinding *binding = aContent->GetXBLBinding();
   if (binding) {
     if (binding->MarkedForDeath()) {
       FlushStyleBindings(aContent);
       binding = nullptr;
--- a/dom/xul/nsXULElement.cpp
+++ b/dom/xul/nsXULElement.cpp
@@ -2778,18 +2778,22 @@ nsXULPrototypeScript::Compile(JS::Source
 {
     // We'll compile the script in the compilation scope.
     AutoJSAPI jsapi;
     if (!jsapi.Init(xpc::CompilationScope())) {
         return NS_ERROR_UNEXPECTED;
     }
     JSContext* cx = jsapi.cx();
 
+    nsresult rv;
     nsAutoCString urlspec;
-    nsContentUtils::GetWrapperSafeScriptFilename(aDocument, aURI, urlspec);
+    nsContentUtils::GetWrapperSafeScriptFilename(aDocument, aURI, urlspec, &rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
 
     // Ok, compile it to create a prototype script object!
     NS_ENSURE_TRUE(JSVersion(mLangVersion) != JSVERSION_UNKNOWN, NS_OK);
     JS::CompileOptions options(cx);
     options.setIntroductionType("scriptElement")
            .setFileAndLine(urlspec.get(), aLineNo)
            .setVersion(JSVersion(mLangVersion));
     // If the script was inline, tell the JS parser to save source for