Carry over Host header on relative redirects (#172)

- Set redirectUrl host from host header if relative redirect.
- Clarify logic to operate on host instead of hostname to keep/remove Host and Authorization headers. Host header is hostname+port.

Co-authored-by: Steven Marusa <steve.marusa@skillsoft.com>
Co-authored-by: Ruben Verborgh <ruben@verborgh.org>

index.js

@@ -361,18 +361,23 @@ RedirectableRequest.prototype._processResponse = function (response) {
     }
 
     // Drop the Host header, as the redirect might lead to a different host
-    var previousHostName = removeMatchingHeaders(/^host$/i, this._options.headers) ||
-      url.parse(this._currentUrl).hostname;
+    var currentHostHeader = removeMatchingHeaders(/^host$/i, this._options.headers);
+
+    // If the redirect is relative, carry over the host of the last request
+    var currentUrlParts = url.parse(this._currentUrl);
+    var currentHost = currentHostHeader || currentUrlParts.host;
+    var currentUrl = /^\w+:/.test(location) ? this._currentUrl :
+      url.format(Object.assign(currentUrlParts, { host: currentHost }));
 
     // Create the redirected request
-    var redirectUrl = url.resolve(this._currentUrl, location);
+    var redirectUrl = url.resolve(currentUrl, location);
     debug("redirecting to", redirectUrl);
     this._isRedirect = true;
     var redirectUrlParts = url.parse(redirectUrl);
     Object.assign(this._options, redirectUrlParts);
 
     // Drop the Authorization header if redirecting to another host
-    if (redirectUrlParts.hostname !== previousHostName) {
+    if (redirectUrlParts.host !== currentHost) {
       removeMatchingHeaders(/^authorization$/i, this._options.headers);
     }
 
@@ -506,7 +511,7 @@ function removeMatchingHeaders(regex, headers) {
   var lastValue;
   for (var header in headers) {
     if (regex.test(header)) {
-      lastValue = headers[header];
+      lastValue = headers[header].toString().trim();
       delete headers[header];
     }
   }

test/test.js

@@ -1219,7 +1219,7 @@ describe("follow-redirects", function () {
   });
 
   describe("when redirecting to a different host while the host header is set", function () {
-    it("uses the new host header", function () {
+    it("uses the new host header if redirect host is different", function () {
       app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
       app.get("/b", function (req, res) {
         res.write(JSON.stringify(req.headers));
@@ -1242,6 +1242,54 @@ describe("follow-redirects", function () {
           assert.equal(body.host, "localhost:3600");
         });
     });
+
+    it("uses the location host if redirect host is the same", function () {
+      app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
+      app.get("/b", function (req, res) {
+        res.write(JSON.stringify(req.headers));
+        req.pipe(res); // will invalidate JSON if non-empty
+      });
+
+      return server.start(app)
+        .then(asPromise(function (resolve, reject) {
+          var opts = url.parse("http://127.0.0.1:3600/a");
+          opts.headers = { hOsT: "localhost:3600" };
+          http.get(opts, resolve).on("error", reject);
+        }))
+        .then(asPromise(function (resolve, reject, res) {
+          assert.deepEqual(res.statusCode, 200);
+          assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
+          res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
+        }))
+        .then(function (str) {
+          var body = JSON.parse(str);
+          assert.equal(body.host, "localhost:3600");
+        });
+    });
+
+    it("uses the existing host header if redirect host is relative", function () {
+      app.get("/a", redirectsTo(302, "/b"));
+      app.get("/b", function (req, res) {
+        res.write(JSON.stringify(req.headers));
+        req.pipe(res); // will invalidate JSON if non-empty
+      });
+
+      return server.start(app)
+        .then(asPromise(function (resolve, reject) {
+          var opts = url.parse("http://127.0.0.1:3600/a");
+          opts.headers = { hOsT: "localhost:3600" };
+          http.get(opts, resolve).on("error", reject);
+        }))
+        .then(asPromise(function (resolve, reject, res) {
+          assert.deepEqual(res.statusCode, 200);
+          assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
+          res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
+        }))
+        .then(function (str) {
+          var body = JSON.parse(str);
+          assert.equal(body.host, "localhost:3600");
+        });
+    });
   });
 
   describe("when the client passes an Authorization header", function () {
@@ -1278,7 +1326,7 @@ describe("follow-redirects", function () {
 
       var opts = url.parse("http://127.0.0.1:3600/a");
       opts.headers = {
-        host: "localhost",
+        host: "localhost:3600",
         authorization: "bearer my-token-1234",
       };
 
@@ -1296,6 +1344,32 @@ describe("follow-redirects", function () {
         });
     });
 
+    it("drops the header when redirected to a different host (same hostname and different port)", function () {
+      app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
+      app.get("/b", function (req, res) {
+        res.end(JSON.stringify(req.headers));
+      });
+
+      var opts = url.parse("http://127.0.0.1:3600/a");
+      opts.headers = {
+        host: "localhost",
+        authorization: "bearer my-token-1234",
+      };
+
+      return server.start(app)
+        .then(asPromise(function (resolve, reject) {
+          http.get(opts, resolve).on("error", reject);
+        }))
+        .then(asPromise(function (resolve, reject, res) {
+          res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
+        }))
+        .then(function (str) {
+          var body = JSON.parse(str);
+          assert.equal(body.host, "localhost:3600");
+          assert.equal(body.authorization, undefined);
+        });
+    });
+
     it("drops the header when redirected to a different host", function () {
       app.get("/a", redirectsTo(302, "http://127.0.0.1:3600/b"));
       app.get("/b", function (req, res) {