Bug 1377689 - linting corrections, added tests for server not setting cookies and multiple listiners, r=mixedpuppy,kmag
MozReview-Commit-ID: 7wdZTWouX6u
--- 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 (e.g. handling multiple
+ * Set-Cookie headers).
*
* For non-HTTP requests, throws NS_ERROR_UNEXPECTED.
*/
[Throws]
- void setRequestHeader(ByteString header, ByteString value, optional boolean merge = false);
+ 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. Otherwise, any
+ * prior value for the header will be overwritten (e.g. handling multiple
+ * Set-Cookie headers).
*
* 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, optional boolean merge = false);
+ 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,10 @@
+<!DOCTYPE html>
+<html lang="en">
+ <head>
+ <meta charset="utf-8">
+ <title>Cookie Example</title>
+ </head>
+ <body>
+ <p>This request should set two cookies.</p>
+ </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
@@ -50,17 +52,16 @@ support-files =
file_privilege_escalation.html
file_ext_test_api_injection.js
file_permission_xhr.html
file_teardown_test.js
lorem.html.gz
lorem.html.gz^headers^
return_headers.sjs
slow_response.sjs
- set_cookies.sjs
webrequest_worker.js
oauth.html
!/toolkit/components/passwordmgr/test/authenticate.sjs
!/dom/tests/mochitest/geolocation/network_geolocation.sjs
[test_ext_clipboard.html]
[test_ext_clipboard_image.html]
skip-if = headless # disabled test case with_permission_allow_copy, see inline comment. Headless: Bug 1405872
@@ -78,18 +79,18 @@ skip-if = (os == 'linux' && debug) || (t
[test_ext_contentscript_context.html]
[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]
-skip-if = os == 'android' # bug 1369440
[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]
deleted file mode 100644
--- a/toolkit/components/extensions/test/mochitest/set_cookies.sjs
+++ /dev/null
@@ -1,20 +0,0 @@
-const returnedHtml = `
-<!doctype html>
-<html lang="en">
- <head>
- <meta charset="utf-8">
- <title>Cookie Example</title>
- </head>
- <body>
- <p>This request should set two cookies.</p>
- </body>
-</html>
-`;
-
-function handleRequest(request, response) {
-
- response.setHeader("Content-Type", "text/html;charset=utf-8");
- response.setHeader("Cache-Control", "no-cache");
- response.setHeader("Set-Cookie", "cookie1=value1");
- response.write(returnedHtml);
-}
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequestblocking_set_cookie.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequestblocking_set_cookie.html
@@ -8,100 +8,126 @@
<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";
-let extension;
-add_task(async function setup() {
+add_task(async function test_modifying_cookies_from_onHeadersReceived() {
+ let extension;
async function background() {
+ /**
+ * Check that all the cookies described by `prefixes` are in the cookie jar.
+ */
+ async function checkCookies(...prefixes) {
+ let numPrefixes = prefixes.length;
+ let currentCookies = await browser.cookies.getAll({});
+ browser.test.assertEq(numPrefixes, currentCookies.length, `${numPrefixes} cookies were set`);
- async function openWindowAndTab(url) {
+ 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}"`);
+ }
+ }
- let tabReadyPromise = new Promise((resolve) => {
+ 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: url,
+ urlPrefix: fileUrl,
}],
});
});
- // This tab is opened for two purposes:
- // 1. To allow tests to run content scripts in the context of a tab,
- // for fetching the value of document.cookie.
- // 2. To work around bug 1309637, based on the analysis in comment 8.
- let {id: windowId} = await browser.windows.create({url});
- let tabId = await tabReadyPromise;
+ const {id: windowId} = await browser.windows.create({url: fileUrl});
+ const tabId = await tabReadyPromise;
return {windowId, tabId};
}
const requestFilter = {
urls: ["<all_urls>"],
- types: ["main_frame", "sub_frame"]
+ types: ["main_frame", "sub_frame"],
};
const extraInfoSpec = ["blocking", "responseHeaders"];
- const onHeadersReceived = function (details) {
-
+ const onHeadersReceived = details => {
details.responseHeaders.push({
name: "Set-Cookie",
- value: "cookie2=value2"
+ value: "extcookie=extvalue",
});
return {
- responseHeaders: details.responseHeaders
+ responseHeaders: details.responseHeaders,
};
};
-
- browser.webRequest.onHeadersReceived.addListener(
- onHeadersReceived,
- requestFilter,
- extraInfoSpec
- );
+ browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, requestFilter, extraInfoSpec);
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 browser.browsingData.removeCookies({});
- let childTabDetails = await openWindowAndTab("http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/set_cookies.sjs");
+ const tabDetails = await openWindowAndTab("file_sample.html");
+ await checkCookies("ext");
+ await browser.windows.remove(tabDetails.windowId);
- let testCookies = await browser.cookies.getAll({});
- browser.test.assertEq(2, testCookies.length, "3 cookies were set (two by the request, one by the extension)");
+ // 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 browser.browsingData.removeCookies({});
+ const cookieSettingTabDetails = await openWindowAndTab("file_webrequestblocking_set_cookie.html");
+ await checkCookies("ext", "req");
+ await browser.windows.remove(cookieSettingTabDetails.windowId);
- for (let i = 1; i <= 2; i += 1) {
- let cookieName =`cookie${i}`;
- let expectedCookieValue = `value${i}`;
- let aCookie = await browser.cookies.getAll({name: cookieName});
- browser.test.assertEq(1, aCookie.length, `Found cookie 1 cookie with name ${cookieName}`);
- browser.test.assertEq(expectedCookieValue, aCookie[0] && aCookie[0].value, `Cookie ${cookieName} has expected value of ${expectedCookieValue}`);
- }
+ // Third, register another onHeadersReceived handler that also
+ // sets a cookie (thirdcookie=thirdvalue), to make sure modifications from
+ // multiple onHeadersReceived listeners are merged correctly.
+ const secondOnHeadersRecievedListiner = details => {
+ details.responseHeaders.push({
+ name: "Set-Cookie",
+ value: "thirdcookie=thirdvalue",
+ });
- await browser.windows.remove(childTabDetails.windowId);
+ return {
+ responseHeaders: details.responseHeaders,
+ };
+ };
+ browser.webRequest.onHeadersReceived.addListener(secondOnHeadersRecievedListiner, requestFilter, extraInfoSpec);
+
+ await browser.browsingData.removeCookies({});
+ const secondCookieSettingTabDetails = await openWindowAndTab("file_webrequestblocking_set_cookie.html");
+ await checkCookies("ext", "req", "third");
+ await browser.windows.remove(secondCookieSettingTabDetails.windowId);
}
browser.test.notifyPass("cookie modifying extension");
- };
+ }
extension = ExtensionTestUtils.loadExtension({
manifest: {
permissions: [
- "webRequest",
+ "browsingData",
"cookies",
- "browsingData",
"webNavigation",
+ "webRequest",
"webRequestBlocking",
"<all_urls>",
],
},
- background
+ background,
});
await extension.startup();
await extension.awaitFinish("cookie modifying extension");
await extension.unload();
});
</script>
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -119,34 +119,34 @@ 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. If there are two headers with the same
- // name (e.g. Set-Cookie), tell setHeader to merge them, instead of having
+ // 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.
let headersAlreadySet = new Set();
for (let {name, value, binaryValue} of headers) {
if (binaryValue) {
value = String.fromCharCode(...binaryValue);
}
- let lowerCaseHeaderName = name.toLowerCase();
- let original = origHeaders.get(lowerCaseHeaderName);
+ let lowerCaseName = name.toLowerCase();
+ let original = origHeaders.get(lowerCaseName);
if (!original || value !== original.value) {
- let shouldMerge = headersAlreadySet.has(lowerCaseHeaderName);
+ let shouldMerge = headersAlreadySet.has(lowerCaseName);
this.setHeader(name, value, shouldMerge);
}
- headersAlreadySet.add(lowerCaseHeaderName);
+ headersAlreadySet.add(lowerCaseName);
}
}
}
class RequestHeaderChanger extends HeaderChanger {
setHeader(name, value, merge) {
try {
this.channel.setRequestHeader(name, value, merge);