Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use existing host header when possible #172

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 10 additions & 5 deletions index.js
Expand Up @@ -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;
smarusa marked this conversation as resolved.
Show resolved Hide resolved
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) {
smarusa marked this conversation as resolved.
Show resolved Hide resolved
if (redirectUrlParts.host !== currentHost) {
removeMatchingHeaders(/^authorization$/i, this._options.headers);
}

Expand Down Expand Up @@ -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];
}
}
Expand Down
78 changes: 76 additions & 2 deletions test/test.js
Expand Up @@ -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));
Expand All @@ -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 () {
smarusa marked this conversation as resolved.
Show resolved Hide resolved
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 () {
Expand Down Expand Up @@ -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",
smarusa marked this conversation as resolved.
Show resolved Hide resolved
authorization: "bearer my-token-1234",
};

Expand All @@ -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) {
Expand Down