Remove onHeadersReceived listener

semenko · diracdeltas · commit db744f562938 · 2013-12-31T00:15:32.000-08:00
The onHeadersReceived listener served one purpose:
Secure cookies set via HTTPS (not HTTP, not JS)

This task is already handled by:
- onBeforeSendHeaders (blocking) which should secure cookies
- onCookieChanged (non-blocking) which secures any cookies on create/update

Moreover, the current function is broken:
- We look for a case-sensitive "Set-Cookie," but many servers send only a
lowercase "set-cookie" (e.g. Google, Twitter, ...)
- "; Secure" is frequently not the end of the cookie (old #L192)

By removing this function, here's what happens for different cookie types:

- Set by HTTP
No change, wasn't ever handled by onHeadersReceived

- Set by Javascript
No change, wasn't ever handled by onHeadersReceived

- Set by HTTPS
Now (as before) secured by onCookieChanged (non-blocking, so maybe a tiny
window for a race to steal the cookie, but ...)
Secured by onBeforeSendHeaders (blocking)

Signed-off-by: Nick Semenkovich &lt;semenko@alum.mit.edu&gt;
diff --git a/chromium/background.js b/chromium/background.js
@@ -177,35 +177,6 @@ function onCookieChanged(changeInfo) {
   }
 }
 
-// Check to see if a newly set cookie in an HTTP request should be secured
-function onHeadersReceived(details) {
-  var a = document.createElement("a");  // hack to parse URLs
-  a.href = details.url;                 //
-  var host = a.hostname;
-
-  // TODO: Verify this with wireshark
-  for (var h in details.responseHeaders) {
-    if (details.responseHeaders[h].name == "Set-Cookie") {
-      log(INFO,"Deciding whether to secure cookies in " + details.url);
-      var cookie = details.responseHeaders[h].value;
-
-      if (cookie.indexOf("; Secure") == -1) {
-        log(INFO, "Got insecure cookie header: "+cookie);
-        // Create a fake "nsICookie2"-ish object to pass in to our rule API:
-        var fake = {domain:host, name:cookie.split("=")[0]};
-        var ruleset = all_rules.shouldSecureCookie(fake, true);
-        if (ruleset) {
-          activeRulesets.addRulesetToTab(details.tabId, ruleset);
-          details.responseHeaders[h].value = cookie+"; Secure";
-          log(INFO, "Secured cookie: "+details.responseHeaders[h].value);
-        }
-      }
-    }
-  }
-
-  return {responseHeaders:details.responseHeaders};
-}
-
 // This event is needed due to the potential race between cookie permissions
 // update and cookie transmission, because the cookie API is non-blocking.
 // It would be less perf impact to have a blocking version of the cookie API
@@ -279,11 +250,6 @@ wr.onBeforeRequest.addListener(onBeforeRequest, {urls: ["https://*/*", "http://*
 wr.onBeforeSendHeaders.addListener(onBeforeSendHeaders, {urls: ["http://*/*"]},
                                    ["requestHeaders", "blocking"]);
 
-// This parses HTTPS cookies and may set their secure flag.
-// We never do this for cookies set by HTTP.
-wr.onHeadersReceived.addListener(onHeadersReceived, {urls: ["https://*/*"]},
-                                    ["responseHeaders", "blocking"]);
-
 wr.onResponseStarted.addListener(onResponseStarted,
                                  {urls: ["https://*/*", "http://*/*"]});
 wr.onErrorOccurred.addListener(onErrorOccurred,
