diff --git a/lib/web/fetch/index.js b/lib/web/fetch/index.js index 935bc9d4c90..29e422894cf 100644 --- a/lib/web/fetch/index.js +++ b/lib/web/fetch/index.js @@ -1388,8 +1388,45 @@ function httpRedirectFetch (fetchParams, response) { // actualResponse. setRequestReferrerPolicyOnRedirect(request, actualResponse) - // 20. Return the result of running main fetch given fetchParams and true. - return mainFetch(fetchParams, true) + // The redirect response is never exposed, so nothing will ever read its body. + // Leaving the stream unread pins the connection it arrived on, and with a pooled + // dispatcher each redirect can hold a slot until the pool is exhausted. + // `request()` already discards 3xx bodies in RedirectHandler; do the same here. + + // The redirect response is never exposed, so nothing will ever read its body. + // An unread stream leaves the connection it arrived on un-reusable, and with a + // pooled dispatcher each redirect can hold a slot until the pool is exhausted. + // `request()` already reads and discards 3xx bodies in RedirectHandler. + // The body cannot simply be cancelled here: the stream's cancel algorithm aborts + // fetchParams' controller, which would also abort the redirect we are about to follow. + return drainResponseBody(actualResponse).then(() => { + // 20. Return the result of running main fetch given fetchParams and true. + return mainFetch(fetchParams, true) + }) +} + +/** + * Reads a response body to completion and discards it, so the underlying + * connection can be released back to the pool. + */ +async function drainResponseBody (response) { + const stream = response.body?.stream + + if (stream == null || stream.locked) { + return + } + + const reader = stream.getReader() + + try { + while (!(await reader.read()).done) { + // discarded on purpose + } + } catch { + // a failed drain must not fail the redirect + } finally { + reader.releaseLock() + } } // https://fetch.spec.whatwg.org/#http-network-or-cache-fetch diff --git a/test/fetch/redirect-body-drain.js b/test/fetch/redirect-body-drain.js new file mode 100644 index 00000000000..ab7362bd573 --- /dev/null +++ b/test/fetch/redirect-body-drain.js @@ -0,0 +1,40 @@ +'use strict' + +const { test } = require('node:test') +const assert = require('node:assert') +const { createServer } = require('node:http') +const { once } = require('node:events') +const { fetch, Agent } = require('../..') + +test('fetch does not pin a pooled connection on a redirect with an unread body', async (t) => { + // Large enough that the 3xx body does not fit in the stream's buffer, so an + // undrained body leaves the socket unusable for the follow-up request. + const redirectBody = Buffer.alloc(128 * 1024, 0x78) + + const server = createServer((req, res) => { + if (req.url === '/redirect') { + res.writeHead(301, { Location: '/final' }) + res.end(redirectBody) + return + } + res.end('ok') + }) + + // A single connection, so a pinned socket cannot be worked around. + const dispatcher = new Agent({ connections: 1, keepAliveTimeout: 10_000 }) + + t.after(async () => { + await dispatcher.destroy() + server.close() + }) + + server.listen(0) + await once(server, 'listening') + + const url = `http://127.0.0.1:${server.address().port}/redirect` + const response = await fetch(url, { dispatcher, redirect: 'follow' }) + + assert.strictEqual(response.status, 200) + assert.strictEqual(await response.text(), 'ok') + assert.ok(response.redirected) +})