From b8d50963172e0a83b279d16cde51ebbf48be0e30 Mon Sep 17 00:00:00 2001 From: "bent.mozilla@gmail.com" Date: Tue, 19 Feb 2008 15:12:23 -0800 Subject: [PATCH] Bug 392322 - "XMLHttpRequest crashes on local file retrieval [@ nsCrossSiteListenerProxy::OnStartRequest]". r+sr=sicking, a=blocking1.9+. --- content/base/Makefile.in | 4 +- content/base/public/nsIXMLHttpRequest.idl | 21 +++++- content/base/src/nsXMLHttpRequest.cpp | 89 +++++++++++++++++------ content/base/src/nsXMLHttpRequest.h | 3 +- content/base/test/Makefile.in | 31 ++++++++ 5 files changed, 119 insertions(+), 29 deletions(-) diff --git a/content/base/Makefile.in b/content/base/Makefile.in index 73d3c2dde52..5efba183177 100644 --- a/content/base/Makefile.in +++ b/content/base/Makefile.in @@ -44,8 +44,8 @@ include $(DEPTH)/config/autoconf.mk DIRS = public src -ifdef MOZ_MOCHITEST -DIRS += test +ifdef ENABLE_TESTS +TOOL_DIRS += test endif include $(topsrcdir)/config/rules.mk diff --git a/content/base/public/nsIXMLHttpRequest.idl b/content/base/public/nsIXMLHttpRequest.idl index 980f119b73b..7c91d3401f0 100644 --- a/content/base/public/nsIXMLHttpRequest.idl +++ b/content/base/public/nsIXMLHttpRequest.idl @@ -37,10 +37,13 @@ #include "nsISupports.idl" +interface nsIChannel; interface nsIDOMDocument; interface nsIDOMEventListener; -interface nsIChannel; +interface nsIPrincipal; +interface nsIScriptContext; interface nsIVariant; +interface nsPIDOMWindow; /** * Mozilla's XMLHttpRequest is modelled after Microsoft's IXMLHttpRequest @@ -87,7 +90,7 @@ interface nsIVariant; * you're aware of all the security implications. And then think twice about * it. */ -[scriptable, uuid(7ceba511-b7a1-4b38-9643-d09c60c08146)] +[scriptable, uuid(f3fb86e6-5914-4b47-a1f6-8907e37e1159)] interface nsIXMLHttpRequest : nsISupports { /** @@ -297,6 +300,20 @@ interface nsIXMLHttpRequest : nsISupports * will be called as each part of the response is received. */ attribute boolean multipart; + + /** + * Initialize the object for use from C++ code with the principal, script + * context, and owner window that should be used. + * + * @param principal The principal to use for the request. This must not be + * null. + * @param scriptContext The script context to use for the request. May be + * null. + * @param ownerWindow The associated window for the request. May be null. + */ + [noscript] void init(in nsIPrincipal principal, + in nsIScriptContext scriptContext, + in nsPIDOMWindow ownerWindow); }; [scriptable, uuid(261676b4-d508-43bf-b099-74635a0ee2e9)] diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index ca76b3c096d..735aedbb53e 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -558,6 +558,9 @@ nsXMLHttpRequest::~nsXMLHttpRequest() nsLayoutStatics::Release(); } +/** + * This Init method is called from the factory constructor. + */ nsresult nsXMLHttpRequest::Init() { @@ -576,28 +579,55 @@ nsXMLHttpRequest::Init() return NS_OK; } - nsIScriptContext* context = GetScriptContextFromJSContext(cx); - if (!context) { - return NS_OK; - } nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager(); nsCOMPtr subjectPrincipal; if (secMan) { secMan->GetSubjectPrincipal(getter_AddRefs(subjectPrincipal)); } NS_ENSURE_STATE(subjectPrincipal); - - mScriptContext = context; mPrincipal = subjectPrincipal; - nsCOMPtr window = - do_QueryInterface(context->GetGlobalObject()); - if (window) { - mOwner = window->GetCurrentInnerWindow(); + + nsIScriptContext* context = GetScriptContextFromJSContext(cx); + if (context) { + mScriptContext = context; + nsCOMPtr window = + do_QueryInterface(context->GetGlobalObject()); + if (window) { + mOwner = window->GetCurrentInnerWindow(); + } + } + + return NS_OK; +} +/** + * This Init method should only be called by C++ consumers. + */ +NS_IMETHODIMP +nsXMLHttpRequest::Init(nsIPrincipal* aPrincipal, + nsIScriptContext* aScriptContext, + nsPIDOMWindow* aOwnerWindow) +{ + NS_ENSURE_ARG_POINTER(aPrincipal); + + // This object may have already been initialized in the other Init call above + // if JS was on the stack. Clear the old values for mScriptContext and mOwner + // if new ones are not supplied here. + + mPrincipal = aPrincipal; + mScriptContext = aScriptContext; + if (aOwnerWindow) { + mOwner = aOwnerWindow->GetCurrentInnerWindow(); + } + else { + mOwner = nsnull; } return NS_OK; } +/** + * This Initialize method is called from XPConnect via nsIJSNativeInitializer. + */ NS_IMETHODIMP nsXMLHttpRequest::Initialize(nsISupports* aOwner, JSContext* cx, JSObject* obj, PRUint32 argc, jsval *argv) @@ -1342,19 +1372,26 @@ nsXMLHttpRequest::GetCurrentHttpChannel() return httpChannel; } +inline PRBool +IsSystemPrincipal(nsIPrincipal* aPrincipal) +{ + PRBool isSystem = PR_FALSE; + nsContentUtils::GetSecurityManager()-> + IsSystemPrincipal(aPrincipal, &isSystem); + return isSystem; +} + static PRBool IsSameOrigin(nsIPrincipal* aPrincipal, nsIChannel* aChannel) { - if (!aPrincipal) { - // XXX Until we got our principal story straight we have to do this to - // support C++ callers. - return PR_TRUE; - } + NS_ASSERTION(!IsSystemPrincipal(aPrincipal), "Shouldn't get here!"); nsCOMPtr codebase; nsresult rv = aPrincipal->GetURI(getter_AddRefs(codebase)); NS_ENSURE_SUCCESS(rv, PR_FALSE); + NS_ASSERTION(codebase, "Principal must have a URI!"); + nsCOMPtr channelURI; rv = aChannel->GetURI(getter_AddRefs(channelURI)); NS_ENSURE_SUCCESS(rv, PR_FALSE); @@ -1413,6 +1450,8 @@ nsXMLHttpRequest::OpenRequest(const nsACString& method, NS_ENSURE_ARG(!method.IsEmpty()); NS_ENSURE_ARG(!url.IsEmpty()); + NS_ENSURE_TRUE(mPrincipal, NS_ERROR_NOT_INITIALIZED); + // Disallow HTTP/1.1 TRACE method (see bug 302489) // and MS IIS equivalent TRACK (see bug 381264) if (method.LowerCaseEqualsLiteral("trace") || @@ -1513,13 +1552,15 @@ nsXMLHttpRequest::OpenRequest(const nsACString& method, if (NS_FAILED(rv)) return rv; // Check if we're doing a cross-origin request. - if (!(mState & XML_HTTP_REQUEST_XSITEENABLED) && - !IsSameOrigin(mPrincipal, mChannel)) { + if (IsSystemPrincipal(mPrincipal)) { + // Chrome callers are always allowed to read from different origins. + mState |= XML_HTTP_REQUEST_XSITEENABLED; + } + else if (!(mState & XML_HTTP_REQUEST_XSITEENABLED) && + !IsSameOrigin(mPrincipal, mChannel)) { mState |= XML_HTTP_REQUEST_USE_XSITE_AC; } - //mChannel->SetAuthTriedWithPrehost(authp); - nsCOMPtr httpChannel(do_QueryInterface(mChannel)); if (httpChannel) { rv = httpChannel->SetRequestMethod(method); @@ -2037,6 +2078,8 @@ nsXMLHttpRequest::SendAsBinary(const nsAString &aBody) NS_IMETHODIMP nsXMLHttpRequest::Send(nsIVariant *aBody) { + NS_ENSURE_TRUE(mPrincipal, NS_ERROR_NOT_INITIALIZED); + nsresult rv = CheckInnerWindowCorrectness(); NS_ENSURE_SUCCESS(rv, rv); @@ -2062,12 +2105,10 @@ nsXMLHttpRequest::Send(nsIVariant *aBody) if (httpChannel) { httpChannel->GetRequestMethod(method); // If GET, method name will be uppercase - if (mPrincipal) { - nsCOMPtr codebase; - mPrincipal->GetURI(getter_AddRefs(codebase)); + nsCOMPtr codebase; + mPrincipal->GetURI(getter_AddRefs(codebase)); - httpChannel->SetReferrer(codebase); - } + httpChannel->SetReferrer(codebase); } if (aBody && httpChannel && !method.EqualsLiteral("GET")) { diff --git a/content/base/src/nsXMLHttpRequest.h b/content/base/src/nsXMLHttpRequest.h index 3dd9d0f4840..20367a38a6d 100644 --- a/content/base/src/nsXMLHttpRequest.h +++ b/content/base/src/nsXMLHttpRequest.h @@ -288,7 +288,8 @@ protected: nsCOMArray mProgressEventListeners; nsCOMArray mUploadProgressEventListeners; nsCOMArray mReadystatechangeEventListeners; - + + // These may be null (native callers or xpcshell). nsCOMPtr mScriptContext; nsCOMPtr mOwner; // Inner window. diff --git a/content/base/test/Makefile.in b/content/base/test/Makefile.in index 4c2c9e4b832..2d0a02d3e83 100644 --- a/content/base/test/Makefile.in +++ b/content/base/test/Makefile.in @@ -42,6 +42,31 @@ VPATH = @srcdir@ relativesrcdir = content/base/test include $(DEPTH)/config/autoconf.mk + +CPP_UNIT_TESTS += TestNativeXMLHttpRequest.cpp + +LOCAL_INCLUDES += -I$(topsrcdir)/xpcom/tests + +REQUIRES += \ + caps \ + content \ + dom \ + js \ + netwerk \ + string \ + xpcom \ + xpconnect \ + $(NULL) + +CPPSRCS += $(CPP_UNIT_TESTS) + +SIMPLE_PROGRAMS += $(CPP_UNIT_TESTS:.cpp=$(BIN_SUFFIX)) + +LIBS += \ + $(XPCOM_GLUE_LDOPTS) \ + $(NSPR_LIBS) \ + $(NULL) + include $(topsrcdir)/config/rules.mk _TEST_FILES = test_bug5141.html \ @@ -149,3 +174,9 @@ _TEST_FILES = test_bug5141.html \ libs:: $(_TEST_FILES) $(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir) + +check:: + @$(EXIT_ON_ERROR) \ + for f in $(subst .cpp,,$(CPP_UNIT_TESTS)); do \ + XPCOM_DEBUG_BREAK=stack-and-abort $(RUN_TEST_PROGRAM) $(DIST)/bin/$$f; \ + done