Skip to content

Commit 2ad9e82

Browse files
smarusaskillsoftstevemarusaRubenVerborgh
authored
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 <[email protected]> Co-authored-by: Ruben Verborgh <[email protected]>
1 parent 77e2a58 commit 2ad9e82

File tree

2 files changed

+86
-7
lines changed

2 files changed

+86
-7
lines changed

index.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,18 +361,23 @@ RedirectableRequest.prototype._processResponse = function (response) {
361361
}
362362

363363
// Drop the Host header, as the redirect might lead to a different host
364-
var previousHostName = removeMatchingHeaders(/^host$/i, this._options.headers) ||
365-
url.parse(this._currentUrl).hostname;
364+
var currentHostHeader = removeMatchingHeaders(/^host$/i, this._options.headers);
365+
366+
// If the redirect is relative, carry over the host of the last request
367+
var currentUrlParts = url.parse(this._currentUrl);
368+
var currentHost = currentHostHeader || currentUrlParts.host;
369+
var currentUrl = /^\w+:/.test(location) ? this._currentUrl :
370+
url.format(Object.assign(currentUrlParts, { host: currentHost }));
366371

367372
// Create the redirected request
368-
var redirectUrl = url.resolve(this._currentUrl, location);
373+
var redirectUrl = url.resolve(currentUrl, location);
369374
debug("redirecting to", redirectUrl);
370375
this._isRedirect = true;
371376
var redirectUrlParts = url.parse(redirectUrl);
372377
Object.assign(this._options, redirectUrlParts);
373378

374379
// Drop the Authorization header if redirecting to another host
375-
if (redirectUrlParts.hostname !== previousHostName) {
380+
if (redirectUrlParts.host !== currentHost) {
376381
removeMatchingHeaders(/^authorization$/i, this._options.headers);
377382
}
378383

@@ -506,7 +511,7 @@ function removeMatchingHeaders(regex, headers) {
506511
var lastValue;
507512
for (var header in headers) {
508513
if (regex.test(header)) {
509-
lastValue = headers[header];
514+
lastValue = headers[header].toString().trim();
510515
delete headers[header];
511516
}
512517
}

test/test.js

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,7 @@ describe("follow-redirects", function () {
12191219
});
12201220

12211221
describe("when redirecting to a different host while the host header is set", function () {
1222-
it("uses the new host header", function () {
1222+
it("uses the new host header if redirect host is different", function () {
12231223
app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
12241224
app.get("/b", function (req, res) {
12251225
res.write(JSON.stringify(req.headers));
@@ -1242,6 +1242,54 @@ describe("follow-redirects", function () {
12421242
assert.equal(body.host, "localhost:3600");
12431243
});
12441244
});
1245+
1246+
it("uses the location host if redirect host is the same", function () {
1247+
app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
1248+
app.get("/b", function (req, res) {
1249+
res.write(JSON.stringify(req.headers));
1250+
req.pipe(res); // will invalidate JSON if non-empty
1251+
});
1252+
1253+
return server.start(app)
1254+
.then(asPromise(function (resolve, reject) {
1255+
var opts = url.parse("http://127.0.0.1:3600/a");
1256+
opts.headers = { hOsT: "localhost:3600" };
1257+
http.get(opts, resolve).on("error", reject);
1258+
}))
1259+
.then(asPromise(function (resolve, reject, res) {
1260+
assert.deepEqual(res.statusCode, 200);
1261+
assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
1262+
res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
1263+
}))
1264+
.then(function (str) {
1265+
var body = JSON.parse(str);
1266+
assert.equal(body.host, "localhost:3600");
1267+
});
1268+
});
1269+
1270+
it("uses the existing host header if redirect host is relative", function () {
1271+
app.get("/a", redirectsTo(302, "/b"));
1272+
app.get("/b", function (req, res) {
1273+
res.write(JSON.stringify(req.headers));
1274+
req.pipe(res); // will invalidate JSON if non-empty
1275+
});
1276+
1277+
return server.start(app)
1278+
.then(asPromise(function (resolve, reject) {
1279+
var opts = url.parse("http://127.0.0.1:3600/a");
1280+
opts.headers = { hOsT: "localhost:3600" };
1281+
http.get(opts, resolve).on("error", reject);
1282+
}))
1283+
.then(asPromise(function (resolve, reject, res) {
1284+
assert.deepEqual(res.statusCode, 200);
1285+
assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
1286+
res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
1287+
}))
1288+
.then(function (str) {
1289+
var body = JSON.parse(str);
1290+
assert.equal(body.host, "localhost:3600");
1291+
});
1292+
});
12451293
});
12461294

12471295
describe("when the client passes an Authorization header", function () {
@@ -1278,7 +1326,7 @@ describe("follow-redirects", function () {
12781326

12791327
var opts = url.parse("http://127.0.0.1:3600/a");
12801328
opts.headers = {
1281-
host: "localhost",
1329+
host: "localhost:3600",
12821330
authorization: "bearer my-token-1234",
12831331
};
12841332

@@ -1296,6 +1344,32 @@ describe("follow-redirects", function () {
12961344
});
12971345
});
12981346

1347+
it("drops the header when redirected to a different host (same hostname and different port)", function () {
1348+
app.get("/a", redirectsTo(302, "http://localhost:3600/b"));
1349+
app.get("/b", function (req, res) {
1350+
res.end(JSON.stringify(req.headers));
1351+
});
1352+
1353+
var opts = url.parse("http://127.0.0.1:3600/a");
1354+
opts.headers = {
1355+
host: "localhost",
1356+
authorization: "bearer my-token-1234",
1357+
};
1358+
1359+
return server.start(app)
1360+
.then(asPromise(function (resolve, reject) {
1361+
http.get(opts, resolve).on("error", reject);
1362+
}))
1363+
.then(asPromise(function (resolve, reject, res) {
1364+
res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
1365+
}))
1366+
.then(function (str) {
1367+
var body = JSON.parse(str);
1368+
assert.equal(body.host, "localhost:3600");
1369+
assert.equal(body.authorization, undefined);
1370+
});
1371+
});
1372+
12991373
it("drops the header when redirected to a different host", function () {
13001374
app.get("/a", redirectsTo(302, "http://127.0.0.1:3600/b"));
13011375
app.get("/b", function (req, res) {

0 commit comments

Comments
 (0)