Skip to content

Commit

Permalink
feat: Add allowInsecureRedirect option
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The allowInsecureRedirect is `false` by default, which may cause issues if your usage relies on insecure redirects. For the former behavior, you can opt in to insecure redirects by setting the option to `true`, but it is not recommended. 

Co-authored-by: Szymon Drosdzol <[email protected]>
  • Loading branch information
legobeat and SzymonDrosdzol authored Aug 8, 2023
1 parent 0664780 commit c5bcf21
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 3 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ The first argument can be either a `url` or an `options` object. The only requir
- `followOriginalHttpMethod` - by default we redirect to HTTP method GET. you can enable this property to redirect to the original HTTP method (default: `false`)
- `maxRedirects` - the maximum number of redirects to follow (default: `10`)
- `removeRefererHeader` - removes the referer header when a redirect happens (default: `false`). **Note:** if true, referer header set in the initial request is preserved during redirect chain.
- `allowInsecureRedirect` - allows cross-protocol redirects (HTTP to HTTPS and vice versa). **Warning:** may lead to bypassing anti SSRF filters (default: `false`)

---

Expand Down
6 changes: 5 additions & 1 deletion lib/redirect.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ function Redirect (request) {
this.redirects = []
this.redirectsFollowed = 0
this.removeRefererHeader = false
this.allowInsecureRedirect = false
}

Redirect.prototype.onRequest = function (options) {
Expand All @@ -40,6 +41,9 @@ Redirect.prototype.onRequest = function (options) {
if (options.followOriginalHttpMethod !== undefined) {
self.followOriginalHttpMethod = options.followOriginalHttpMethod
}
if (options.allowInsecureRedirect !== undefined) {
self.allowInsecureRedirect = options.allowInsecureRedirect
}
}

Redirect.prototype.redirectTo = function (response) {
Expand Down Expand Up @@ -113,7 +117,7 @@ Redirect.prototype.onResponse = function (response, callback) {
request.uri = url.parse(redirectTo)

// handle the case where we change protocol from https to http or vice versa
if (request.uri.protocol !== uriPrev.protocol) {
if (request.uri.protocol !== uriPrev.protocol && self.allowInsecureRedirect) {
delete request.agent
}

Expand Down
2 changes: 1 addition & 1 deletion tests/test-httpModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function runTests (name, httpModules) {
tape(name, function (t) {
var toHttps = 'http://localhost:' + plainServer.port + '/to_https'
var toPlain = 'https://localhost:' + httpsServer.port + '/to_plain'
var options = { httpModules: httpModules, strictSSL: false }
var options = { httpModules: httpModules, strictSSL: false, allowInsecureRedirect: true }
var modulesTest = httpModules || {}

clearFauxRequests()
Expand Down
1 change: 1 addition & 0 deletions tests/test-redirect-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ request = request.defaults({
user: 'test',
pass: 'testing'
},
allowInsecureRedirect: true,
rejectUnauthorized: false
})

Expand Down
1 change: 1 addition & 0 deletions tests/test-redirect-complex.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ tape('lots of redirects', function (t) {
request({
url: (i % 2 ? s.url : ss.url) + '/a',
headers: { 'x-test-key': key },
allowInsecureRedirect: true,
rejectUnauthorized: false
}, function (err, res, body) {
t.equal(err, null)
Expand Down
15 changes: 14 additions & 1 deletion tests/test-redirect.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ tape('http to https redirect', function (t) {
hits = {}
request.get({
uri: require('url').parse(s.url + '/ssl'),
rejectUnauthorized: false
rejectUnauthorized: false,
allowInsecureRedirect: true
}, function (err, res, body) {
t.equal(err, null)
t.equal(res.statusCode, 200)
Expand All @@ -454,6 +455,18 @@ tape('http to https redirect', function (t) {
})
})

tape('http to https redirect should fail without the explicit "allowInsecureRedirect" option', function (t) {
hits = {}
request.get({
uri: require('url').parse(s.url + '/ssl'),
rejectUnauthorized: false
}, function (err, res, body) {
t.notEqual(err, null)
t.equal(err.code, 'ERR_INVALID_PROTOCOL', 'Failed to cross-protocol redirect')
t.end()
})
})

tape('should have referer header by default when following redirect', function (t) {
request.post({
uri: s.url + '/temp',
Expand Down
3 changes: 3 additions & 0 deletions tests/test-tunnel.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ function addTests () {
'https->http over http, tunnel=true',
{
url: ss.url + '/redirect/http',
allowInsecureRedirect: true,
proxy: s.url,
tunnel: true
},
Expand All @@ -372,6 +373,7 @@ function addTests () {
'https->http over http, tunnel=false',
{
url: ss.url + '/redirect/http',
allowInsecureRedirect: true,
proxy: s.url,
tunnel: false
},
Expand All @@ -388,6 +390,7 @@ function addTests () {
'https->http over http, tunnel=default',
{
url: ss.url + '/redirect/http',
allowInsecureRedirect: true,
proxy: s.url
},
[
Expand Down

0 comments on commit c5bcf21

Please sign in to comment.