Bug 1254204: Part 1 - Apply WebRequest header changes differentially, after all listeners have run in parallel. r?aswan
MozReview-Commit-ID: Jk1ja5Y3lMI
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
@@ -165,48 +165,48 @@ function backgroundScript() {
};
let testHeaders = {
request: {
added: {
"X-WebRequest-request": "text",
"X-WebRequest-request-binary": "binary",
},
modified: {
- "User-Agent": "WebRequest",
+ "user-agent": "WebRequest",
},
deleted: [
- "Referer",
+ "referer",
],
},
response: {
added: {
"X-WebRequest-response": "text",
"X-WebRequest-response-binary": "binary",
},
modified: {
- "Server": "WebRequest",
- "Content-Type": "text/html; charset=utf-8",
+ "server": "WebRequest",
+ "content-type": "text/html; charset=utf-8",
},
deleted: [
- "Connection",
+ "connection",
],
},
};
function checkResourceType(type) {
let key = type.toUpperCase();
browser.test.assertTrue(key in browser.webRequest.ResourceType, `valid resource type ${key}`);
}
function processHeaders(phase, details) {
let headers = details[`${phase}Headers`];
browser.test.assertTrue(Array.isArray(headers), `${phase}Headers array present`);
- let processedMark = "WebRequest-processed";
- if (headers.find(h => h.name === processedMark)) {
+ let processedMark = "webrequest-processed";
+ if (headers.find(h => h.name.toLowerCase() === processedMark)) {
// This may happen because of redirections or cache
browser.test.log(`${phase}Headers in ${details.requestId} already processed`);
skippedRequests.add(details.requestId);
return null;
}
headers.push({name: processedMark, value: "1"});
let {added, modified, deleted} = testHeaders[phase];
@@ -218,25 +218,27 @@ function backgroundScript() {
header.binaryValue = Array.from(added[name], c => c.charCodeAt(0));
} else {
header.value = added[name];
}
headers.push(header);
}
let modifiedAny = false;
- for (let header of headers.filter(h => h.name in modified)) {
- header.value = modified[header.name];
- modifiedAny = true;
+ for (let header of headers) {
+ if (header.name.toLowerCase() in modified) {
+ header.value = modified[header.name.toLowerCase()];
+ modifiedAny = true;
+ }
}
browser.test.assertTrue(modifiedAny, `at least one ${phase}Headers element to modify`);
let deletedAny = false;
for (let j = headers.length; j-- > 0;) {
- if (deleted.includes(headers[j].name)) {
+ if (deleted.includes(headers[j].name.toLowerCase())) {
headers.splice(j, 1);
deletedAny = true;
}
}
browser.test.assertTrue(deletedAny, `at least one ${phase}Headers element to delete`);
return headers;
}
@@ -246,17 +248,17 @@ function backgroundScript() {
return;
}
let headers = details[`${phase}Headers`];
browser.test.assertTrue(Array.isArray(headers), `valid ${phase}Headers array`);
let {added, modified, deleted} = testHeaders[phase];
for (let name in added) {
- browser.test.assertTrue(headers.some(h => h.name === name && h.value === added[name]), `header ${name} correctly injected in ${phase}Headers`);
+ browser.test.assertTrue(headers.some(h => h.name.toLowerCase() === name.toLowerCase() && h.value === added[name]), `header ${name} correctly injected in ${phase}Headers`);
}
let modifiedAny = false;
browser.test.log(`HEADERS ${JSON.stringify(headers)}`);
for (let header of headers.filter(h => h.name in modified)) {
let {name, value} = header;
if (name.toLowerCase() === "content-type" && skippedRequests.has(details.requestId)) {
// Changes to Content-Type headers are not persisted in the cache.
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -110,16 +110,128 @@ function mergeStatus(data, channel, even
// NS_ERROR_NOT_AVAILABLE might be thrown if it's an internal redirect, happening before
// any actual HTTP traffic. Otherwise, let's report.
if (event !== "onRedirect" || e.result !== Cr.NS_ERROR_NOT_AVAILABLE) {
Cu.reportError(`webRequest Error: ${e} trying to merge status in ${event}@${channel.name}`);
}
}
}
+class HeaderChanger {
+ constructor(channel) {
+ this.channel = channel;
+
+ this.originalHeaders = new Map();
+ this.visitHeaders((name, value) => {
+ this.originalHeaders.set(name.toLowerCase(), value);
+ });
+ }
+
+ toArray() {
+ return Array.from(this.originalHeaders,
+ ([name, value]) => ({name, value}));
+ }
+
+ validateHeaders(headers) {
+ // We should probably use schema validation for this.
+
+ if (!Array.isArray(headers)) {
+ return false;
+ }
+
+ return headers.every(header => {
+ if (typeof header !== "object" || header === null) {
+ return false;
+ }
+
+ if (typeof header.name !== "string") {
+ return false;
+ }
+
+ return (typeof header.value === "string" ||
+ Array.isArray(header.binaryValue));
+ });
+ }
+
+ applyChanges(headers) {
+ if (!this.validateHeaders(headers)) {
+ /* globals uneval */
+ Cu.reportError(`Invalid header array: ${uneval(headers)}`);
+ dump(`Invalid header array: ${uneval(headers)}\n`);
+ return;
+ }
+
+ let newHeaders = new Set(headers.map(
+ ({name}) => name.toLowerCase()));
+
+ // Remove missing headers.
+ for (let name of this.originalHeaders.keys()) {
+ if (!newHeaders.has(name)) {
+ this.setHeader(name, "");
+ }
+ }
+
+ // Set new or changed headers.
+ for (let {name, value, binaryValue} of headers) {
+ if (binaryValue) {
+ value = String.fromCharCode(...binaryValue);
+ }
+ if (value !== this.originalHeaders.get(name.toLowerCase())) {
+ this.setHeader(name, value);
+ }
+ }
+ }
+}
+
+class RequestHeaderChanger extends HeaderChanger {
+ setHeader(name, value) {
+ try {
+ this.channel.setRequestHeader(name, value, false);
+ } catch (e) {
+ dump(`Error setting request header ${name}: ${e}\n`);
+ Cu.reportError(new Error(`Error setting request header ${name}: ${e}`));
+ }
+ }
+
+ visitHeaders(visitor) {
+ this.channel.visitRequestHeaders(visitor);
+ }
+}
+
+class ResponseHeaderChanger extends HeaderChanger {
+ setHeader(name, value) {
+ try {
+ if (name.toLowerCase() === "content-type" && value) {
+ // The Content-Type header value can't be modified, so we
+ // set the channel's content type directly, instead, and
+ // record that we made the change for the sake of
+ // subsequent observers.
+ this.channel.contentType = value;
+
+ getData(this.channel).contentType = value;
+ } else {
+ this.channel.setResponseHeader(name, value, false);
+ }
+ } catch (e) {
+ dump(`Error setting response header ${name}: ${e}\n`);
+ Cu.reportError(new Error(`Error setting response header ${name}: ${e}`));
+ }
+ }
+
+ visitHeaders(visitor) {
+ this.channel.visitResponseHeaders((name, value) => {
+ if (name.toLowerCase() === "content-type") {
+ value = getData(this.channel).contentType || value;
+ }
+
+ visitor(name, value);
+ });
+ }
+}
+
var HttpObserverManager;
var ContentPolicyManager = {
policyData: new Map(),
policies: new Map(),
idMap: new Map(),
nextId: 0,
@@ -366,71 +478,16 @@ HttpObserverManager = {
.notificationCallbacks
.getInterface(Components.interfaces.nsILoadContext);
} catch (e) {
return null;
}
}
},
- getHeaders(channel, method, event) {
- let headers = [];
- let visitor = {
- visitHeader(name, value) {
- try {
- value = channel.getProperty(`webrequest-header-${name.toLowerCase()}`);
- } catch (e) {
- // This will throw if the property does not exist.
- }
- headers.push({name, value});
- },
-
- QueryInterface: XPCOMUtils.generateQI([Ci.nsIHttpHeaderVisitor,
- Ci.nsISupports]),
- };
-
- try {
- channel.QueryInterface(Ci.nsIPropertyBag);
- channel[method](visitor);
- } catch (e) {
- Cu.reportError(`webRequest Error: ${e} trying to perform ${method} in ${event}@${channel.name}`);
- }
- return headers;
- },
-
- replaceHeaders(headers, originalNames, setHeader) {
- let failures = new Set();
- // Start by clearing everything.
- for (let name of originalNames) {
- try {
- setHeader(name, "");
- } catch (e) {
- // Let's collect physiological failures in order
- // to know what is worth reporting.
- failures.add(name);
- }
- }
- try {
- for (let {name, value, binaryValue} of headers) {
- try {
- if (Array.isArray(binaryValue)) {
- value = String.fromCharCode.apply(String, binaryValue);
- }
- setHeader(name, value);
- } catch (e) {
- if (!failures.has(name)) {
- Cu.reportError(e);
- }
- }
- }
- } catch (e) {
- Cu.reportError(e);
- }
- },
-
observe(subject, topic, data) {
let channel = subject.QueryInterface(Ci.nsIHttpChannel);
switch (topic) {
case "http-on-modify-request":
this.modify(channel, topic, data);
break;
case "http-on-examine-cached-response":
case "http-on-examine-merged-response":
@@ -513,30 +570,36 @@ HttpObserverManager = {
}
let listeners = this.listeners[kind];
let browser = loadContext ? loadContext.topFrameElement : null;
let loadInfo = channel.loadInfo;
let policyType = loadInfo ?
loadInfo.externalContentPolicyType :
Ci.nsIContentPolicy.TYPE_OTHER;
- let requestHeaderNames;
- let responseHeaderNames;
-
let requestBody;
let includeStatus = (
kind === "headersReceived" ||
kind === "onRedirect" ||
kind === "onStart" ||
kind === "onStop"
) && channel instanceof Ci.nsIHttpChannel;
+ let requestHeaders = new RequestHeaderChanger(channel);
+ let responseHeaders;
+ try {
+ responseHeaders = new ResponseHeaderChanger(channel);
+ } catch (e) {
+ // Meh.
+ }
+
let commonData = null;
let uri = channel.URI;
+ let handlerResults = [];
for (let [callback, opts] of listeners.entries()) {
if (!this.shouldRunListener(policyType, uri, opts.filter)) {
continue;
}
if (!commonData) {
commonData = {
requestId: RequestId.get(channel),
@@ -576,80 +639,74 @@ HttpObserverManager = {
// The remoteAddress getter throws if the address is unavailable,
// but ip is an optional property so just ignore the exception.
}
}
if (extraData) {
Object.assign(commonData, extraData);
}
}
+
let data = Object.assign({}, commonData);
+
if (opts.requestHeaders) {
- data.requestHeaders = this.getHeaders(channel, "visitRequestHeaders", kind);
- requestHeaderNames = data.requestHeaders.map(h => h.name);
+ data.requestHeaders = requestHeaders.toArray();
}
- if (opts.responseHeaders) {
- data.responseHeaders = this.getHeaders(channel, "visitResponseHeaders", kind);
- responseHeaderNames = data.responseHeaders.map(h => h.name);
+
+ if (opts.responseHeaders && responseHeaders) {
+ data.responseHeaders = responseHeaders.toArray();
}
+
if (opts.requestBody) {
if (requestBody === undefined) {
requestBody = WebRequestUpload.createRequestBody(channel);
}
if (requestBody) {
data.requestBody = requestBody;
}
}
+
if (includeStatus) {
mergeStatus(data, channel, kind);
}
- let result = null;
try {
- result = callback(data);
+ let result = callback(data);
+
+ if (result && typeof result === "object" && opts.blocking) {
+ handlerResults.push({opts, result});
+ }
} catch (e) {
Cu.reportError(e);
}
+ }
- if (!result || !opts.blocking) {
- continue;
- }
+ for (let {opts, result} of handlerResults) {
if (result.cancel) {
channel.cancel(Cr.NS_ERROR_ABORT);
this.errorCheck(channel, loadContext);
return false;
}
+
+ // FIXME: This should only be available in some phases.
if (result.redirectUrl) {
- channel.redirectTo(BrowserUtils.makeURI(result.redirectUrl));
- return false;
+ try {
+ channel.redirectTo(BrowserUtils.makeURI(result.redirectUrl));
+ return false;
+ } catch (e) {
+ Cu.reportError(e);
+ }
}
+
if (opts.requestHeaders && result.requestHeaders) {
- this.replaceHeaders(
- result.requestHeaders, requestHeaderNames,
- (name, value) => channel.setRequestHeader(name, value, false)
- );
+ requestHeaders.applyChanges(result.requestHeaders);
}
+
if (opts.responseHeaders && result.responseHeaders) {
- this.replaceHeaders(
- result.responseHeaders, responseHeaderNames,
- (name, value) => {
- if (name.toLowerCase() === "content-type" && value) {
- // The Content-Type header value can't be modified, so we
- // set the channel's content type directly, instead, and
- // record that we made the change for the sake of
- // subsequent observers.
- channel.contentType = value;
-
- channel.QueryInterface(Ci.nsIWritablePropertyBag);
- channel.setProperty("webrequest-header-content-type", value);
- } else {
- channel.setResponseHeader(name, value, false);
- }
- }
- );
+ responseHeaders.applyChanges(result.responseHeaders);
}
}
return true;
},
modify(channel, topic, data) {
let loadContext = this.getLoadContext(channel);