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

This commit is contained in:
Boris Zbarsky 2011-12-19 12:48:12 -05:00
parent f78b6a85f7
commit 9e927dd338
4 changed files with 29 additions and 27 deletions

View File

@ -120,7 +120,7 @@ public:
nsString mScriptText; // Holds script for loaded scripts nsString mScriptText; // Holds script for loaded scripts
PRUint32 mJSVersion; PRUint32 mJSVersion;
nsCOMPtr<nsIURI> mURI; nsCOMPtr<nsIURI> mURI;
nsCOMPtr<nsIURI> mFinalURI; nsCOMPtr<nsIPrincipal> mOriginPrincipal;
PRInt32 mLineNo; PRInt32 mLineNo;
}; };
@ -882,8 +882,6 @@ nsScriptLoader::EvaluateScript(nsScriptLoadRequest* aRequest,
return NS_ERROR_FAILURE; return NS_ERROR_FAILURE;
} }
nsIURI* uri = aRequest->mFinalURI ? aRequest->mFinalURI : aRequest->mURI;
bool oldProcessingScriptTag = context->GetProcessingScriptTag(); bool oldProcessingScriptTag = context->GetProcessingScriptTag();
context->SetProcessingScriptTag(true); context->SetProcessingScriptTag(true);
@ -891,13 +889,15 @@ nsScriptLoader::EvaluateScript(nsScriptLoadRequest* aRequest,
nsCOMPtr<nsIScriptElement> oldCurrent = mCurrentScript; nsCOMPtr<nsIScriptElement> oldCurrent = mCurrentScript;
mCurrentScript = aRequest->mElement; 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; nsCAutoString url;
nsContentUtils::GetWrapperSafeScriptFilename(mDocument, uri, url); nsContentUtils::GetWrapperSafeScriptFilename(mDocument, aRequest->mURI, url);
bool isUndefined; bool isUndefined;
rv = context->EvaluateString(aScript, globalObject->GetGlobalJSObject(), rv = context->EvaluateString(aScript, globalObject->GetGlobalJSObject(),
mDocument->NodePrincipal(), mDocument->NodePrincipal(),
mDocument->NodePrincipal(), aRequest->mOriginPrincipal,
url.get(), aRequest->mLineNo, url.get(), aRequest->mLineNo,
aRequest->mJSVersion, nsnull, &isUndefined); aRequest->mJSVersion, nsnull, &isUndefined);
@ -1214,7 +1214,10 @@ nsScriptLoader::PrepareLoadedRequest(nsScriptLoadRequest* aRequest,
} }
nsCOMPtr<nsIChannel> channel = do_QueryInterface(req); 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) { if (aStringLen) {
// Check the charset attribute to determine script charset. // Check the charset attribute to determine script charset.
nsAutoString hintCharset; nsAutoString hintCharset;

View File

@ -19,7 +19,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=461735
var errorFired = false; var errorFired = false;
window.onerror = function(message, uri, line) { window.onerror = function(message, uri, line) {
is(message, "Script error.", "Should have empty error message"); 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"); is(line, 0, "Shouldn't have a line here");
errorFired = true; errorFired = true;
} }
@ -32,7 +34,9 @@ window.onerror = function(message, uri, line) {
<script type="application/javascript"> <script type="application/javascript">
window.onerror = function(message, uri, line) { window.onerror = function(message, uri, line) {
is(message, "c is not defined", "Should have correct error message"); 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"); is(line, 3, "Should have a line here");
errorFired = true; errorFired = true;
} }

View File

@ -89,6 +89,8 @@
#include "nsGlobalWindow.h" #include "nsGlobalWindow.h"
#include "nsScriptNameSpaceManager.h" #include "nsScriptNameSpaceManager.h"
#include "nsJSPrincipals.h"
#ifdef XP_MACOSX #ifdef XP_MACOSX
// AssertMacros.h defines 'check' and conflicts with AccessCheck.h // AssertMacros.h defines 'check' and conflicts with AccessCheck.h
#undef check #undef check
@ -283,13 +285,15 @@ class ScriptErrorEvent : public nsRunnable
{ {
public: public:
ScriptErrorEvent(nsIScriptGlobalObject* aScriptGlobal, ScriptErrorEvent(nsIScriptGlobalObject* aScriptGlobal,
nsIPrincipal* aScriptOriginPrincipal,
PRUint32 aLineNr, PRUint32 aColumn, PRUint32 aFlags, PRUint32 aLineNr, PRUint32 aColumn, PRUint32 aFlags,
const nsAString& aErrorMsg, const nsAString& aErrorMsg,
const nsAString& aFileName, const nsAString& aFileName,
const nsAString& aSourceLine, const nsAString& aSourceLine,
bool aDispatchEvent, bool aDispatchEvent,
PRUint64 aInnerWindowID) PRUint64 aInnerWindowID)
: mScriptGlobal(aScriptGlobal), mLineNr(aLineNr), mColumn(aColumn), : mScriptGlobal(aScriptGlobal), mOriginPrincipal(aScriptOriginPrincipal),
mLineNr(aLineNr), mColumn(aColumn),
mFlags(aFlags), mErrorMsg(aErrorMsg), mFileName(aFileName), mFlags(aFlags), mErrorMsg(aErrorMsg), mFileName(aFileName),
mSourceLine(aSourceLine), mDispatchEvent(aDispatchEvent), mSourceLine(aSourceLine), mDispatchEvent(aDispatchEvent),
mInnerWindowID(aInnerWindowID) mInnerWindowID(aInnerWindowID)
@ -320,17 +324,11 @@ public:
nsIPrincipal* p = sop->GetPrincipal(); nsIPrincipal* p = sop->GetPrincipal();
NS_ENSURE_STATE(p); NS_ENSURE_STATE(p);
bool sameOrigin = mFileName.IsVoid(); bool sameOrigin = !mOriginPrincipal;
if (p && !sameOrigin) { if (p && !sameOrigin) {
nsCOMPtr<nsIURI> errorURI; if (NS_FAILED(p->Subsumes(mOriginPrincipal, &sameOrigin))) {
NS_NewURI(getter_AddRefs(errorURI), mFileName); sameOrigin = false;
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));
} }
} }
@ -342,13 +340,6 @@ public:
NS_WARNING("Not same origin error!"); NS_WARNING("Not same origin error!");
errorevent.errorMsg = xoriginMsg.get(); errorevent.errorMsg = xoriginMsg.get();
errorevent.lineNr = 0; 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, nsEventDispatcher::Dispatch(win, presContext, &errorevent, nsnull,
@ -407,6 +398,7 @@ public:
nsCOMPtr<nsIScriptGlobalObject> mScriptGlobal; nsCOMPtr<nsIScriptGlobalObject> mScriptGlobal;
nsCOMPtr<nsIPrincipal> mOriginPrincipal;
PRUint32 mLineNr; PRUint32 mLineNr;
PRUint32 mColumn; PRUint32 mColumn;
PRUint32 mFlags; PRUint32 mFlags;
@ -502,8 +494,11 @@ NS_ScriptErrorReporter(JSContext *cx,
innerWindowID = innerWin->WindowID(); innerWindowID = innerWin->WindowID();
} }
} }
JSPrincipals *prin = report->originPrincipals;
nsIPrincipal *principal =
prin ? static_cast<nsJSPrincipals*>(prin)->nsIPrincipalPtr : nsnull;
nsContentUtils::AddScriptRunner( nsContentUtils::AddScriptRunner(
new ScriptErrorEvent(globalObject, report->lineno, new ScriptErrorEvent(globalObject, principal, report->lineno,
report->uctokenptr - report->uclinebuf, report->uctokenptr - report->uclinebuf,
report->flags, msg, fileName, sourceLine, report->flags, msg, fileName, sourceLine,
report->errorNumber != JSMSG_OUT_OF_MEMORY, report->errorNumber != JSMSG_OUT_OF_MEMORY,

View File

@ -51,7 +51,7 @@ if (typeof window == 'undefined')
} }
else 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 = window.onerror;
window.onerror = (function (msg, page, line) { window.onerror = (function (msg, page, line) {