Fixing bug 410851. Expose a faster way of getting the subject principal, and use that from performance critical code. r+sr=mrbkap@gmail.com
authorjst@mozilla.org
Fri, 04 Jan 2008 15:59:12 -0800
changeset 9825 8ea600c2848c182b81a6f628d21f15c3e059e0e6
parent 9824 eac116b0d5062b7b4afaea9efa1018c80d119375
child 9826 d934642dbd7477ebce6de50db68abe0bf69cd07f
push idunknown
push userunknown
push dateunknown
bugs410851
milestone1.9b3pre
Fixing bug 410851. Expose a faster way of getting the subject principal, and use that from performance critical code. r+sr=mrbkap@gmail.com
caps/idl/nsIScriptSecurityManager.idl
caps/src/nsScriptSecurityManager.cpp
js/src/xpconnect/shell/xpcshell.cpp
js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
--- a/caps/idl/nsIScriptSecurityManager.idl
+++ b/caps/idl/nsIScriptSecurityManager.idl
@@ -37,17 +37,17 @@
 
 #include "nsISupports.idl"
 #include "nsIPrincipal.idl"
 #include "nsIXPCSecurityManager.idl"
 interface nsIURI;
 interface nsIChannel;
 
 
-[scriptable, uuid(0b8a9b32-713f-4c39-bea0-6cacec46f385)]
+[scriptable, uuid(ce216cf7-3bcb-48ab-9ff8-d03a24f19ca5)]
 interface nsIScriptSecurityManager : nsIXPCSecurityManager
 {
     ///////////////// Security Checks //////////////////
     /**
      * Checks whether the running script is allowed to access aProperty.
      */
     [noscript] void checkPropertyAccess(in JSContextPtr aJSContext,
                                         in JSObjectPtr aJSObject,
@@ -312,14 +312,22 @@ interface nsIScriptSecurityManager : nsI
     nsIPrincipal getChannelPrincipal(in nsIChannel aChannel);
 
     /**
      * Check whether a given principal is a system principal.  This allows us
      * to avoid handing back the system principal to script while allowing
      * script to check whether a given principal is system.
      */
     boolean isSystemPrincipal(in nsIPrincipal aPrincipal);
+
+    /**
+     * Same as getSubjectPrincipal(), only faster. cx must *never* be
+     * passed null, and it must be the context on the top of the
+     * context stack. Does *not* reference count the returned
+     * principal.
+     */
+    [noscript,notxpcom] nsIPrincipal getCxSubjectPrincipal(in JSContextPtr cx);
 };
 
 %{C++
 #define NS_SCRIPTSECURITYMANAGER_CONTRACTID "@mozilla.org/scriptsecuritymanager;1"
 #define NS_SCRIPTSECURITYMANAGER_CLASSNAME "scriptsecuritymanager"
 %}
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -465,16 +465,30 @@ nsScriptSecurityManager::GetChannelPrinc
 NS_IMETHODIMP
 nsScriptSecurityManager::IsSystemPrincipal(nsIPrincipal* aPrincipal,
                                            PRBool* aIsSystem)
 {
     *aIsSystem = (aPrincipal == mSystemPrincipal);
     return NS_OK;
 }
 
+NS_IMETHODIMP_(nsIPrincipal *)
+nsScriptSecurityManager::GetCxSubjectPrincipal(JSContext *cx)
+{
+    NS_ASSERTION(cx == GetCurrentJSContext(),
+                 "Uh, cx is not the current JS context!");
+
+    nsresult rv = NS_ERROR_FAILURE;
+    nsIPrincipal *principal = GetSubjectPrincipal(cx, &rv);
+    if (NS_FAILED(rv))
+        return nsnull;
+
+    return principal;
+}
+
 ////////////////////
 // Policy Storage //
 ////////////////////
 
 // Table of security levels
 PR_STATIC_CALLBACK(PRBool)
 DeleteCapability(nsHashKey *aKey, void *aData, void* closure)
 {
--- a/js/src/xpconnect/shell/xpcshell.cpp
+++ b/js/src/xpconnect/shell/xpcshell.cpp
@@ -1193,16 +1193,23 @@ FullTrustSecMan::GetChannelPrincipal(nsI
 
 /* boolean isSystemPrincipal (in nsIPrincipal aPrincipal); */
 NS_IMETHODIMP
 FullTrustSecMan::IsSystemPrincipal(nsIPrincipal *aPrincipal, PRBool *_retval)
 {
     *_retval = aPrincipal == mSystemPrincipal;
     return NS_OK;
 }
+
+NS_IMETHODIMP_(nsIPrincipal *)
+FullTrustSecMan::GetCxSubjectPrincipal(JSContext *cx)
+{
+    return mSystemPrincipal;
+}
+
 #endif
 
 /***************************************************************************/
 
 // #define TEST_InitClassesWithNewWrappedGlobal
 
 #ifdef TEST_InitClassesWithNewWrappedGlobal
 // XXX hacky test code...
--- a/js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
+++ b/js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
@@ -225,43 +225,41 @@ IsValFrame(JSContext *cx, JSObject *obj,
 // wrapper. Uses nsIScriptSecurityManager::CheckSameOriginPrincipal.
 // |cx| must be the top context on the context stack.
 // If the two principals have the same origin, returns NS_OK. If they differ,
 // returns NS_ERROR_DOM_PROP_ACCESS_DENIED, returns another error code on
 // failure.
 nsresult
 IsWrapperSameOrigin(JSContext *cx, JSObject *wrappedObj)
 {
-  nsCOMPtr<nsIPrincipal> subjectPrin, objectPrin;
-
   // Get the subject principal from the execution stack.
   nsIScriptSecurityManager *ssm = GetSecurityManager();
   if (!ssm) {
     ThrowException(NS_ERROR_NOT_INITIALIZED, cx);
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  nsresult rv = ssm->GetSubjectPrincipal(getter_AddRefs(subjectPrin));
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsIPrincipal *subjectPrin = ssm->GetCxSubjectPrincipal(cx);
 
   if (!subjectPrin) {
     ThrowException(NS_ERROR_FAILURE, cx);
     return NS_ERROR_FAILURE;
   }
 
   PRBool isSystem = PR_FALSE;
-  rv = ssm->IsSystemPrincipal(subjectPrin, &isSystem);
+  nsresult rv = ssm->IsSystemPrincipal(subjectPrin, &isSystem);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // If we somehow end up being called from chrome, just allow full access.
   // This can happen from components with xpcnativewrappers=no.
   if (isSystem) {
     return NS_OK;
   }
 
+  nsCOMPtr<nsIPrincipal> objectPrin;
   rv = ssm->GetObjectPrincipal(cx, wrappedObj, getter_AddRefs(objectPrin));
   if (NS_FAILED(rv)) {
     return rv;
   }
   NS_ASSERTION(objectPrin, "Object didn't have principals?");
 
   // Micro-optimization: don't call into caps if we know the answer.
   if (subjectPrin == objectPrin) {
--- a/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
+++ b/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
@@ -113,21 +113,23 @@ FindPrincipals(JSContext *cx, JSObject *
   }
 
   nsCOMPtr<nsIXPCSecurityManager> sm = ccx.GetXPCContext()->
     GetAppropriateSecurityManager(nsIXPCSecurityManager::HOOK_CALL_METHOD);
 
   nsCOMPtr<nsIScriptSecurityManager> ssm(do_QueryInterface(sm));
 
   if (subjectPrincipal) {
-    ssm->GetSubjectPrincipal(subjectPrincipal);
+    nsCOMPtr<nsIPrincipal> tmp = ssm->GetCxSubjectPrincipal(cx);
 
-    if (!*subjectPrincipal) {
+    if (!tmp) {
       return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
     }
+
+    tmp.swap(*subjectPrincipal);
   }
 
   ssm->GetObjectPrincipal(cx, obj, objectPrincipal);
 
   if (secMgr) {
     *secMgr = nsnull;
     ssm.swap(*secMgr);
   }