Bug 624621 part 3. Use the pre-redirect filename as the script filename and the channel principal as the origin principal, and base our cross-origin check on the origin principal. r=mrbkap
☠☠ backed out by b8d2110daaa6 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 19 Dec 2011 12:48:12 -0500
changeset 84722 316d6a49a603ce672f6d62d0c09f8673f24fcf12
parent 84721 98617f7b667bf984aa9839faa11fcf7be4319081
child 84723 1d0ffc477fe81f7203b13fa37eb1293098f441a8
push idunknown
push userunknown
push dateunknown
reviewersmrbkap
bugs624621
milestone11.0a1
Bug 624621 part 3. Use the pre-redirect filename as the script filename and the channel principal as the origin principal, and base our cross-origin check on the origin principal. r=mrbkap
content/base/src/nsScriptLoader.cpp
content/base/test/test_bug461735.html
dom/base/nsJSEnvironment.cpp
js/src/tests/js1_5/Regress/regress-328897.js
--- a/content/base/src/nsScriptLoader.cpp
+++ b/content/base/src/nsScriptLoader.cpp
@@ -115,17 +115,17 @@ public:
   }
 
   nsCOMPtr<nsIScriptElement> mElement;
   bool mLoading;             // Are we still waiting for a load to complete?
   bool mIsInline;            // Is the script inline or loaded?
   nsString mScriptText;              // Holds script for loaded scripts
   PRUint32 mJSVersion;
   nsCOMPtr<nsIURI> mURI;
-  nsCOMPtr<nsIURI> mFinalURI;
+  nsCOMPtr<nsIPrincipal> mOriginPrincipal;
   PRInt32 mLineNo;
 };
 
 // The nsScriptLoadRequest is passed as the context to necko, and thus
 // it needs to be threadsafe. Necko won't do anything with this
 // context, but it will AddRef and Release it on other threads.
 NS_IMPL_THREADSAFE_ISUPPORTS0(nsScriptLoadRequest)
 
