Bug 1377689 - merge identical headers in set{Request,Response}Header, r=mixedpuppy,bz
MozReview-Commit-ID: Kpli9YzEvlt
--- a/dom/webidl/ChannelWrapper.webidl
+++ b/dom/webidl/ChannelWrapper.webidl
@@ -325,35 +325,45 @@ interface ChannelWrapper : EventTarget {
* actual Content-Type header.
*/
[Throws]
sequence<MozHTTPHeader> getResponseHeaders();
/**
* Sets the given request header to the given value, overwriting any
* previous value. Setting a header to a null string has the effect of
- * removing it.
+ * removing it. If merge is true, then the passed value will be merged
+ * to any existing value that exists for the header. Otherwise, any prior
+ * value for the header will be overwritten. Merge is ignored for headers
+ * that cannot be merged.
*
* For non-HTTP requests, throws NS_ERROR_UNEXPECTED.
*/
[Throws]
- void setRequestHeader(ByteString header, ByteString value);
+ void setRequestHeader(ByteString header,
+ ByteString value,
+ optional boolean merge = false);
/**
* Sets the given response header to the given value, overwriting any
* previous value. Setting a header to a null string has the effect of
- * removing it.
+ * removing it. If merge is true, then the passed value will be merged
+ * to any existing value that exists for the header (e.g. handling multiple
+ * Set-Cookie headers). Otherwise, any prior value for the header will be
+ * overwritten. Merge is ignored for headers that cannot be merged.
*
* For non-HTTP requests, throws NS_ERROR_UNEXPECTED.
*
* Note: The content type header is handled specially by this function. See
* getResponseHeaders() for details.
*/
[Throws]
- void setResponseHeader(ByteString header, ByteString value);
+ void setResponseHeader(ByteString header,
+ ByteString value,
+ optional boolean merge = false);
};
/**
* Information about the proxy server handing a request. This is approximately
* equivalent to nsIProxyInfo.
*/
dictionary MozProxyInfo {
/**
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/file_webrequestblocking_set_cookie.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html lang="en">
+ <head>
+ <meta charset="utf-8">
+ <title>Set Cookie Example</title>
+ </head>
+ <body>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/file_webrequestblocking_set_cookie.html^headers^
@@ -0,0 +1,1 @@
+Set-Cookie: reqcookie=reqvalue
--- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini
@@ -13,16 +13,18 @@ support-files =
file_WebRequest_page3.html
file_WebRequest_permission_original.html
file_WebRequest_permission_redirected.html
file_WebRequest_permission_original.js
file_WebRequest_permission_redirected.js
file_webNavigation_clientRedirect.html
file_webNavigation_clientRedirect_httpHeaders.html
file_webNavigation_clientRedirect_httpHeaders.html^headers^
+ file_webrequestblocking_set_cookie.html
+ file_webrequestblocking_set_cookie.html^headers^
file_webNavigation_frameClientRedirect.html
file_webNavigation_frameRedirect.html
file_webNavigation_manualSubframe.html
file_webNavigation_manualSubframe_page1.html
file_webNavigation_manualSubframe_page2.html
file_WebNavigation_page1.html
file_WebNavigation_page2.html
file_WebNavigation_page3.html
@@ -78,16 +80,17 @@ skip-if = (os == 'linux' && debug) || (t
[test_ext_contentscript_create_iframe.html]
[test_ext_contentscript_devtools_metadata.html]
[test_ext_contentscript_exporthelpers.html]
[test_ext_contentscript_incognito.html]
skip-if = os == 'android' # Android does not support multiple windows.
[test_ext_contentscript_css.html]
[test_ext_contentscript_about_blank.html]
skip-if = os == 'android' # bug 1369440
+[test_ext_webrequestblocking_set_cookie.html]
[test_ext_contentscript_permission.html]
[test_ext_contentscript_teardown.html]
[test_ext_exclude_include_globs.html]
[test_ext_external_messaging.html]
[test_ext_generate.html]
[test_ext_geolocation.html]
skip-if = os == 'android' # Android support Bug 1336194
[test_ext_identity.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequestblocking_set_cookie.html
@@ -0,0 +1,238 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <title>Testing modifying cookies in webRequest.onHeadersReceived</title>
+ <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+ <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+ <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+ <script type="text/javascript" src="head.js"></script>
+ <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(async function test_modifying_cookies_from_onHeadersReceived() {
+ let extension;
+ SimpleTest.waitForExplicitFinish();
+ async function background() {
+ /**
+ * Check that all the cookies described by `prefixes` are in the cookie jar.
+ *
+ * @param {Array.string} prefixes
+ * Zero or more prefixes, describing cookies that are expected to be set
+ * in the current cookie jar. Each prefix describes both a cookie
+ * name and corresponding value. For example, if the string "ext"
+ * is passed as an argument, then this function expects to see
+ * a cookie called "extcookie" and corresponding value of "extvalue".
+ */
+ async function checkCookies(prefixes) {
+ const numPrefixes = prefixes.length;
+ const currentCookies = await browser.cookies.getAll({});
+ browser.test.assertEq(numPrefixes, currentCookies.length, `${numPrefixes} cookies were set`);
+
+ for (let cookiePrefix of prefixes) {
+ let cookieName = `${cookiePrefix}cookie`;
+ let expectedCookieValue = `${cookiePrefix}value`;
+ let fetchedCookie = await browser.cookies.getAll({name: cookieName});
+ browser.test.assertEq(1, fetchedCookie.length, `Found 1 cookie with name "${cookieName}"`);
+ browser.test.assertEq(expectedCookieValue, fetchedCookie[0] && fetchedCookie[0].value, `Cookie "${cookieName}" has expected value of "${expectedCookieValue}"`);
+ }
+ }
+
+ /**
+ * Opens up a new tab, loading the given file.
+ *
+ * @param {string} filename
+ * The name of a html file in the
+ * "toolkit/components/extensions/test/mochitest" directory.
+ *
+ * @returns {object}
+ * An object with windowId and tabId properties, describing the tab
+ * that was opened.
+ */
+ async function openWindowAndTab(filename) {
+ const fileUrl = `http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/${filename}?nocache=${Math.random()}`;
+ const tabReadyPromise = new Promise(resolve => {
+ browser.webNavigation.onDOMContentLoaded.addListener(function listener({tabId}) {
+ browser.webNavigation.onDOMContentLoaded.removeListener(listener);
+ resolve(tabId);
+ }, {
+ url: [{
+ urlPrefix: fileUrl,
+ }],
+ });
+ });
+ const {id: windowId} = await browser.windows.create({url: fileUrl});
+ const tabId = await tabReadyPromise;
+ return {windowId, tabId};
+ }
+
+ /**
+ * Tests that expected cookies are in the cookie jar after opening a file.
+ *
+ * @param {string} filename
+ * The name of a html file in the
+ * "toolkit/components/extensions/test/mochitest" directory.
+ * @param {?Array.string} prefixes
+ * Zero or more prefixes, describing cookies that are expected to be set
+ * in the current cookie jar. Each prefix describes both a cookie
+ * name and corresponding value. For example, if the string "ext"
+ * is passed as an argument, then this function expects to see
+ * a cookie called "extcookie" and corresponding value of "extvalue".
+ * If undefined, then no checks are automatically performed, and the
+ * caller should provide a callback to perform the checks.
+ * @param {?Function} callback
+ * An optional async callback function that, if provided, will be called
+ * with an object that contains windowId and tabId parameters.
+ * Callers can use this callback to apply extra tests about the state of
+ * the cookie jar, or to query the state of the opened page.
+ */
+ async function testCookiesWithFile(filename, prefixes, callback) {
+ await browser.browsingData.removeCookies({});
+ const tabDetails = await openWindowAndTab(filename);
+
+ if (prefixes !== undefined) {
+ await checkCookies(prefixes);
+ }
+
+ if (callback !== undefined) {
+ await callback(tabDetails);
+ }
+ await browser.windows.remove(tabDetails.windowId);
+ }
+
+ const filter = {
+ urls: ["<all_urls>"],
+ types: ["main_frame", "sub_frame"],
+ };
+
+ const headersReceivedInfoSpec = ["blocking", "responseHeaders"];
+
+ const onHeadersReceived = details => {
+ details.responseHeaders.push({
+ name: "Set-Cookie",
+ value: "extcookie=extvalue",
+ });
+
+ return {
+ responseHeaders: details.responseHeaders,
+ };
+ };
+ browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, filter, headersReceivedInfoSpec);
+
+ if (browser.windows) {
+ // First, perform a request that should not set any cookies, and check
+ // that the cookie the extension sets is the only cookie in the
+ // cookie jar.
+ await testCookiesWithFile("file_sample.html", ["ext"]);
+
+ // Next, preform a request that will set on cookie (reqcookie=reqvalue)
+ // and check that two cookies wind up in the cookie jar (the request
+ // set cookie, and the extension set cookie).
+ await testCookiesWithFile("file_webrequestblocking_set_cookie.html", ["ext", "req"]);
+
+ // Third, register another onHeadersReceived handler that also
+ // sets a cookie (thirdcookie=thirdvalue), to make sure modifications from
+ // multiple onHeadersReceived listeners are merged correctly.
+ const thirdOnHeadersRecievedListener = details => {
+ details.responseHeaders.push({
+ name: "Set-Cookie",
+ value: "thirdcookie=thirdvalue",
+ });
+
+ browser.test.log(JSON.stringify(details.responseHeaders));
+
+ return {
+ responseHeaders: details.responseHeaders,
+ };
+ };
+ browser.webRequest.onHeadersReceived.addListener(thirdOnHeadersRecievedListener, filter, headersReceivedInfoSpec);
+ await testCookiesWithFile("file_webrequestblocking_set_cookie.html", ["ext", "req", "third"]);
+ browser.webRequest.onHeadersReceived.removeListener(onHeadersReceived);
+ browser.webRequest.onHeadersReceived.removeListener(thirdOnHeadersRecievedListener);
+
+ // Fourth, test to make sure that extensions can remove cookies
+ // using onHeadersReceived too, by 1. making a request that
+ // sets a cookie (reqcookie=reqvalue), 2. having the extension remove
+ // that cookie by removing that header, and 3. adding a new cookie
+ // (extcookie=extvalue).
+ const fourthOnHeadersRecievedListener = details => {
+ // Remove the cookie set by the request (reqcookie=reqvalue).
+ const newHeaders = details.responseHeaders.filter(cookie => cookie.name !== "set-cookie");
+
+ // And then add a new cookie in its place (extcookie=extvalue).
+ newHeaders.push({
+ name: "Set-Cookie",
+ value: "extcookie=extvalue",
+ });
+
+ return {
+ responseHeaders: newHeaders,
+ };
+ };
+ browser.webRequest.onHeadersReceived.addListener(fourthOnHeadersRecievedListener, filter, headersReceivedInfoSpec);
+ await testCookiesWithFile("file_webrequestblocking_set_cookie.html", ["ext"]);
+ browser.webRequest.onHeadersReceived.removeListener(fourthOnHeadersRecievedListener);
+
+ // Fifth, check that extensions are able to overwrite headers set by
+ // pages. In this test, make a request that will set "reqcookie=reqvalue",
+ // and add a listener that sets "reqcookie=changedvalue". Check
+ // to make sure that the cookie jar contains "reqcookie=changedvalue"
+ // and not "reqcookie=reqvalue".
+ const fifthOnHeadersRecievedListener = details => {
+ // Remove the cookie set by the request (reqcookie=reqvalue).
+ const newHeaders = details.responseHeaders.filter(cookie => cookie.name !== "set-cookie");
+
+ // And then add a new cookie in its place (reqcookie=changedvalue).
+ newHeaders.push({
+ name: "Set-Cookie",
+ value: "reqcookie=changedvalue",
+ });
+
+ return {
+ responseHeaders: newHeaders,
+ };
+ };
+ browser.webRequest.onHeadersReceived.addListener(fifthOnHeadersRecievedListener, filter, headersReceivedInfoSpec);
+
+ await testCookiesWithFile("file_webrequestblocking_set_cookie.html", undefined, async tabDetails => {
+ const currentCookies = await browser.cookies.getAll({});
+ browser.test.assertEq(1, currentCookies.length, `1 cookie was set`);
+
+ const cookieName = "reqcookie";
+ const expectedCookieValue = "changedvalue";
+ const fetchedCookie = await browser.cookies.getAll({name: cookieName});
+
+ browser.test.assertEq(1, fetchedCookie.length, `Found 1 cookie with name "${cookieName}"`);
+ browser.test.assertEq(expectedCookieValue, fetchedCookie[0] && fetchedCookie[0].value, `Cookie "${cookieName}" has expected value of "${expectedCookieValue}"`);
+ });
+ browser.webRequest.onHeadersReceived.removeListener(fifthOnHeadersRecievedListener);
+ }
+
+ browser.test.notifyPass("cookie modifying extension");
+ }
+
+ extension = ExtensionTestUtils.loadExtension({
+ manifest: {
+ permissions: [
+ "browsingData",
+ "cookies",
+ "webNavigation",
+ "webRequest",
+ "webRequestBlocking",
+ "<all_urls>",
+ ],
+ },
+ background,
+ });
+
+ await extension.startup();
+ await extension.awaitFinish("cookie modifying extension");
+ await extension.unload();
+});
+
+</script>
+</body>
+</html>
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
@@ -272,39 +272,39 @@ ChannelWrapper::GetResponseHeaders(nsTAr
HeaderVisitor visitor(aRetVal, mContentTypeHdr);
visitor.VisitResponseHeaders(chan, aRv);
} else {
aRv.Throw(NS_ERROR_UNEXPECTED);
}
}
void
-ChannelWrapper::SetRequestHeader(const nsCString& aHeader, const nsCString& aValue, ErrorResult& aRv)
+ChannelWrapper::SetRequestHeader(const nsCString& aHeader, const nsCString& aValue, bool aMerge, ErrorResult& aRv)
{
nsresult rv = NS_ERROR_UNEXPECTED;
if (nsCOMPtr<nsIHttpChannel> chan = MaybeHttpChannel()) {
- rv = chan->SetRequestHeader(aHeader, aValue, false);
+ rv = chan->SetRequestHeader(aHeader, aValue, aMerge);
}
if (NS_FAILED(rv)) {
aRv.Throw(rv);
}
}
void
-ChannelWrapper::SetResponseHeader(const nsCString& aHeader, const nsCString& aValue, ErrorResult& aRv)
+ChannelWrapper::SetResponseHeader(const nsCString& aHeader, const nsCString& aValue, bool aMerge, ErrorResult& aRv)
{
nsresult rv = NS_ERROR_UNEXPECTED;
if (nsCOMPtr<nsIHttpChannel> chan = MaybeHttpChannel()) {
if (aHeader.LowerCaseEqualsLiteral("content-type")) {
rv = chan->SetContentType(aValue);
if (NS_SUCCEEDED(rv)) {
mContentTypeHdr = aValue;
}
} else {
- rv = chan->SetResponseHeader(aHeader, aValue, false);
+ rv = chan->SetResponseHeader(aHeader, aValue, aMerge);
}
}
if (NS_FAILED(rv)) {
aRv.Throw(rv);
}
}
/*****************************************************************************
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.h
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.h
@@ -215,19 +215,19 @@ public:
void GetRemoteAddress(nsCString& aRetVal) const;
void GetRequestHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const;
void GetResponseHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const;
- void SetRequestHeader(const nsCString& header, const nsCString& value, ErrorResult& aRv);
+ void SetRequestHeader(const nsCString& header, const nsCString& value, bool merge, ErrorResult& aRv);
- void SetResponseHeader(const nsCString& header, const nsCString& value, ErrorResult& aRv);
+ void SetResponseHeader(const nsCString& header, const nsCString& value, bool merge, ErrorResult& aRv);
using EventTarget::EventListenerAdded;
using EventTarget::EventListenerRemoved;
virtual void EventListenerAdded(nsAtom* aType) override;
virtual void EventListenerRemoved(nsAtom* aType) override;
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -119,47 +119,62 @@ class HeaderChanger {
// Remove missing headers.
let origHeaders = this.getMap();
for (let name of origHeaders.keys()) {
if (!newHeaders.has(name)) {
this.setHeader(name, "");
}
}
- // Set new or changed headers.
+ // Set new or changed headers. If there are multiple headers with the same
+ // name (e.g. Set-Cookie), merge them, instead of having new values
+ // overwrite previous ones.
+ //
+ // When the new value of a header is equal the existing value of the header
+ // (e.g. the initial response set "Set-Cookie: examplename=examplevalue",
+ // and an extension also added the header
+ // "Set-Cookie: examplename=examplevalue") then the header value is not
+ // re-set, but subsequent headers of the same type will be merged in.
+ let headersAlreadySet = new Set();
for (let {name, value, binaryValue} of headers) {
if (binaryValue) {
value = String.fromCharCode(...binaryValue);
}
- let original = origHeaders.get(name.toLowerCase());
+
+ let lowerCaseName = name.toLowerCase();
+ let original = origHeaders.get(lowerCaseName);
+
if (!original || value !== original.value) {
- this.setHeader(name, value);
+ let shouldMerge = headersAlreadySet.has(lowerCaseName);
+ this.setHeader(name, value, shouldMerge);
}
+
+ headersAlreadySet.add(lowerCaseName);
}
}
}
class RequestHeaderChanger extends HeaderChanger {
- setHeader(name, value) {
+ setHeader(name, value, merge) {
try {
- this.channel.setRequestHeader(name, value);
+ this.channel.setRequestHeader(name, value, merge);
} catch (e) {
Cu.reportError(new Error(`Error setting request header ${name}: ${e}`));
}
}
readHeaders() {
return this.channel.getRequestHeaders();
}
}
class ResponseHeaderChanger extends HeaderChanger {
- setHeader(name, value) {
+ setHeader(name, value, merge) {
try {
- this.channel.setResponseHeader(name, value);
+ this.channel.setResponseHeader(name, value, merge);
} catch (e) {
Cu.reportError(new Error(`Error setting response header ${name}: ${e}`));
}
}
readHeaders() {
return this.channel.getResponseHeaders();
}