author | Carsten "Tomcat" Book <cbook@mozilla.com> |
Thu, 21 Jul 2016 06:14:41 +0200 | |
changeset 306006 | 316baefffba940722b5d7096207d69dfce2c5aa2 |
parent 306005 | b1f9e16f3e5a5873ef8b3bb7555a4b2cebd7d958 |
child 306007 | b73eac8de8146084eb61d84eca4cf1e8b2a8b85c |
push id | 30474 |
push user | cbook@mozilla.com |
push date | Thu, 21 Jul 2016 14:25:10 +0000 |
treeherder | mozilla-central@6b180266ac16 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
bugs | 1285036 |
milestone | 50.0a1 |
backs out | 16fefebdbb50f9c10ea7786500e0bfe19545b42c |
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
|
--- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -7066,18 +7066,19 @@ nsContentUtils::IsForbiddenRequestHeader // static bool nsContentUtils::IsForbiddenSystemRequestHeader(const nsACString& aHeader) { static const char *kInvalidHeaders[] = { "accept-charset", "accept-encoding", "access-control-request-headers", "access-control-request-method", "connection", "content-length", - "cookie", "cookie2", "date", "dnt", "expect", "host", "keep-alive", - "origin", "referer", "te", "trailer", "transfer-encoding", "upgrade", "via" + "cookie", "cookie2", "content-transfer-encoding", "date", "dnt", + "expect", "host", "keep-alive", "origin", "referer", "te", "trailer", + "transfer-encoding", "upgrade", "via" }; for (uint32_t i = 0; i < ArrayLength(kInvalidHeaders); ++i) { if (aHeader.LowerCaseEqualsASCII(kInvalidHeaders[i])) { return true; } } return false; }
--- a/dom/xhr/XMLHttpRequestMainThread.cpp +++ b/dom/xhr/XMLHttpRequestMainThread.cpp @@ -1460,17 +1460,17 @@ XMLHttpRequestMainThread::Open(const nsA userpass.Append(':'); AppendUTF16toUTF8(password.Value(), userpass); } uri->SetUserPass(userpass); } // Clear our record of previously set headers so future header set // operations will merge/override correctly. - mAuthorRequestHeaders.Clear(); + mAlreadySetHeaders.Clear(); // When we are called from JS we can find the load group for the page, // and add ourselves to it. This way any pending requests // will be automatically aborted if the user leaves the page. nsCOMPtr<nsILoadGroup> loadGroup = GetLoadGroup(); nsSecurityFlags secFlags; nsLoadFlags loadFlags = nsIRequest::LOAD_BACKGROUND | @@ -2475,19 +2475,16 @@ XMLHttpRequestMainThread::Send(nsIVarian // an asynchronous call. // Ignore argument if method is GET, there is no point in trying to // upload anything nsAutoCString method; nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel)); if (httpChannel) { - // Spec step 5 - SetAuthorRequestHeadersOnChannel(httpChannel); - httpChannel->GetRequestMethod(method); // If GET, method name will be uppercase if (!IsSystemXHR()) { nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner(); nsCOMPtr<nsIDocument> doc = owner ? owner->GetExtantDoc() : nullptr; nsContentUtils::SetFetchReferrerURIWithPolicy(mPrincipal, doc, httpChannel, mozilla::net::RP_Default); } @@ -2532,24 +2529,25 @@ XMLHttpRequestMainThread::Send(nsIVarian &size_u64, defaultContentType, charset); NS_ENSURE_SUCCESS(rv, rv); // make sure it fits within js MAX_SAFE_INTEGER mUploadTotal = net::InScriptableRange(size_u64) ? static_cast<int64_t>(size_u64) : -1; if (postDataStream) { - // If author set no Content-Type, use the default from GetRequestBody(). + // If no content type header was set by the client, we set it to + // application/xml. nsAutoCString contentType; - - GetAuthorRequestHeaderValue("content-type", contentType); - if (contentType.IsVoid()) { + if (NS_FAILED(httpChannel-> + GetRequestHeader(NS_LITERAL_CSTRING("Content-Type"), + contentType))) { contentType = defaultContentType; - if (!charset.IsEmpty()) { + if (!charset.IsEmpty() && !contentType.IsVoid()) { // If we are providing the default content type, then we also need to // provide a charset declaration. contentType.Append(NS_LITERAL_CSTRING(";charset=")); contentType.Append(charset); } } // We don't want to set a charset for streams. @@ -2717,36 +2715,18 @@ XMLHttpRequestMainThread::Send(nsIVarian // Check if we should enabled cross-origin upload listeners. if (mUpload && mUpload->HasListeners()) { mFlagHadUploadListenersOnSend = true; } // Set up the preflight if needed if (!IsSystemXHR()) { - nsTArray<nsCString> CORSUnsafeHeaders; - const char *kCrossOriginSafeHeaders[] = { - "accept", "accept-language", "content-language", "content-type", - "last-event-id" - }; - for (RequestHeader& header : mAuthorRequestHeaders) { - bool safe = false; - for (uint32_t i = 0; i < ArrayLength(kCrossOriginSafeHeaders); ++i) { - if (header.name.EqualsASCII(kCrossOriginSafeHeaders[i])) { - safe = true; - break; - } - } - if (!safe) { - CORSUnsafeHeaders.AppendElement(header.name); - } - } - nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo(); - loadInfo->SetCorsPreflightInfo(CORSUnsafeHeaders, + loadInfo->SetCorsPreflightInfo(mCORSUnsafeHeaders, mFlagHadUploadListenersOnSend); } mIsMappedArrayBuffer = false; if (mResponseType == XMLHttpRequestResponseType::Arraybuffer && Preferences::GetBool("dom.mapped_arraybuffer.enabled", true)) { nsCOMPtr<nsIURI> uri; nsAutoCString scheme; @@ -2862,72 +2842,109 @@ XMLHttpRequestMainThread::Send(nsIVarian return NS_ERROR_FAILURE; } return rv; } // http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#dom-xmlhttprequest-setrequestheader NS_IMETHODIMP -XMLHttpRequestMainThread::SetRequestHeader(const nsACString& aName, - const nsACString& aValue) +XMLHttpRequestMainThread::SetRequestHeader(const nsACString& header, + const nsACString& value) { // Steps 1 and 2 if (mState != State::opened || mFlagSend) { return NS_ERROR_DOM_INVALID_STATE_ERR; } + NS_ASSERTION(mChannel, "mChannel must be valid if we're OPENED."); // Step 3 - nsAutoCString value(aValue); - static const char kHTTPWhitespace[] = "\n\t\r "; - value.Trim(kHTTPWhitespace); - - // Step 4 - if (!NS_IsValidHTTPToken(aName) || !NS_IsReasonableHTTPHeaderValue(value)) { + // Make sure we don't store an invalid header name in mCORSUnsafeHeaders + if (!NS_IsValidHTTPToken(header)) { return NS_ERROR_DOM_SYNTAX_ERR; } - // Step 5 - bool isPrivilegedCaller = IsSystemXHR(); - bool isForbiddenHeader = nsContentUtils::IsForbiddenRequestHeader(aName); - if (!isPrivilegedCaller && isForbiddenHeader) { - NS_WARNING("refusing to set request header"); + if (!mChannel) // open() initializes mChannel, and open() + return NS_ERROR_FAILURE; // must be called before first setRequestHeader() + + nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel); + if (!httpChannel) { return NS_OK; } - // Step 6.1 - nsAutoCString lowercaseName; - nsContentUtils::ASCIIToLower(aName, lowercaseName); - - // Step 6.2 - bool notAlreadySet = true; - for (RequestHeader& header : mAuthorRequestHeaders) { - if (header.name.Equals(lowercaseName)) { - // Gecko-specific: invalid headers can be set by privileged - // callers, but will not merge. - if (isPrivilegedCaller && isForbiddenHeader) { - header.value.Assign(value); - } else { - header.value.AppendLiteral(", "); - header.value.Append(value); + // We will merge XHR headers, per the spec (secion 4.6.2) unless: + // 1 - The caller is privileged and setting an invalid header, + // or + // 2 - we have not yet explicitly set that header; this allows web + // content to override default headers the first time they set them. + bool mergeHeaders = true; + + if (!IsSystemXHR()) { + // Step 5: Check for dangerous headers. + // Prevent modification to certain HTTP headers (see bug 302263), unless + // the executing script is privileged. + if (nsContentUtils::IsForbiddenRequestHeader(header)) { + NS_WARNING("refusing to set request header"); + return NS_OK; + } + + // Check for dangerous cross-site headers + bool safeHeader = IsSystemXHR(); + if (!safeHeader) { + // Content-Type isn't always safe, but we'll deal with it in Send() + const char *kCrossOriginSafeHeaders[] = { + "accept", "accept-language", "content-language", "content-type", + "last-event-id" + }; + for (uint32_t i = 0; i < ArrayLength(kCrossOriginSafeHeaders); ++i) { + if (header.LowerCaseEqualsASCII(kCrossOriginSafeHeaders[i])) { + safeHeader = true; + break; + } } - notAlreadySet = false; - break; + } + + if (!safeHeader) { + if (!mCORSUnsafeHeaders.Contains(header, nsCaseInsensitiveCStringArrayComparator())) { + mCORSUnsafeHeaders.AppendElement(header); + } + } + } else { + // Case 1 above + if (nsContentUtils::IsForbiddenSystemRequestHeader(header)) { + mergeHeaders = false; } } - // Step 6.3 - if (notAlreadySet) { - RequestHeader newHeader = { - nsCString(lowercaseName), nsCString(value) + if (!mAlreadySetHeaders.Contains(header)) { + // Case 2 above + mergeHeaders = false; + } + + nsresult rv; + if (value.IsEmpty()) { + rv = httpChannel->SetEmptyRequestHeader(header); + } else { + // Merge headers depending on what we decided above. + rv = httpChannel->SetRequestHeader(header, value, mergeHeaders); + } + if (rv == NS_ERROR_INVALID_ARG) { + return NS_ERROR_DOM_SYNTAX_ERR; + } + if (NS_SUCCEEDED(rv)) { + // Remember that we've set this header, so subsequent set operations will merge values. + mAlreadySetHeaders.PutEntry(nsCString(header)); + + // We'll want to duplicate this header for any replacement channels (eg. on redirect) + RequestHeader reqHeader = { + nsCString(header), nsCString(value) }; - mAuthorRequestHeaders.AppendElement(newHeader); + mModifiedRequestHeaders.AppendElement(reqHeader); } - - return NS_OK; + return rv; } NS_IMETHODIMP XMLHttpRequestMainThread::GetTimeout(uint32_t *aTimeout) { *aTimeout = Timeout(); return NS_OK; } @@ -3157,55 +3174,37 @@ XMLHttpRequestMainThread::AsyncOnChannel mNewRedirectChannel = nullptr; } return rv; } OnRedirectVerifyCallback(NS_OK); return NS_OK; } -void -XMLHttpRequestMainThread::GetAuthorRequestHeaderValue(const char* aName, - nsACString& outValue) -{ - for (RequestHeader& header : mAuthorRequestHeaders) { - if (header.name.Equals(aName)) { - outValue.Assign(header.value); - return; - } - } - outValue.SetIsVoid(true); -} - -void -XMLHttpRequestMainThread::SetAuthorRequestHeadersOnChannel( - nsCOMPtr<nsIHttpChannel> aHttpChannel) -{ - for (RequestHeader& header : mAuthorRequestHeaders) { - if (header.value.IsEmpty()) { - aHttpChannel->SetEmptyRequestHeader(header.name); - } else { - aHttpChannel->SetRequestHeader(header.name, header.value, false); - } - } -} - nsresult XMLHttpRequestMainThread::OnRedirectVerifyCallback(nsresult result) { NS_ASSERTION(mRedirectCallback, "mRedirectCallback not set in callback"); NS_ASSERTION(mNewRedirectChannel, "mNewRedirectChannel not set in callback"); if (NS_SUCCEEDED(result)) { mChannel = mNewRedirectChannel; nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel)); if (httpChannel) { // Ensure all original headers are duplicated for the new channel (bug #553888) - SetAuthorRequestHeadersOnChannel(httpChannel); + for (RequestHeader& requestHeader : mModifiedRequestHeaders) { + if (requestHeader.value.IsEmpty()) { + httpChannel->SetEmptyRequestHeader(requestHeader.header); + } else { + httpChannel->SetRequestHeader(requestHeader.header, + requestHeader.value, + false); + } + } } } else { mErrorLoad = true; } mNewRedirectChannel = nullptr; mRedirectCallback->OnRedirectVerifyCallback(result); @@ -3511,18 +3510,19 @@ XMLHttpRequestMainThread::EnsureXPCOMifi } bool XMLHttpRequestMainThread::ShouldBlockAuthPrompt() { // Verify that it's ok to prompt for credentials here, per spec // http://xhr.spec.whatwg.org/#the-send%28%29-method - for (RequestHeader& requestHeader : mAuthorRequestHeaders) { - if (requestHeader.name.EqualsLiteral("authorization")) { + for (uint32_t i = 0, len = mModifiedRequestHeaders.Length(); i < len; ++i) { + if (mModifiedRequestHeaders[i].header. + LowerCaseEqualsLiteral("authorization")) { return true; } } nsCOMPtr<nsIURI> uri; nsresult rv = mChannel->GetURI(getter_AddRefs(uri)); if (NS_WARN_IF(NS_FAILED(rv))) { return false;
--- a/dom/xhr/XMLHttpRequestMainThread.h +++ b/dom/xhr/XMLHttpRequestMainThread.h @@ -215,20 +215,20 @@ public: const Optional<nsAString>& aPassword, ErrorResult& aRv) override { aRv = Open(aMethod, NS_ConvertUTF16toUTF8(aUrl), aAsync, aUser, aPassword); } virtual void - SetRequestHeader(const nsACString& aName, const nsACString& aValue, + SetRequestHeader(const nsACString& aHeader, const nsACString& aValue, ErrorResult& aRv) override { - aRv = SetRequestHeader(aName, aValue); + aRv = SetRequestHeader(aHeader, aValue); } virtual uint32_t Timeout() const override { return mTimeoutMilliseconds; } @@ -615,16 +615,17 @@ protected: const Optional<nsAString>& password); already_AddRefed<nsXMLHttpRequestXPCOMifier> EnsureXPCOMifier(); nsCOMPtr<nsISupports> mContext; nsCOMPtr<nsIPrincipal> mPrincipal; nsCOMPtr<nsIChannel> mChannel; nsCOMPtr<nsIDocument> mResponseXML; + nsTArray<nsCString> mCORSUnsafeHeaders; nsCOMPtr<nsIStreamListener> mXMLParserStreamListener; // used to implement getAllResponseHeaders() class nsHeaderVisitor : public nsIHttpHeaderVisitor { public: NS_DECL_ISUPPORTS @@ -785,23 +786,22 @@ protected: bool mIsMappedArrayBuffer; void ResetResponse(); bool ShouldBlockAuthPrompt(); struct RequestHeader { - nsCString name; + nsCString header; nsCString value; }; - nsTArray<RequestHeader> mAuthorRequestHeaders; + nsTArray<RequestHeader> mModifiedRequestHeaders; - void GetAuthorRequestHeaderValue(const char* aName, nsACString& outValue); - void SetAuthorRequestHeadersOnChannel(nsCOMPtr<nsIHttpChannel> aChannel); + nsTHashtable<nsCStringHashKey> mAlreadySetHeaders; // Helper object to manage our XPCOM scriptability bits nsXMLHttpRequestXPCOMifier* mXPCOMifier; static bool sDontWarnAboutSyncXHR; }; class MOZ_STACK_CLASS AutoDontWarnAboutSyncXHR
--- a/dom/xhr/tests/test_xhr_forbidden_headers.html +++ b/dom/xhr/tests/test_xhr_forbidden_headers.html @@ -23,16 +23,17 @@ var headers = [ "aCCept-chaRset", "acCePt-eNcoDing", "aCcEsS-cOnTrOl-ReQuEsT-mEtHoD", "aCcEsS-cOnTrOl-ReQuEsT-hEaDeRs", "coNnEctIon", "coNtEnt-LEngth", "CoOKIe", "cOOkiE2", + "cOntEnt-tRAnsFer-enCoDiNg", "DATE", "dNT", "exPeCt", "hOSt", "keep-alive", "oRiGiN", "reFERer", "te", @@ -48,17 +49,16 @@ var headers = [ var i, request; function startTest() { // Try setting headers in unprivileged context request = new XMLHttpRequest(); request.open("GET", window.location.href); for (i = 0; i < headers.length; i++) request.setRequestHeader(headers[i], "test" + i); - request.send(); // headers aren't set on the channel until send() // Read out headers var channel = SpecialPowers.wrap(request).channel.QueryInterface(SpecialPowers.Ci.nsIHttpChannel); for (i = 0; i < headers.length; i++) { // Retrieving Content-Length will throw an exception var value = null; try { value = channel.getRequestHeader(headers[i]); @@ -68,17 +68,16 @@ function startTest() { isnot(value, "test" + i, "Setting " + headers[i] + " header in unprivileged context"); } // Try setting headers in privileged context request = new XMLHttpRequest({mozAnon: true, mozSystem: true}); request.open("GET", window.location.href); for (i = 0; i < headers.length; i++) request.setRequestHeader(headers[i], "test" + i); - request.send(); // headers aren't set on the channel until send() // Read out headers var channel = SpecialPowers.wrap(request).channel.QueryInterface(SpecialPowers.Ci.nsIHttpChannel); for (i = 0; i < headers.length; i++) { var value = channel.getRequestHeader(headers[i]); is(value, "test" + i, "Setting " + headers[i] + " header in privileged context"); }
new file mode 100644 --- /dev/null +++ b/testing/web-platform/meta/XMLHttpRequest/setrequestheader-case-insensitive.htm.ini @@ -0,0 +1,5 @@ +[setrequestheader-case-insensitive.htm] + type: testharness + [XMLHttpRequest: setRequestHeader() - headers that differ in case] + expected: FAIL +
new file mode 100644 --- /dev/null +++ b/testing/web-platform/meta/XMLHttpRequest/setrequestheader-header-allowed.htm.ini @@ -0,0 +1,14 @@ +[setrequestheader-header-allowed.htm] + type: testharness + [XMLHttpRequest: setRequestHeader() - headers that are allowed (Authorization)] + expected: FAIL + + [XMLHttpRequest: setRequestHeader() - headers that are allowed (Content-Transfer-Encoding)] + expected: FAIL + + [XMLHttpRequest: setRequestHeader() - headers that are allowed (Content-Type)] + expected: FAIL + + [XMLHttpRequest: setRequestHeader() - headers that are allowed (User-Agent)] + expected: FAIL +