Bug 1460609 - Cookies are for parents r=mayhemer

Make sure cookies aren't saved on channel headers in the content process.
Adds test to verify that this works, and removes tests that expected cookie headers to be visible to the child.

MozReview-Commit-ID: KOB83xpuAlF

--HG--
extra : rebase_source : 6f9a5ef570fb23200acf8d75285e67d80b7c27f0

netwerk/protocol/http/HttpChannelChild.cpp

@@ -582,6 +582,10 @@ HttpChannelChild::OnStartRequest(const nsresult& channelStatus,
     mStatus = channelStatus;
   }
 
+  // Cookies headers should not be visible to the child process
+  MOZ_ASSERT(!requestHeaders.HasHeader(nsHttp::Cookie));
+  MOZ_ASSERT(!nsHttpResponseHead(responseHead).HasHeader(nsHttp::Set_Cookie));
+
   if (useResponseHead && !mCanceled)
     mResponseHead = new nsHttpResponseHead(responseHead);
 
@@ -1666,6 +1670,9 @@ HttpChannelChild::RecvRedirect1Begin(const uint32_t& registrarId,
   // Then it will be updated to new peer in OnStartRequest
   mPeerAddr = oldPeerAddr;
 
+  // Cookies headers should not be visible to the child process
+  MOZ_ASSERT(!nsHttpResponseHead(responseHead).HasHeader(nsHttp::Set_Cookie));
+
   mEventQ->RunOrEnqueue(new Redirect1Event(this, registrarId, newUri,
                                            redirectFlags, loadInfoForwarder,
                                            responseHead, securityInfoSerialization,

netwerk/protocol/http/HttpChannelParent.cpp

@@ -1383,8 +1383,6 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext)
     MOZ_ASSERT(NS_SUCCEEDED(rv));
   }
 
-  nsHttpResponseHead *responseHead = chan->GetResponseHead();
-  nsHttpRequestHead  *requestHead = chan->GetRequestHead();
   bool isFromCache = false;
   uint64_t cacheEntryId = 0;
   int32_t fetchCount = 0;
@@ -1460,14 +1458,37 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext)
   ParentLoadInfoForwarderArgs loadInfoForwarderArg;
   mozilla::ipc::LoadInfoToParentLoadInfoForwarder(loadInfo, &loadInfoForwarderArg);
 
+  nsHttpResponseHead *responseHead = chan->GetResponseHead();
+  bool useResponseHead = !!responseHead;
+  nsHttpResponseHead cleanedUpResponseHead;
+  if (responseHead && responseHead->HasHeader(nsHttp::Set_Cookie)) {
+    cleanedUpResponseHead = *responseHead;
+    cleanedUpResponseHead.ClearHeader(nsHttp::Set_Cookie);
+    responseHead = &cleanedUpResponseHead;
+  }
+
+  if (!responseHead) {
+    responseHead = &cleanedUpResponseHead;
+  }
+
+  nsHttpRequestHead *requestHead = chan->GetRequestHead();
   // !!! We need to lock headers and please don't forget to unlock them !!!
   requestHead->Enter();
