Bug 1377689 - merge identical headers in set{Request,Response}Header, r=bz,mixedpuppy

MozReview-Commit-ID: Kpli9YzEvlt

--HG--
extra : rebase_source : 717c44a3b4b55f3ff0fa40ddb4778bca5265b8d4
This commit is contained in:
Peter Snyder 2017-12-04 22:48:54 -06:00
parent 0ef9411420
commit 28001d0dce
8 changed files with 293 additions and 17 deletions

View File

@ -330,17 +330,25 @@ interface ChannelWrapper : EventTarget {
/**
* 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.
*
@ -348,7 +356,9 @@ interface ChannelWrapper : EventTarget {
* getResponseHeaders() for details.
*/
[Throws]
void setResponseHeader(ByteString header, ByteString value);
void setResponseHeader(ByteString header,
ByteString value,
optional boolean merge = false);
};
/**

View File

@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Set Cookie Example</title>
</head>
<body>
</body>
</html>

View File

@ -0,0 +1 @@
Set-Cookie: reqcookie=reqvalue

View File

@ -18,6 +18,8 @@ support-files =
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
@ -83,6 +85,7 @@ 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]

View File

@ -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>

View File

@ -277,11 +277,11 @@ ChannelWrapper::GetResponseHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorR
}
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);
@ -289,7 +289,7 @@ ChannelWrapper::SetRequestHeader(const nsCString& aHeader, const nsCString& aVal
}
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()) {
@ -299,7 +299,7 @@ ChannelWrapper::SetResponseHeader(const nsCString& aHeader, const nsCString& aVa
mContentTypeHdr = aValue;
}
} else {
rv = chan->SetResponseHeader(aHeader, aValue, false);
rv = chan->SetResponseHeader(aHeader, aValue, aMerge);
}
}
if (NS_FAILED(rv)) {

View File

@ -220,9 +220,9 @@ public:
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;

View File

@ -124,23 +124,38 @@ class HeaderChanger {
}
}
// 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}`));
}
@ -152,9 +167,9 @@ class RequestHeaderChanger extends HeaderChanger {
}
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}`));
}