@@ -877,32 +877,32 @@ nsScriptLoader::EvaluateScript(nsScriptL
   // Make sure context is a strong reference since we access it after
   // we've executed a script, which may cause all other references to
   // the context to go away.
   nsCOMPtr<nsIScriptContext> context = globalObject->GetScriptContext(stid);
   if (!context) {
     return NS_ERROR_FAILURE;
   }
 
-  nsIURI* uri = aRequest->mFinalURI ? aRequest->mFinalURI : aRequest->mURI;
-
   bool oldProcessingScriptTag = context->GetProcessingScriptTag();
   context->SetProcessingScriptTag(true);
 
   // Update our current script.
   nsCOMPtr<nsIScriptElement> oldCurrent = mCurrentScript;
   mCurrentScript = aRequest->mElement;
 
+  // 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.
   nsCAutoString url;
-  nsContentUtils::GetWrapperSafeScriptFilename(mDocument, uri, url);
+  nsContentUtils::GetWrapperSafeScriptFilename(mDocument, aRequest->mURI, url);
 
   bool isUndefined;
   rv = context->EvaluateString(aScript, globalObject->GetGlobalJSObject(),
                                mDocument->NodePrincipal(),
-                               mDocument->NodePrincipal(),
+                               aRequest->mOriginPrincipal,
                                url.get(), aRequest->mLineNo,
                                aRequest->mJSVersion, nsnull, &isUndefined);
 
   // Put the old script back in case it wants to do anything else.
   mCurrentScript = oldCurrent;
 
   JSContext *cx = nsnull; // Initialize this to keep GCC happy.
   if (stid == nsIProgrammingLanguage::JAVASCRIPT) {
@@ -1209,17 +1209,20 @@ nsScriptLoader::PrepareLoadedRequest(nsS
     bool requestSucceeded;
     rv = httpChannel->GetRequestSucceeded(&requestSucceeded);
     if (NS_SUCCEEDED(rv) && !requestSucceeded) {
       return NS_ERROR_NOT_AVAILABLE;
     }
   }
 
   nsCOMPtr<nsIChannel> channel = do_QueryInterface(req);
-  NS_GetFinalChannelURI(channel, getter_AddRefs(aRequest->mFinalURI));
+  rv = nsContentUtils::GetSecurityManager()->
+    GetChannelPrincipal(channel, getter_AddRefs(aRequest->mOriginPrincipal));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   if (aStringLen) {
     // Check the charset attribute to determine script charset.
     nsAutoString hintCharset;
     if (!aRequest->IsPreload()) {
       aRequest->mElement->GetScriptCharset(hintCharset);
     } else {
       nsTArray<PreloadInfo>::index_type i =
         mPreloads.IndexOf(aRequest, 0, PreloadRequestComparator());
--- a/content/base/test/test_bug461735.html
+++ b/content/base/test/test_bug461735.html
@@ -14,30 +14,34 @@ https://bugzilla.mozilla.org/show_bug.cg
 <div id="content" style="display: none">
   
 </div>
 <pre id="test">
 <script type="application/javascript">
 var errorFired = false;
 window.onerror = function(message, uri, line) {
   is(message, "Script error.", "Should have empty error message");
-  is(uri, "", "Should have empty error location URI");
+  is(uri,
+     "http://mochi.test:8888/tests/content/base/test/bug461735-redirect1.sjs",
+     "Should have pre-redirect error location URI");
   is(line, 0, "Shouldn't have a line here");
   errorFired = true;
 }
 </script>
 <script src="bug461735-redirect1.sjs"></script>
 <script>
   is(errorFired, true, "Should have error in redirected different origin script");
   errorFired = false;
 </script>
 <script type="application/javascript">
 window.onerror = function(message, uri, line) {
   is(message, "c is not defined", "Should have correct error message");
-  is(uri, "http://mochi.test:8888/tests/content/base/test/bug461735-post-redirect.js", "Unexpected error location URI");
+  is(uri,
+     "http://mochi.test:8888/tests/content/base/test/bug461735-redirect2.sjs",
+     "Unexpected error location URI");
   is(line, 3, "Should have a line here");
   errorFired = true;
 }
 </script>
 <script src="bug461735-redirect2.sjs"></script>
 <script>
   is(errorFired, true, "Should have error in same origin script");
 </script>
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -84,16 +84,18 @@
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsDOMScriptObjectHolder.h"
 #include "prmem.h"
 #include "WrapperFactory.h"
 #include "nsGlobalWindow.h"
 #include "nsScriptNameSpaceManager.h"
 
+#include "nsJSPrincipals.h"
+
 #ifdef XP_MACOSX
 // AssertMacros.h defines 'check' and conflicts with AccessCheck.h
 #undef check
 #endif
 #include "AccessCheck.h"
 
 #ifdef MOZ_JSDEBUGGER
 #include "jsdIDebuggerService.h"
@@ -278,23 +280,25 @@ NS_HandleScriptError(nsIScriptGlobalObje
   }
   return called;
 }
 
 class ScriptErrorEvent : public nsRunnable
 {
 public:
   ScriptErrorEvent(nsIScriptGlobalObject* aScriptGlobal,
+                   nsIPrincipal* aScriptOriginPrincipal,
                    PRUint32 aLineNr, PRUint32 aColumn, PRUint32 aFlags,
                    const nsAString& aErrorMsg,
                    const nsAString& aFileName,
                    const nsAString& aSourceLine,
                    bool aDispatchEvent,
                    PRUint64 aInnerWindowID)
-  : mScriptGlobal(aScriptGlobal), mLineNr(aLineNr), mColumn(aColumn),
+    : mScriptGlobal(aScriptGlobal), mOriginPrincipal(aScriptOriginPrincipal),
+      mLineNr(aLineNr), mColumn(aColumn),
     mFlags(aFlags), mErrorMsg(aErrorMsg), mFileName(aFileName),
     mSourceLine(aSourceLine), mDispatchEvent(aDispatchEvent),
     mInnerWindowID(aInnerWindowID)
   {}
 
   NS_IMETHOD Run()
   {
     nsEventStatus status = nsEventStatus_eIgnore;
@@ -315,45 +319,32 @@ public:
 
           errorevent.fileName = mFileName.get();
 
           nsCOMPtr<nsIScriptObjectPrincipal> sop(do_QueryInterface(win));
           NS_ENSURE_STATE(sop);
           nsIPrincipal* p = sop->GetPrincipal();
           NS_ENSURE_STATE(p);
 
-          bool sameOrigin = mFileName.IsVoid();
+          bool sameOrigin = !mOriginPrincipal;
 
           if (p && !sameOrigin) {
-            nsCOMPtr<nsIURI> errorURI;
-            NS_NewURI(getter_AddRefs(errorURI), mFileName);
-            if (errorURI) {
-              // FIXME: Once error reports contain the origin of the
-              // error (principals) we should change this to do the
-              // security check based on the principals and not
-              // URIs. See bug 387476.
-              sameOrigin = NS_SUCCEEDED(p->CheckMayLoad(errorURI, false));
+            if (NS_FAILED(p->Subsumes(mOriginPrincipal, &sameOrigin))) {
+              sameOrigin = false;
             }
           }
 
           NS_NAMED_LITERAL_STRING(xoriginMsg, "Script error.");
           if (sameOrigin) {
             errorevent.errorMsg = mErrorMsg.get();
             errorevent.lineNr = mLineNr;
           } else {
             NS_WARNING("Not same origin error!");
             errorevent.errorMsg = xoriginMsg.get();
             errorevent.lineNr = 0;
-            // FIXME: once the principal of the script is not tied to
-            // the filename, we can stop using the post-redirect
-            // filename if we want and remove this line.  Note that
-            // apparently we can't handle null filenames in the error
-            // event dispatching code.
-            static PRUnichar nullFilename[] = { PRUnichar(0) };
-            errorevent.fileName = nullFilename;
           }
 
           nsEventDispatcher::Dispatch(win, presContext, &errorevent, nsnull,
                                       &status);
         }
 
         sHandlingScriptError = false;
       }
@@ -402,16 +393,17 @@ public:
         }
       }
     }
     return NS_OK;
   }
 
 
   nsCOMPtr<nsIScriptGlobalObject> mScriptGlobal;