+
+  nsHttpHeaderArray cleanedUpRequestHeaders;
+  bool cleanedUpRequest = false;
+  if (requestHead->HasHeader(nsHttp::Cookie)) {
+    cleanedUpRequestHeaders = requestHead->Headers();
+    cleanedUpRequestHeaders.ClearHeader(nsHttp::Cookie);
+    cleanedUpRequest = true;
+  }
+
   rv = NS_OK;
   if (mIPCClosed ||
       !SendOnStartRequest(channelStatus,
-                          responseHead ? *responseHead : nsHttpResponseHead(),
-                          !!responseHead,
-                          requestHead->Headers(),
+                          *responseHead,
+                          useResponseHead,
+                          cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(),
                           loadInfoForwarderArg,
                           isFromCache,
                           mCacheEntry ? true : false,
@@ -1840,12 +1861,23 @@ HttpChannelParent::StartRedirect(uint32_t registrarId,
   mozilla::ipc::LoadInfoToParentLoadInfoForwarder(loadInfo, &loadInfoForwarderArg);
 
   nsHttpResponseHead *responseHead = mChannel->GetResponseHead();
+
+  nsHttpResponseHead cleanedUpResponseHead;
+  if (responseHead && responseHead->HasHeader(nsHttp::Set_Cookie)) {
+    cleanedUpResponseHead = *responseHead;
+    cleanedUpResponseHead.ClearHeader(nsHttp::Set_Cookie);
+    responseHead = &cleanedUpResponseHead;
+  }
+
+  if (!responseHead) {
+    responseHead = &cleanedUpResponseHead;
+  }
+
   bool result = false;
   if (!mIPCClosed) {
     result = SendRedirect1Begin(registrarId, uriParams, redirectFlags,
                                 loadInfoForwarderArg,
-                                responseHead ? *responseHead
-                                             : nsHttpResponseHead(),
+                                *responseHead,
                                 secInfoSerialization,
                                 channelId,
                                 mChannel->GetPeerAddr());

netwerk/test/unit_ipc/child_cookie_header.js

@@ -0,0 +1,77 @@
+"use strict";
+
+ChromeUtils.import("resource://testing-common/httpd.js");
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
+
+
+function inChildProcess() {
+  return Cc["@mozilla.org/xre/app-info;1"]
+           .getService(Ci.nsIXULRuntime)
+           .processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT;
+}
+
+let URL = null;
+function makeChan() {
+  return NetUtil.newChannel({uri: URL, loadUsingSystemPrincipal: true})
+                .QueryInterface(Ci.nsIHttpChannel);
+}
+
+function OpenChannelPromise(aChannel, aClosure) {
+  return new Promise(resolve => {
+    function processResponse(request, buffer, context)
+    {
+      aClosure(request.QueryInterface(Ci.nsIHttpChannel), buffer, context);
+      resolve();
+    }
+    aChannel.asyncOpen2(new ChannelListener(processResponse, null));
+  });
+}
+
+// This test doesn't do much, except to communicate with the parent, and get
+// URL we need to connect to.
+add_task(async function setup() {
+  ok(inChildProcess(), "Sanity check. This should run in the child process");
+  // Initialize the URL. Parent runs the server
+  do_send_remote_message("start-test");
+  URL = await do_await_remote_message("start-test-done");
+});
+
+// This test performs a request, and checks that no cookie header are visible
+// to the child process
+add_task(async function test1() {
+  let chan = makeChan();
+
+  await OpenChannelPromise(chan, (request, buffer) => {
+    equal(buffer, "response");
+    Assert.throws(() => request.getRequestHeader("Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on request in the child");
+    Assert.throws(() => request.getResponseHeader("Set-Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on response in the child");
+  });
+
+  // We also check that a cookie was saved by the Set-Cookie header
+  // in the parent.
+  do_send_remote_message("check-cookie-count");
+  let count = await do_await_remote_message("check-cookie-count-done");
+  equal(count, 1);
+});
+
+// This test communicates with the parent, to locally save a new cookie.
+// Then it performs another request, makes sure no cookie headers are visible,
+// after which it checks that both cookies are visible to the parent.
+add_task(async function test2() {
+  do_send_remote_message("set-cookie");
+  await do_await_remote_message('set-cookie-done');
+
+  let chan = makeChan();
+  await OpenChannelPromise(chan, (request, buffer) => {
+    equal(buffer, "response");
+    Assert.throws(() => request.getRequestHeader("Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on request in the child");
+    Assert.throws(() => request.getResponseHeader("Set-Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on response in the child");
+  });
+
+  // We should have two cookies. One set by the Set-Cookie header sent by the
+  // server, and one that was manually set in the parent.
+  do_send_remote_message("second-check-cookie-count");
+  let count = await do_await_remote_message("second-check-cookie-count-done");
+  equal(count, 2);
+});

netwerk/test/unit_ipc/test_bug248970_cookie_wrap.js

@@ -1,7 +0,0 @@
-ChromeUtils.import("resource://gre/modules/Services.jsm");
-
-function run_test() {
-  // Allow all cookies.
-  Services.prefs.setIntPref("network.cookie.cookieBehavior", 0);
-  run_test_in_child("../unit/test_bug248970_cookie.js");
-}

netwerk/test/unit_ipc/test_cookie_header_stripped.js

@@ -0,0 +1,68 @@
+"use strict";
+
+ChromeUtils.import("resource://testing-common/httpd.js");
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+
+const TEST_DOMAIN = "www.example.com";
+XPCOMUtils.defineLazyGetter(this, "URL", function() {
+  return "http://" + TEST_DOMAIN +":" + httpserv.identity.primaryPort + "/path";
+});
+
+const responseBody1 = "response";
+function requestHandler(metadata, response)
+{
+  response.setHeader("Content-Type", "text/plain");
+  response.setHeader("Set-Cookie", "tom=cool; Max-Age=10", true);
+  response.bodyOutputStream.write(responseBody1, responseBody1.length);
+}
+
+let httpserv = null;
+
+function run_test() {
+  httpserv = new HttpServer();
+  httpserv.registerPathHandler("/path", requestHandler);
+  httpserv.start(-1);
+  httpserv.identity.add("http", TEST_DOMAIN, httpserv.identity.primaryPort);
+
+  registerCleanupFunction(() => {
+    Services.cookies.removeCookiesWithOriginAttributes("{}", TEST_DOMAIN);
+    Services.prefs.clearUserPref("network.dns.localDomains");
+    Services.prefs.clearUserPref("network.cookie.cookieBehavior");
+
+    httpserv.stop();
+    httpserv = null;
+  });
+
+  Services.prefs.setCharPref("network.dns.localDomains", TEST_DOMAIN);
+  Services.prefs.setIntPref("network.cookie.cookieBehavior", 0);
+  Services.cookies.removeCookiesWithOriginAttributes("{}", TEST_DOMAIN);
+
+  // Sends back the URL to the child script
+  do_await_remote_message("start-test").then(() => {
+    do_send_remote_message("start-test-done", URL);
+  });
+
+  // Sends back the cookie count for the domain
+  // Should only be one - from Set-Cookie
+  do_await_remote_message("check-cookie-count").then(() => {
+    do_send_remote_message("check-cookie-count-done", Services.cookies.countCookiesFromHost(TEST_DOMAIN));
+  });
+
+  // Sends back the cookie count for the domain
+  // There should be 2 cookies. One from the Set-Cookie header, the other set
+  // manually.
+  do_await_remote_message("second-check-cookie-count").then(() => {
+    do_send_remote_message("second-check-cookie-count-done", Services.cookies.countCookiesFromHost(TEST_DOMAIN));
+  });
+
+  // Sets a cookie for the test domain
+  do_await_remote_message("set-cookie").then(() => {
+    const expiry = Date.now() + 24 * 60 * 60;
+    Services.cookies.add(TEST_DOMAIN, "/", "cookieName", "cookieValue", false, false, false, expiry, {});
+    do_send_remote_message("set-cookie-done");
+  });
+
+  // Run the actual test logic
+  run_test_in_child("child_cookie_header.js");
+}

netwerk/test/unit_ipc/test_cookie_header_wrap.js

@@ -1,12 +0,0 @@
-//
-// Run test script in content process instead of chrome (xpcshell's default)
-//
-
-ChromeUtils.import("resource://gre/modules/Services.jsm");
-
-function run_test() {
-  // Allow all cookies.
-  Services.prefs.setIntPref("network.cookie.cookieBehavior", 0);
-  Services.prefs.setBoolPref("network.cookie.ipc.sync", true);
-  run_test_in_child("../unit/test_cookie_header.js");
-}

netwerk/test/unit_ipc/xpcshell.ini

@@ -4,13 +4,11 @@ skip-if = toolkit == 'android'
 support-files =
   child_channel_id.js
   !/netwerk/test/unit/test_XHR_redirects.js
-  !/netwerk/test/unit/test_bug248970_cookie.js
   !/netwerk/test/unit/test_bug528292.js
   !/netwerk/test/unit/test_cache-entry-id.js
   !/netwerk/test/unit/test_cache_jar.js
   !/netwerk/test/unit/test_cacheflags.js
   !/netwerk/test/unit/test_channel_close.js
-  !/netwerk/test/unit/test_cookie_header.js
   !/netwerk/test/unit/test_cookiejars.js
   !/netwerk/test/unit/test_dns_cancel.js
   !/netwerk/test/unit/test_dns_service.js
@@ -59,14 +57,14 @@ support-files =
   !/netwerk/test/unit/test_multipart_streamconv.js
   !/netwerk/test/unit/test_original_sent_received_head.js
   !/netwerk/test/unit/test_alt-data_cross_process.js
+  child_cookie_header.js
 
 [test_bug528292_wrap.js]
-[test_bug248970_cookie_wrap.js]
+[test_cookie_header_stripped.js]
 [test_cacheflags_wrap.js]
 [test_cache-entry-id_wrap.js]
 [test_cache_jar_wrap.js]
 [test_channel_close_wrap.js]
-[test_cookie_header_wrap.js]
 [test_cookiejars_wrap.js]
 [test_dns_cancel_wrap.js]
 [test_dns_service_wrap.js]