diff --git a/lib/handler/redirect-handler.js b/lib/handler/redirect-handler.js index 40adeb52415..02c302d9aa8 100644 --- a/lib/handler/redirect-handler.js +++ b/lib/handler/redirect-handler.js @@ -49,7 +49,6 @@ class RedirectHandler { this.maxRedirections = maxRedirections this.handler = handler this.history = [] - this.redirectionLimitReached = false if (util.isStream(this.opts.body)) { // TODO (fix): Provide some way for the user to cache the file to e.g. /tmp @@ -99,27 +98,42 @@ class RedirectHandler { this.handler.onError(error) } - onHeaders (statusCode, headers, resume, statusText) { - this.location = this.history.length >= this.maxRedirections || util.isDisturbed(this.opts.body) - ? null - : parseLocation(statusCode, headers) - + onHeaders (statusCode, rawHeaders, resume, statusText) { if (this.opts.throwOnMaxRedirect && this.history.length >= this.maxRedirections) { - if (this.request) { - this.request.abort(new Error('max redirects')) + throw new Error('max redirects') + } + + // https://tools.ietf.org/html/rfc7231#section-6.4.2 + // https://fetch.spec.whatwg.org/#http-redirect-fetch + // In case of HTTP 301 or 302 with POST, change the method to GET + if ((statusCode === 301 || statusCode === 302) && this.opts.method === 'POST') { + this.opts.method = 'GET' + if (util.isStream(this.opts.body)) { + util.destroy(this.opts.body.on('error', noop)) } + this.opts.body = null + } - this.redirectionLimitReached = true - this.abort(new Error('max redirects')) - return + // https://tools.ietf.org/html/rfc7231#section-6.4.4 + // In case of HTTP 303, always replace method to be either HEAD or GET + if (statusCode === 303 && this.opts.method !== 'HEAD') { + this.opts.method = 'GET' + if (util.isStream(this.opts.body)) { + util.destroy(this.opts.body.on('error', noop)) + } + this.opts.body = null } + this.location = this.history.length >= this.maxRedirections || util.isDisturbed(this.opts.body) + ? null + : parseLocation(statusCode, rawHeaders) + if (this.opts.origin) { this.history.push(new URL(this.opts.path, this.opts.origin)) } if (!this.location) { - return this.handler.onHeaders(statusCode, headers, resume, statusText) + return this.handler.onHeaders(statusCode, rawHeaders, resume, statusText) } const { origin, pathname, search } = util.parseURL(new URL(this.location, this.opts.origin && new URL(this.opts.path, this.opts.origin))) @@ -133,23 +147,6 @@ class RedirectHandler { this.opts.origin = origin this.opts.maxRedirections = 0 this.opts.query = null - - // https://tools.ietf.org/html/rfc7231#section-6.4.2 - // https://fetch.spec.whatwg.org/#http-redirect-fetch - // In case of HTTP 301 or 302 with POST, change the method to GET - if ((statusCode === 301 || statusCode === 302) && this.opts.method === 'POST') { - this.opts.method = 'GET' - if (util.isStream(this.opts.body)) util.destroy(this.opts.body.on('error', noop)) - this.opts.body = null - } - - // https://tools.ietf.org/html/rfc7231#section-6.4.4 - // In case of HTTP 303, always replace method to be either HEAD or GET - if (statusCode === 303 && this.opts.method !== 'HEAD') { - this.opts.method = 'GET' - if (util.isStream(this.opts.body)) util.destroy(this.opts.body.on('error', noop)) - this.opts.body = null - } } onData (chunk) { @@ -203,14 +200,14 @@ class RedirectHandler { } } -function parseLocation (statusCode, headers) { +function parseLocation (statusCode, rawHeaders) { if (redirectableStatusCodes.indexOf(statusCode) === -1) { return null } - for (let i = 0; i < headers.length; i += 2) { - if (headers[i].length === 8 && util.headerNameToString(headers[i]) === 'location') { - return headers[i + 1] + for (let i = 0; i < rawHeaders.length; i += 2) { + if (rawHeaders[i].length === 8 && util.headerNameToString(rawHeaders[i]) === 'location') { + return rawHeaders[i + 1] } } } diff --git a/test/interceptors/redirect.js b/test/interceptors/redirect.js index fe4cc86256c..c337f1f45ae 100644 --- a/test/interceptors/redirect.js +++ b/test/interceptors/redirect.js @@ -564,7 +564,7 @@ for (const factory of [ headers, body: bodyStream } = await request(t, server, undefined, `http://${server}/301`, { - method: 'POST', + method: 'PUT', body: createReadable('REQUEST'), maxRedirections: 10 }) @@ -576,6 +576,26 @@ for (const factory of [ t.strictEqual(body.length, 0) await t.completed }) + + test('should follow redirects when using Readable request bodies w/ POST 101', async t => { + t = tspl(t, { plan: 1 }) + + const server = await startRedirectingServer() + + const { + statusCode, + body: bodyStream + } = await request(t, server, undefined, `http://${server}/301`, { + method: 'POST', + body: createReadable('REQUEST'), + maxRedirections: 10 + }) + + await bodyStream.text() + + t.strictEqual(statusCode, 200) + await t.completed + }) } test('should follow redirections when going cross origin', async t => { diff --git a/test/redirect-request.js b/test/redirect-request.js index 55583d7ac05..9938cd7a043 100644 --- a/test/redirect-request.js +++ b/test/redirect-request.js @@ -505,7 +505,7 @@ for (const factory of [ const server = await startRedirectingServer() const { statusCode, headers, body: bodyStream } = await request(t, server, undefined, `http://${server}/301`, { - method: 'POST', + method: 'PUT', body: createReadable('REQUEST'), maxRedirections: 10 }) @@ -517,6 +517,23 @@ for (const factory of [ t.strictEqual(body.length, 0) await t.completed }) + + test('should follow redirects when using Readable request bodies for POST 301', async t => { + t = tspl(t, { plan: 1 }) + + const server = await startRedirectingServer() + + const { statusCode, body: bodyStream } = await request(t, server, undefined, `http://${server}/301`, { + method: 'POST', + body: createReadable('REQUEST'), + maxRedirections: 10 + }) + + await bodyStream.text() + + t.strictEqual(statusCode, 200) + await t.completed + }) } test('should follow redirections when going cross origin', async t => {