+  nsCOMPtr<nsIPrincipal>          mOriginPrincipal;
   PRUint32                        mLineNr;
   PRUint32                        mColumn;
   PRUint32                        mFlags;
   nsString                        mErrorMsg;
   nsString                        mFileName;
   nsString                        mSourceLine;
   bool                            mDispatchEvent;
   PRUint64                        mInnerWindowID;
@@ -497,18 +489,21 @@ NS_ScriptErrorReporter(JSContext *cx,
       nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(globalObject);
       PRUint64 innerWindowID = 0;
       if (win) {
         nsCOMPtr<nsPIDOMWindow> innerWin = win->GetCurrentInnerWindow();
         if (innerWin) {
           innerWindowID = innerWin->WindowID();
         }
       }
+      JSPrincipals *prin = report->originPrincipals;
+      nsIPrincipal *principal =
+        prin ? static_cast<nsJSPrincipals*>(prin)->nsIPrincipalPtr : nsnull;
       nsContentUtils::AddScriptRunner(
-        new ScriptErrorEvent(globalObject, report->lineno,
+        new ScriptErrorEvent(globalObject, principal, report->lineno,
                              report->uctokenptr - report->uclinebuf,
                              report->flags, msg, fileName, sourceLine,
                              report->errorNumber != JSMSG_OUT_OF_MEMORY,
                              innerWindowID));
     }
   }
 
 #ifdef DEBUG
--- a/js/src/tests/js1_5/Regress/regress-328897.js
+++ b/js/src/tests/js1_5/Regress/regress-328897.js
@@ -46,17 +46,17 @@ printBugNumber(BUGNUMBER);
 printStatus (summary);
  
 if (typeof window == 'undefined')
 {
   reportCompare(expect, actual, summary);
 }
 else
 {
-  expect = /(Script error.|uncaught exception: Permission denied to get property UnnamedClass.classes)/;
+  expect = /(Script error.|Permission denied for to get property XPCComponents.classes)/;
 
   window._onerror = window.onerror;
   window.onerror = (function (msg, page, line) { 
       actual = msg; 
       gDelayTestDriverEnd = false;
       jsTestDriverEnd();
       reportMatch(expect, actual, summary);
     });