refactor: reuse wrapRequestBody in RedirectHandler (#4992)

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>

Author: trivikr

lib/handler/redirect-handler.js

@@ -1,30 +1,13 @@
 'use strict'
 
 const util = require('../core/util')
-const { kBodyUsed } = require('../core/symbols')
 const assert = require('node:assert')
 const { InvalidArgumentError } = require('../core/errors')
-const EE = require('node:events')
 
 const redirectableStatusCodes = [300, 301, 302, 303, 307, 308]
 
-const kBody = Symbol('body')
-
 const noop = () => {}
 
-class BodyAsyncIterable {
-  constructor (body) {
-    this[kBody] = body
-    this[kBodyUsed] = false
-  }
-
-  async * [Symbol.asyncIterator] () {
-    assert(!this[kBodyUsed], 'disturbed')
-    this[kBodyUsed] = true
-    yield * this[kBody]
-  }
-}
-
 class RedirectHandler {
   static buildDispatch (dispatcher, maxRedirections) {
     if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) {
@@ -44,43 +27,10 @@ class RedirectHandler {
     this.location = null
     const { maxRedirections: _, ...cleanOpts } = opts
     this.opts = cleanOpts // opts must be a copy, exclude maxRedirections
+    this.opts.body = util.wrapRequestBody(this.opts.body)
     this.maxRedirections = maxRedirections
     this.handler = handler
     this.history = []
-
-    if (util.isStream(this.opts.body)) {
-      // TODO (fix): Provide some way for the user to cache the file to e.g. /tmp
-      // so that it can be dispatched again?
-      // TODO (fix): Do we need 100-expect support to provide a way to do this properly?
-      if (util.bodyLength(this.opts.body) === 0) {
-        this.opts.body
-          .on('data', function () {
-            assert(false)
-          })
-      }
-
-      if (typeof this.opts.body.readableDidRead !== 'boolean') {
-        this.opts.body[kBodyUsed] = false
-        EE.prototype.on.call(this.opts.body, 'data', function () {
-          this[kBodyUsed] = true
-        })
-      }
-    } else if (this.opts.body && typeof this.opts.body.pipeTo === 'function') {
-      // TODO (fix): We can't access ReadableStream internal state
-      // to determine whether or not it has been disturbed. This is just
-      // a workaround.
-      this.opts.body = new BodyAsyncIterable(this.opts.body)
-    } else if (
-      this.opts.body &&
-      typeof this.opts.body !== 'string' &&
-      !ArrayBuffer.isView(this.opts.body) &&
-      util.isIterable(this.opts.body) &&
-      !util.isFormDataLike(this.opts.body)
-    ) {
-      // TODO: Should we allow re-using iterable if !this.opts.idempotent
-      // or through some other flag?
-      this.opts.body = new BodyAsyncIterable(this.opts.body)
-    }
   }
 
   onRequestStart (controller, context) {

test/interceptors/redirect.js

@@ -554,6 +554,32 @@ for (const factory of [
     await t.completed
   })
 
+  test('should stop following redirects once async iterable request bodies are disturbed', async t => {
+    t = tspl(t, { plan: 3 })
+
+    const server = await startRedirectingServer()
+
+    const {
+      statusCode,
+      headers,
+      body: bodyStream
+    } = await request(t, server, undefined, `http://${server}/301`, {
+      method: 'PUT',
+      body: (async function * () {
+        yield 'REQUEST'
+      })(),
+      maxRedirections: 10
+    })
+
+    const body = await bodyStream.text()
+
+    t.strictEqual(statusCode, 301)
+    t.strictEqual(headers.location, `http://${server}/301/2`)
+    t.strictEqual(body.length, 0)
+
+    await t.completed
+  })
+
   test('should not follow redirects when using Readable request bodies', async t => {
     t = tspl(t, { plan: 3 })
 

test/redirect-request.js

@@ -500,6 +500,28 @@ for (const factory of [
     await t.completed
   })
 
+  test('should stop following redirects once async iterable request bodies are disturbed', async t => {
+    t = tspl(t, { plan: 3 })
+
+    const server = await startRedirectingServer()
+
+    const { statusCode, headers, body: bodyStream } = await request(t, server, undefined, `http://${server}/301`, {
+      method: 'PUT',
+      body: (async function * () {
+        yield 'REQUEST'
+      })(),
+      maxRedirections: 10
+    })
+
+    const body = await bodyStream.text()
+
+    t.strictEqual(statusCode, 301)
+    t.strictEqual(headers.location, `http://${server}/301/2`)
+    t.strictEqual(body.length, 0)
+
+    await t.completed
+  })
+
   test('should not follow redirects when using Readable request bodies', async t => {
     t = tspl(t, { plan: 3 })