Skip to content

Commit

Permalink
Review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
skillsoftstevemarusa committed Oct 5, 2021
1 parent f415196 commit 2cc405e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 18 deletions.
27 changes: 11 additions & 16 deletions index.js
Expand Up @@ -361,19 +361,24 @@ RedirectableRequest.prototype._processResponse = function (response) {
}

// Prefer previous host from Host header
var previousHost = getMatchingHeader(/^host$/i, this._options.headers) ||
url.parse(this._currentUrl).host;
var currentUrlParts = url.parse(this._currentUrl);
var previousHost = removeMatchingHeaders(/^host$/i, this._options.headers) ||
currentUrlParts.host;

// If the redirect is relative, carry over the host of the last request from host header
var previousHostUrl = !url.parse(location).host ?
url.format(Object.assign({}, currentUrlParts, { host: previousHost })) :
this._currentUrl;

// Create the redirected request
var redirectUrl = url.resolve(this._currentUrl, location);
var redirectUrl = url.resolve(previousHostUrl, location);
debug("redirecting to", redirectUrl);
this._isRedirect = true;
var redirectUrlParts = url.parse(redirectUrl);
Object.assign(this._options, redirectUrlParts);

// Drop the Host & Authorization header if redirecting to another host
if (redirectUrlParts.host !== previousHost && url.parse(location).host) {
removeMatchingHeaders(/^host$/i, this._options.headers);
// Drop the Authorization header if redirecting to another host
if (redirectUrlParts.host !== previousHost) {
removeMatchingHeaders(/^authorization$/i, this._options.headers);
}

Expand Down Expand Up @@ -503,16 +508,6 @@ function urlToOptions(urlObject) {
return options;
}

function getMatchingHeader(regex, headers) {
var lastValue;
for (var header in headers) {
if (regex.test(header)) {
lastValue = headers[header];
}
}
return lastValue;
}

function removeMatchingHeaders(regex, headers) {
var lastValue;
for (var header in headers) {
Expand Down
30 changes: 28 additions & 2 deletions test/test.js
Expand Up @@ -1243,7 +1243,7 @@ describe("follow-redirects", function () {
});
});

it("uses the existing host header if redirect host is the same", function () {
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));
Expand Down Expand Up @@ -1282,7 +1282,7 @@ describe("follow-redirects", function () {
}))
.then(asPromise(function (resolve, reject, res) {
assert.deepEqual(res.statusCode, 200);
assert.deepEqual(res.responseUrl, "http://127.0.0.1:3600/b");
assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
res.pipe(concat({ encoding: "string" }, resolve)).on("error", reject);
}))
.then(function (str) {
Expand Down Expand Up @@ -1344,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

0 comments on commit 2cc405e

Please sign in to comment.