From 5a47b01f7e2a86f5b3041aa3f456599df6579fb3 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sat, 23 Nov 2024 18:43:40 +0100
Subject: [PATCH] fix: we can redirect disturbed request body if it's not going
to be used (#3873)
* fix: we can redirect disturbed request body if it's not going to be used
* fixup
* fixup
* fixup
* fixup
---
lib/handler/redirect-handler.js | 63 ++++++++++++++++-----------------
test/interceptors/redirect.js | 22 +++++++++++-
test/redirect-request.js | 19 +++++++++-
3 files changed, 69 insertions(+), 35 deletions(-)
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 => {