From c82923f4ef17d0530c6af4ebf0b3aa0f179f9bf0 Mon Sep 17 00:00:00 2001 From: Maxime Bargiel Date: Tue, 24 May 2022 19:44:23 -0400 Subject: [PATCH 1/2] Fix redirecting to relative URL when using proxy --- index.js | 11 +++++++++-- test/test.js | 42 +++++++++++++++++++++++++++++++++++++++++- test/util.js | 24 ++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 252c9bf..06a9f46 100644 --- a/index.js +++ b/index.js @@ -5,6 +5,7 @@ var https = require("https"); var Writable = require("stream").Writable; var assert = require("assert"); var debug = require("./debug"); +const path = require("path"); // Create handlers that pass events from native requests var events = ["abort", "aborted", "connect", "error", "socket", "timeout"]; @@ -277,7 +278,13 @@ RedirectableRequest.prototype._performRequest = function () { // Create the native request var request = this._currentRequest = nativeProtocol.request(this._options, this._onNativeResponse); - this._currentUrl = url.format(this._options); + if (path.isAbsolute(this._options.path)) { + // retrieving resource on target host + this._currentUrl = url.format(this._options); + } else { + // retrieving resource on other host - proxying + this._currentUrl = this._options.path; + } // Set up event handlers request._redirectable = this; @@ -590,4 +597,4 @@ function isSubdomain(subdomain, domain) { // Exports module.exports = wrap({ http: http, https: https }); -module.exports.wrap = wrap; +module.exports.wrap = wrap; \ No newline at end of file diff --git a/test/test.js b/test/test.js index ce98a5e..7e956d4 100644 --- a/test/test.js +++ b/test/test.js @@ -17,6 +17,7 @@ var delay = util.delay; var redirectsTo = util.redirectsTo; var sendsJson = util.sendsJson; var asPromise = util.asPromise; +var proxy = util.proxy; var testFile = path.resolve(__dirname, "assets/input.txt"); var testFileBuffer = fs.readFileSync(testFile); @@ -297,7 +298,7 @@ describe("follow-redirects", function () { }); }); - it("emits an error whem url.resolve fails", function () { + it("emits an error when url.resolve fails", function () { app.get("/a", redirectsTo("/b")); var urlResolve = url.resolve; url.resolve = function () { @@ -2080,6 +2081,45 @@ describe("follow-redirects", function () { }); }); }); + + describe("when request is going through an HTTP proxy (without a tunnel)", function() { + [ + { redirectType: "absolute", redirectUrl: "http://localhost:3600/b" }, + { redirectType: "relative", redirectUrl: "/b" }, + ].forEach(function (testCase) { + it("redirects to proper URL when Location header is " + testCase.redirectType, function() { + app.get("/a", redirectsTo(testCase.redirectUrl)); + app.get("/b", sendsJson({ good: "yes" })); + app2.port = 3601; + app2.all("*", proxy("localhost:3601")); + + function setProxy(opts) { + // assuming opts is a url.parse result + // Update path and Host header + opts.path = opts.href; + opts.pathname = opts.href; + + // Update port and host to target proxy host + opts.port = 3601; + opts.host = opts.hostname + ":" + opts.port; + + // redirected requests use proxy too + opts.beforeRedirect = setProxy; + } + return Promise.all([server.start(app), server.start(app2)]) + .then(asPromise(function (resolve, reject) { + var opts = Object.assign({}, url.parse("http://localhost:3600/a")); + setProxy(opts); + + http.get(opts, concatJson(resolve, reject)).on("error", reject); + })) + .then(function (res) { + assert.deepEqual(res.parsedJson, { good: "yes" }); + assert.deepEqual(res.responseUrl, "http://localhost:3600/b"); + }); + }); + }); + }); }); function noop() { /* noop */ } diff --git a/test/util.js b/test/util.js index 4ff6974..b91f629 100644 --- a/test/util.js +++ b/test/util.js @@ -1,4 +1,7 @@ var concat = require("concat-stream"); +var http = require("http"); +var https = require("https"); +var url = require("url"); function redirectsTo() { var args = Array.prototype.slice.call(arguments); @@ -42,10 +45,31 @@ function asPromise(cb) { }; } +function proxy(proxyHost) { + return function(req, res) { + var upstreamUrl = url.parse(req.originalUrl); + if (upstreamUrl.host === proxyHost) { + res.writeHead(400, 'Bad request'); + res.write(JSON.stringify({ bad: 'detected proxy recursion' })); + return res.end(); + } + + var transport = /https:?/.test(upstreamUrl.protocol) ? https : http; + var upstreamReq = transport.request(req.originalUrl, { + headers: req.headers, + }, function (upstreamRes) { + res.writeHead(upstreamRes.statusCode, upstreamRes.statusMessage, upstreamRes.headers); + upstreamRes.pipe(res); + }); + upstreamReq.end(); + } +} + module.exports = { asPromise: asPromise, concatJson: concatJson, delay: delay, + proxy: proxy, redirectsTo: redirectsTo, sendsJson: sendsJson, }; From 70ed4001f9d125c5aa5769f4bc0fe642ed49fe74 Mon Sep 17 00:00:00 2001 From: Maxime Bargiel Date: Tue, 24 May 2022 21:18:24 -0400 Subject: [PATCH 2/2] Fix lint errors --- index.js | 7 ++++--- test/test.js | 6 +++--- test/util.js | 29 +++++++++++++++-------------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/index.js b/index.js index 06a9f46..eccad6e 100644 --- a/index.js +++ b/index.js @@ -279,9 +279,10 @@ RedirectableRequest.prototype._performRequest = function () { var request = this._currentRequest = nativeProtocol.request(this._options, this._onNativeResponse); if (path.isAbsolute(this._options.path)) { - // retrieving resource on target host + // retrieving resource on target host this._currentUrl = url.format(this._options); - } else { + } + else { // retrieving resource on other host - proxying this._currentUrl = this._options.path; } @@ -597,4 +598,4 @@ function isSubdomain(subdomain, domain) { // Exports module.exports = wrap({ http: http, https: https }); -module.exports.wrap = wrap; \ No newline at end of file +module.exports.wrap = wrap; diff --git a/test/test.js b/test/test.js index 7e956d4..41f1866 100644 --- a/test/test.js +++ b/test/test.js @@ -2082,12 +2082,12 @@ describe("follow-redirects", function () { }); }); - describe("when request is going through an HTTP proxy (without a tunnel)", function() { + describe("when request is going through an HTTP proxy (without a tunnel)", function () { [ { redirectType: "absolute", redirectUrl: "http://localhost:3600/b" }, { redirectType: "relative", redirectUrl: "/b" }, ].forEach(function (testCase) { - it("redirects to proper URL when Location header is " + testCase.redirectType, function() { + it("redirects to proper URL when Location header is " + testCase.redirectType, function () { app.get("/a", redirectsTo(testCase.redirectUrl)); app.get("/b", sendsJson({ good: "yes" })); app2.port = 3601; @@ -2116,7 +2116,7 @@ describe("follow-redirects", function () { .then(function (res) { assert.deepEqual(res.parsedJson, { good: "yes" }); assert.deepEqual(res.responseUrl, "http://localhost:3600/b"); - }); + }); }); }); }); diff --git a/test/util.js b/test/util.js index b91f629..4f26e0f 100644 --- a/test/util.js +++ b/test/util.js @@ -46,23 +46,24 @@ function asPromise(cb) { } function proxy(proxyHost) { - return function(req, res) { + return function (req, res) { var upstreamUrl = url.parse(req.originalUrl); if (upstreamUrl.host === proxyHost) { - res.writeHead(400, 'Bad request'); - res.write(JSON.stringify({ bad: 'detected proxy recursion' })); - return res.end(); + res.writeHead(400, "Bad request"); + res.write(JSON.stringify({ bad: "detected proxy recursion" })); + res.end(); } - - var transport = /https:?/.test(upstreamUrl.protocol) ? https : http; - var upstreamReq = transport.request(req.originalUrl, { - headers: req.headers, - }, function (upstreamRes) { - res.writeHead(upstreamRes.statusCode, upstreamRes.statusMessage, upstreamRes.headers); - upstreamRes.pipe(res); - }); - upstreamReq.end(); - } + else { + var transport = /https:?/.test(upstreamUrl.protocol) ? https : http; + var upstreamReq = transport.request(req.originalUrl, { + headers: req.headers, + }, function (upstreamRes) { + res.writeHead(upstreamRes.statusCode, upstreamRes.statusMessage, upstreamRes.headers); + upstreamRes.pipe(res); + }); + upstreamReq.end(); + } + }; } module.exports = {