Bug 840488 - Get rid of aAllowIfNoScriptContext. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Tue, 12 Nov 2013 16:43:32 -0800
changeset 169370 4f3f338af4c7aa15367ab7f316cb4e4958572cf7
parent 169369 9bff006f8fdab1d214af8424021150696189a6a0
child 169371 3c6edeaccb47caf2f1f605976fb07873bb10c8c4
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs840488
milestone28.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 840488 - Get rid of aAllowIfNoScriptContext. r=bz The old code seemed to feel that the lack of a script context was some sort of showdown that required either unconditional allow or deny. Instead, let's just make the scriptcontext-relevant checks conditional on there being a script context, which lets us switch CheckFunctionAccess over to ScriptAllowed.
caps/include/nsScriptSecurityManager.h
caps/src/nsScriptSecurityManager.cpp
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -457,24 +457,18 @@ private:
      *                                or equal privileges to the object.
      */
     nsresult
     CheckXPCPermissions(JSContext* cx,
                         nsISupports* aObj, JSObject* aJSObject,
                         nsIPrincipal* aSubjectPrincipal,
                         const char* aObjectSecurityLevel);
 
-    /**
-     * Helper for CanExecuteScripts that allows the caller to specify
-     * whether execution should be allowed if cx has no
-     * nsIScriptContext.
-     */
     nsresult
-    CanExecuteScripts(JSContext* cx, nsIPrincipal *aPrincipal,
-                      bool aAllowIfNoScriptContext, bool *result);
+    CanExecuteScripts(JSContext* cx, nsIPrincipal *aPrincipal, bool *result);
 
     nsresult
     Init();
 
     nsresult
     InitPrefs();
 
     nsresult
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -1615,23 +1615,18 @@ nsScriptSecurityManager::CheckFunctionAc
 
     if (subject == mSystemPrincipal)
         // This is the system principal: just allow access
         return NS_OK;
 
     // Check if the principal the function was compiled under is
     // allowed to execute scripts.
 
-    bool result;
-    rv = CanExecuteScripts(aCx, subject, true, &result);
-    if (NS_FAILED(rv))
-      return rv;
-
-    if (!result)
-      return NS_ERROR_DOM_SECURITY_ERR;
+    if (!ScriptAllowed(js::GetGlobalForObjectCrossCompartment(rootedFunObj)))
+        return NS_ERROR_DOM_SECURITY_ERR;
 
     if (!aTargetObj) {
         // We're done here
         return NS_OK;
     }
 
     /*
     ** Get origin of subject and object and compare.
@@ -1648,17 +1643,16 @@ nsScriptSecurityManager::CheckFunctionAc
         rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
     }
     return rv;
 }
 
 nsresult
 nsScriptSecurityManager::CanExecuteScripts(JSContext* cx,
                                            nsIPrincipal *aPrincipal,
-                                           bool aAllowIfNoScriptContext,
                                            bool *result)
 {
     *result = false; 
 
     if (aPrincipal == mSystemPrincipal)
     {
         // Even if JavaScript is disabled, we must still execute system scripts
         *result = true;
@@ -1670,67 +1664,59 @@ nsScriptSecurityManager::CanExecuteScrip
     if (ep)
     {
         *result = true;
         return NS_OK;
     }
 
     //-- See if the current window allows JS execution
     nsIScriptContext *scriptContext = GetScriptContext(cx);
-    if (!scriptContext) {
-        if (aAllowIfNoScriptContext) {
-            *result = true;
+    if (scriptContext) {
+        if (!scriptContext->GetScriptsEnabled()) {
+            // No scripting on this context, folks
+            *result = false;
             return NS_OK;
         }
-        return NS_ERROR_FAILURE;
-    }
 
-    if (!scriptContext->GetScriptsEnabled()) {
-        // No scripting on this context, folks
-        *result = false;
-        return NS_OK;
-    }
-    
-    nsIScriptGlobalObject *sgo = scriptContext->GetGlobalObject();
-
-    if (!sgo) {
-        return NS_ERROR_FAILURE;
-    }
+        nsIScriptGlobalObject *sgo = scriptContext->GetGlobalObject();
+        if (!sgo) {
+            return NS_ERROR_FAILURE;
+        }
 
-    // window can be null here if we're running with a non-DOM window
-    // as the script global (i.e. a XUL prototype document).
-    nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(sgo);
-    nsCOMPtr<nsIDocShell> docshell;
-    nsresult rv;
+        // window can be null here if we're running with a non-DOM window
+        // as the script global (i.e. a XUL prototype document).
+        nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(sgo);
+        nsCOMPtr<nsIDocShell> docshell;
+        nsresult rv;
+        if (window) {
+            docshell = window->GetDocShell();
+        }
 
-    if (window) {
-        docshell = window->GetDocShell();
-    }
-
-    if (docshell) {
-      rv = docshell->GetCanExecuteScripts(result);
-      if (NS_FAILED(rv)) return rv;
-      if (!*result) return NS_OK;
+        if (docshell) {
+          rv = docshell->GetCanExecuteScripts(result);
+          if (NS_FAILED(rv)) return rv;
+          if (!*result) return NS_OK;
+        }
     }
 
     // OK, the docshell doesn't have script execution explicitly disabled.
     // Check whether our URI is an "about:" URI that allows scripts.  If it is,
     // we need to allow JS to run.  In this case, don't apply the JS enabled
     // pref or policies.  On failures, just press on and don't do this special
     // case.
     nsCOMPtr<nsIURI> principalURI;
     aPrincipal->GetURI(getter_AddRefs(principalURI));
     if (!principalURI) {
         // Broken principal of some sort.  Disallow.
         *result = false;
         return NS_ERROR_UNEXPECTED;
     }
         
     bool isAbout;
-    rv = principalURI->SchemeIs("about", &isAbout);
+    nsresult rv = principalURI->SchemeIs("about", &isAbout);
     if (NS_SUCCEEDED(rv) && isAbout) {
         nsCOMPtr<nsIAboutModule> module;
         rv = NS_GetAboutModule(principalURI, getter_AddRefs(module));
         if (NS_SUCCEEDED(rv)) {
             uint32_t flags;
             rv = module->GetURIFlags(principalURI, &flags);
             if (NS_SUCCEEDED(rv) &&
                 (flags & nsIAboutModule::ALLOW_SCRIPT)) {
@@ -1769,19 +1755,17 @@ nsScriptSecurityManager::ScriptAllowed(J
     MOZ_ASSERT(aGlobal);
     MOZ_ASSERT(JS_IsGlobalObject(aGlobal) || js::IsOuterObject(aGlobal));
     AutoJSContext cx_;
     JS::RootedObject global(cx_, js::UncheckedUnwrap(aGlobal, /* stopAtOuter = */ false));
 
     nsCOMPtr<nsIScriptContext> scx = nsJSUtils::GetStaticScriptContext(global);
     AutoPushJSContext cx(scx ? scx->GetNativeContext() : GetSafeJSContext());
     bool result = false;
-    nsresult rv = CanExecuteScripts(cx, doGetObjectPrincipal(global),
-                                    /* aAllowIfNoScriptContext = */ false,
-                                    &result);
+    nsresult rv = CanExecuteScripts(cx, doGetObjectPrincipal(global), &result);
     return NS_SUCCEEDED(rv) && result;
 }
 
 ///////////////// Principals ///////////////////////
 NS_IMETHODIMP
 nsScriptSecurityManager::GetSubjectPrincipal(nsIPrincipal **aSubjectPrincipal)
 {
     nsresult rv;