diff --git a/services/common/rest.js b/services/common/rest.js index 3fee0ca0e597..63110b67b566 100644 --- a/services/common/rest.js +++ b/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. diff --git a/services/common/tests/unit/test_restrequest.js b/services/common/tests/unit/test_restrequest.js index b394cd7533d7..22fbbe65d2a6 100644 --- a/services/common/tests/unit/test_restrequest.js +++ b/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(); }); diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js index df72760e86d8..a8e0c6c48590 100644 --- a/services/sync/modules/resource.js +++ b/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); }