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

feat: Add allowInsecureRedirect option #28

Merged
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
1 change: 1 addition & 0 deletions README.md
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
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
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
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
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
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
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