Bug 798430 - Copy our set HTTP headers across channel redirects. r=gps, a=lsblakk

services/common/rest.js

@@ -532,6 +532,19 @@ RESTRequest.prototype = {
     return true;
   },
 
+  /**
+   * Returns true if headers from the old channel should be
+   * copied to the new channel. Invoked when a channel redirect
+   * is in progress.
+   */
+  shouldCopyOnRedirect: function shouldCopyOnRedirect(oldChannel, newChannel, flags) {
+    let isInternal = !!(flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL);
+    let isSameURI  = newChannel.URI.equals(oldChannel.URI);
+    this._log.debug("Channel redirect: " + oldChannel.URI.spec + ", " +
+                    newChannel.URI.spec + ", internal = " + isInternal);
+    return isInternal && isSameURI;
+  },
+
   /*** nsIChannelEventSink ***/
   asyncOnChannelRedirect:
     function asyncOnChannelRedirect(oldChannel, newChannel, flags, callback) {
@@ -544,6 +557,18 @@ RESTRequest.prototype = {
       return;
     }
 
+    // For internal redirects, copy the headers that our caller set.
+    try {
+      if (this.shouldCopyOnRedirect(oldChannel, newChannel, flags)) {
+        this._log.trace("Copying headers for safe internal redirect.");
+        for (let key in this._headers) {
+          newChannel.setRequestHeader(key, this._headers[key], false);
+        }
+      }
+    } catch (ex) {
+      this._log.error("Error copying headers: " + CommonUtils.exceptionStr(ex));
+    }
+
     this.channel = newChannel;
 
     // We let all redirects proceed.

services/common/tests/unit/test_restrequest.js

@@ -730,8 +730,15 @@ add_test(function test_exception_in_onProgress() {
 add_test(function test_new_channel() {
   _("Ensure a redirect to a new channel is handled properly.");
 
+  function checkUA(metadata) {
+    let ua = metadata.getHeader("User-Agent");
+    _("User-Agent is " + ua);
+    do_check_eq("foo bar", ua);
+  }
+
   let redirectRequested = false;
   function redirectHandler(metadata, response) {
+    checkUA(metadata);
     redirectRequested = true;
 
     let body = "Redirecting";
@@ -742,12 +749,14 @@ add_test(function test_new_channel() {
 
   let resourceRequested = false;
   function resourceHandler(metadata, response) {
+    checkUA(metadata);
     resourceRequested = true;
 
     let body = "Test";
     response.setHeader("Content-Type", "text/plain");
     response.bodyOutputStream.write(body, body.length);
   }
+
   let server1 = httpd_setup({"/redirect": redirectHandler}, 8080);
   let server2 = httpd_setup({"/resource": resourceHandler}, 8081);
 
@@ -758,11 +767,25 @@ add_test(function test_new_channel() {
   }
 
   let request = new RESTRequest("http://localhost:8080/redirect");
+  request.setHeader("User-Agent", "foo bar");
+
+  // Swizzle in our own fakery, because this redirect is neither
+  // internal nor URI-preserving. RESTRequest's policy is to only
+  // copy headers under certain circumstances.
+  let protoMethod = request.shouldCopyOnRedirect;
+  request.shouldCopyOnRedirect = function wrapped(o, n, f) {
+    // Check the default policy.
+    do_check_false(protoMethod.call(this, o, n, f));
+    return true;
+  };
+
   request.get(function onComplete(error) {
     let response = this.response;
 
     do_check_eq(200, response.status);
     do_check_eq("Test", response.body);
+    do_check_true(redirectRequested);
+    do_check_true(resourceRequested);
 
     advance();
   });

services/sync/modules/resource.js

@@ -20,6 +20,13 @@ Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-common/log4moz.js");
 Cu.import("resource://services-sync/util.js");
 
+const DEFAULT_LOAD_FLAGS =
+  // Always validate the cache:
+  Ci.nsIRequest.LOAD_BYPASS_CACHE |
+  Ci.nsIRequest.INHIBIT_CACHING |
+  // Don't send user cookies over the wire (Bug 644734).
+  Ci.nsIRequest.LOAD_ANONYMOUS;
+
 /*
  * AsyncResource represents a remote network resource, identified by a URI.
  * Create an instance like so:
@@ -97,6 +104,9 @@ AsyncResource.prototype = {
   setHeader: function Res_setHeader(header, value) {
     this._headers[header.toLowerCase()] = value;
   },
+  get headerNames() {
+    return Object.keys(this.headers);
+  },
 
   // ** {{{ AsyncResource.uri }}} **
   //
@@ -140,14 +150,11 @@ AsyncResource.prototype = {
                           .QueryInterface(Ci.nsIRequest)
                           .QueryInterface(Ci.nsIHttpChannel);
 
-    // Always validate the cache:
-    channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE;
-    channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING;
-    // Don't send user cookies & such over the wire (Bug 644734)
-    channel.loadFlags |= Ci.nsIRequest.LOAD_ANONYMOUS;
+    channel.loadFlags |= DEFAULT_LOAD_FLAGS;
 
     // Setup a callback to handle channel notifications.
-    channel.notificationCallbacks = new ChannelNotificationListener();
+    let listener = new ChannelNotificationListener(this.headerNames);
+    channel.notificationCallbacks = listener;
 
     // Compose a UA string fragment from the various available identifiers.
     if (Svc.Prefs.get("sendVersionInfo", true)) {
@@ -556,10 +563,20 @@ ChannelListener.prototype = {
  * This class handles channel notification events.
  *
  * An instance of this class is bound to each created channel.
+ *
+ * Optionally pass an array of header names. Each header named
+ * in this array will be copied between the channels in the
+ * event of a redirect.
  */
-function ChannelNotificationListener() {
+function ChannelNotificationListener(headersToCopy) {
+  this._headersToCopy = headersToCopy;
+
+  this._log = Log4Moz.repository.getLogger(this._logName);
+  this._log.level = Log4Moz.Level[Svc.Prefs.get("log.logger.network.resources")];
 }
 ChannelNotificationListener.prototype = {
+  _logName: "Sync.Resource",
+
   getInterface: function(aIID) {
     return this.QueryInterface(aIID);
   },
@@ -585,6 +602,25 @@ ChannelNotificationListener.prototype = {
   asyncOnChannelRedirect:
     function asyncOnChannelRedirect(oldChannel, newChannel, flags, callback) {
 
+    this._log.debug("Channel redirect: " + oldChannel.URI.spec + ", " +
+                    newChannel.URI.spec + ", " + flags);
+
+    // For internal redirects, copy the headers that our caller set.
+    try {
+      if ((flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL) &&
+          newChannel.URI.equals(oldChannel.URI)) {
+        this._log.trace("Copying headers for safe internal redirect.");
+        for (let header of this._headersToCopy) {
+          let value = oldChannel.getRequestHeader(header);
+          if (value) {
+            newChannel.setRequestHeader(header, value);
+          }
+        }
+      }
+    } catch (ex) {
+      this._log.error("Error copying headers: " + CommonUtils.exceptionStr(ex));
+    }
+
     // We let all redirects proceed.
     callback.onRedirectVerifyCallback(Cr.NS_OK);
   }