Bug 1254204: Part 1 - Apply WebRequest header changes differentially, after all listeners have run in parallel. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Wed, 09 Nov 2016 13:36:54 -0800
changeset 351996 30aa827dc7849d11478f2bcc821825c324f41de8
parent 351995 43835f5fa2b217ba7e2a59fb9f52910a2e06a28a
child 351997 07b39b68b0844b18d16747dd9a9502d47d2c487a
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1254204
milestone52.0a1
Bug 1254204: Part 1 - Apply WebRequest header changes differentially, after all listeners have run in parallel. r=mixedpuppy MozReview-Commit-ID: Jk1ja5Y3lMI
toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
toolkit/modules/addons/WebRequest.jsm
--- 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,125 @@ 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)}`);
+      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) {
+      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) {
+      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 +475,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 +567,37 @@ 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) {
+      // Just ignore this for the request phases where response headers
+      // aren't yet available.
+    }
+
     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 +637,73 @@ 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;
       }
+
       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);