Skip to content

Commit 948c30c

Browse files
mbargielRubenVerborgh
authored andcommitted
Fix redirecting to relative URL when using proxy
1 parent 22e81fc commit 948c30c

3 files changed

Lines changed: 75 additions & 6 deletions

File tree

‎index.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,21 +274,26 @@ RedirectableRequest.prototype._performRequest = function () {
274274
this._options.agent = this._options.agents[scheme];
275275
}
276276

277-
// Create the native request
277+
// Create the native request and set up its event handlers
278278
var request = this._currentRequest =
279279
nativeProtocol.request(this._options, this._onNativeResponse);
280-
this._currentUrl = url.format(this._options);
281-
282-
// Set up event handlers
283280
request._redirectable = this;
284281
for (var e = 0; e < events.length; e++) {
285282
request.on(events[e], eventHandlers[events[e]]);
286283
}
287284

285+
// RFC7230§5.3.1: When making a request directly to an origin server, […]
286+
// a client MUST send only the absolute path […] as the request-target.
287+
this._currentUrl = /^\//.test(this._options.path) ?
288+
url.format(this._options) :
289+
// When making a request to a proxy, […]
290+
// a client MUST send the target URI in absolute-form […].
291+
this._currentUrl = this._options.path;
292+
288293
// End a redirected request
289294
// (The first request must be ended explicitly with RedirectableRequest#end)
290295
if (this._isRedirect) {
291-
// Write the request entity and end.
296+
// Write the request entity and end
292297
var i = 0;
293298
var self = this;
294299
var buffers = this._requestBodyBuffers;

‎test/test.js‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var delay = util.delay;
1717
var redirectsTo = util.redirectsTo;
1818
var sendsJson = util.sendsJson;
1919
var asPromise = util.asPromise;
20+
var proxy = util.proxy;
2021

2122
var testFile = path.resolve(__dirname, "assets/input.txt");
2223
var testFileBuffer = fs.readFileSync(testFile);
@@ -297,7 +298,7 @@ describe("follow-redirects", function () {
297298
});
298299
});
299300

300-
it("emits an error whem url.resolve fails", function () {
301+
it("emits an error when url.resolve fails", function () {
301302
app.get("/a", redirectsTo("/b"));
302303
var urlResolve = url.resolve;
303304
url.resolve = function () {
@@ -2080,6 +2081,45 @@ describe("follow-redirects", function () {
20802081
});
20812082
});
20822083
});
2084+
2085+
describe("when request is going through an HTTP proxy (without a tunnel)", function () {
2086+
[
2087+
{ redirectType: "absolute", redirectUrl: "http://localhost:3600/b" },
2088+
{ redirectType: "relative", redirectUrl: "/b" },
2089+
].forEach(function (testCase) {
2090+
it("redirects to proper URL when Location header is " + testCase.redirectType, function () {
2091+
app.get("/a", redirectsTo(testCase.redirectUrl));
2092+
app.get("/b", sendsJson({ good: "yes" }));
2093+
app2.port = 3601;
2094+
app2.all("*", proxy("localhost:3601"));
2095+
2096+
function setProxy(opts) {
2097+
// assuming opts is a url.parse result
2098+
// Update path and Host header
2099+
opts.path = opts.href;
2100+
opts.pathname = opts.href;
2101+
2102+
// Update port and host to target proxy host
2103+
opts.port = 3601;
2104+
opts.host = opts.hostname + ":" + opts.port;
2105+
2106+
// redirected requests use proxy too
2107+
opts.beforeRedirect = setProxy;
2108+
}
2109+
return Promise.all([server.start(app), server.start(app2)])
2110+
.then(asPromise(function (resolve, reject) {
2111+
var opts = Object.assign({}, url.parse("http://localhost:3600/a"));
2112+
setProxy(opts);
2113+
2114+
http.get(opts, concatJson(resolve, reject)).on("error", reject);
2115+
}))
2116+
.then(function (res) {
2117+
assert.deepEqual(res.parsedJson, { good: "yes" });
2118+
assert.deepEqual(res.responseUrl, "http://localhost:3600/b");
2119+
});
2120+
});
2121+
});
2122+
});
20832123
});
20842124

20852125
function noop() { /* noop */ }

‎test/util.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
var concat = require("concat-stream");
2+
var http = require("http");
3+
var https = require("https");
4+
var url = require("url");
25

36
function redirectsTo() {
47
var args = Array.prototype.slice.call(arguments);
@@ -42,10 +45,31 @@ function asPromise(cb) {
4245
};
4346
}
4447

48+
function proxy(proxyHost) {
49+
return function (req, res) {
50+
var upstreamUrl = url.parse(req.originalUrl);
51+
if (upstreamUrl.host === proxyHost) {
52+
res.writeHead(400, "Bad request");
53+
res.write(JSON.stringify({ bad: "detected proxy recursion" }));
54+
res.end();
55+
}
56+
else {
57+
var transport = /https:?/.test(upstreamUrl.protocol) ? https : http;
58+
upstreamUrl.headers = req.headers;
59+
var upstreamReq = transport.request(upstreamUrl, function (upstreamRes) {
60+
res.writeHead(upstreamRes.statusCode, upstreamRes.statusMessage, upstreamRes.headers);
61+
upstreamRes.pipe(res);
62+
});
63+
upstreamReq.end();
64+
}
65+
};
66+
}
67+
4568
module.exports = {
4669
asPromise: asPromise,
4770
concatJson: concatJson,
4871
delay: delay,
72+
proxy: proxy,
4973
redirectsTo: redirectsTo,
5074
sendsJson: sendsJson,
5175
};

0 commit comments

Comments
 (0)