diff --git a/lib/dispatcher/dispatcher1-wrapper.js b/lib/dispatcher/dispatcher1-wrapper.js index f5813288cb3..7795e314673 100644 --- a/lib/dispatcher/dispatcher1-wrapper.js +++ b/lib/dispatcher/dispatcher1-wrapper.js @@ -60,6 +60,56 @@ class LegacyHandlerWrapper { } } +// Legacy consumers (e.g. Node's bundled fetch) may send an identical +// comma-repeated content-length ("58, 58") that the current core rejects. +// RFC 9110 allows collapsing identical repeats; conflicting values still +// fail downstream. See https://github.com/nodejs/undici/issues/5500 +function collapseRepeatedContentLength (value) { + if (typeof value !== 'string' || !value.includes(',')) { + return value + } + + const parts = value.split(',') + const first = parts[0].trim() + + if (first === '') { + return value + } + + for (let i = 1; i < parts.length; i++) { + if (parts[i].trim() !== first) { + return value + } + } + + return first +} + +function normalizeLegacyHeaders (headers) { + if (Array.isArray(headers)) { + for (let i = 0; i + 1 < headers.length; i += 2) { + if (typeof headers[i] === 'string' && headers[i].toLowerCase() === 'content-length') { + const collapsed = collapseRepeatedContentLength(headers[i + 1]) + if (collapsed !== headers[i + 1]) { + headers = headers.slice() + headers[i + 1] = collapsed + } + } + } + } else if (headers && typeof headers === 'object') { + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === 'content-length') { + const collapsed = collapseRepeatedContentLength(headers[key]) + if (collapsed !== headers[key]) { + headers = { ...headers, [key]: collapsed } + } + } + } + } + + return headers +} + class Dispatcher1Wrapper extends Dispatcher { #dispatcher @@ -92,6 +142,11 @@ class Dispatcher1Wrapper extends Dispatcher { opts = { ...opts, allowH2: false } } + const headers = normalizeLegacyHeaders(opts.headers) + if (headers !== opts.headers) { + opts = { ...opts, headers } + } + return this.#dispatcher.dispatch(opts, Dispatcher1Wrapper.wrapHandler(handler)) } diff --git a/test/dispatcher1-wrapper-content-length.js b/test/dispatcher1-wrapper-content-length.js new file mode 100644 index 00000000000..bec910e99ac --- /dev/null +++ b/test/dispatcher1-wrapper-content-length.js @@ -0,0 +1,117 @@ +'use strict' + +const { test, after } = require('node:test') +const assert = require('node:assert') +const { createServer } = require('node:http') +const { once } = require('node:events') +const { Agent, Dispatcher1Wrapper } = require('..') + +// See https://github.com/nodejs/undici/issues/5500 + +function legacyDispatch (wrapper, opts) { + return new Promise((resolve, reject) => { + let body = '' + wrapper.dispatch(opts, { + onConnect () {}, + onHeaders (statusCode) { + this.statusCode = statusCode + return true + }, + onData (chunk) { + body += chunk + return true + }, + onComplete () { + resolve({ statusCode: this.statusCode, body }) + }, + onError (err) { + reject(err) + } + }) + }) +} + +async function startServer (t) { + const server = createServer((req, res) => { + let length = 0 + req.on('data', (chunk) => { length += chunk.length }) + req.on('end', () => { + res.end(`${length}:${req.headers['content-length']}`) + }) + }) + server.listen(0) + await once(server, 'listening') + return server +} + +test('collapses identical repeated content-length from a legacy consumer', async (t) => { + const server = await startServer(t) + const agent = new Agent() + const wrapper = new Dispatcher1Wrapper(agent) + after(() => { server.close(); return agent.close() }) + + const { statusCode, body } = await legacyDispatch(wrapper, { + origin: `http://127.0.0.1:${server.address().port}`, + path: '/', + method: 'POST', + headers: { 'Content-Length': '13, 13', 'content-type': 'text/plain' }, + body: 'update=INSERT' + }) + + assert.strictEqual(statusCode, 200) + assert.strictEqual(body, '13:13') +}) + +test('collapses identical repeated content-length in flat array headers', async (t) => { + const server = await startServer(t) + const agent = new Agent() + const wrapper = new Dispatcher1Wrapper(agent) + after(() => { server.close(); return agent.close() }) + + const { statusCode, body } = await legacyDispatch(wrapper, { + origin: `http://127.0.0.1:${server.address().port}`, + path: '/', + method: 'POST', + headers: ['content-length', '13 , 13', 'content-type', 'text/plain'], + body: 'update=INSERT' + }) + + assert.strictEqual(statusCode, 200) + assert.strictEqual(body, '13:13') +}) + +test('still rejects conflicting repeated content-length', async (t) => { + const server = await startServer(t) + const agent = new Agent() + const wrapper = new Dispatcher1Wrapper(agent) + after(() => { server.close(); return agent.close() }) + + await assert.rejects( + legacyDispatch(wrapper, { + origin: `http://127.0.0.1:${server.address().port}`, + path: '/', + method: 'POST', + headers: { 'content-length': '10, 13' }, + body: 'update=INSERT' + }), + { code: 'UND_ERR_INVALID_ARG', message: 'invalid content-length header' } + ) +}) + +test('does not mutate the caller-provided headers object', async (t) => { + const server = await startServer(t) + const agent = new Agent() + const wrapper = new Dispatcher1Wrapper(agent) + after(() => { server.close(); return agent.close() }) + + const headers = { 'content-length': '13, 13' } + await legacyDispatch(wrapper, { + origin: `http://127.0.0.1:${server.address().port}`, + path: '/', + method: 'POST', + headers, + body: 'update=INSERT' + }) + + assert.strictEqual(headers['content-length'], '13, 13') +}) diff --git a/test/fixtures/global-fetch-content-length.js b/test/fixtures/global-fetch-content-length.js new file mode 100644 index 00000000000..42e030bcf78 --- /dev/null +++ b/test/fixtures/global-fetch-content-length.js @@ -0,0 +1,38 @@ +'use strict' + +// Exercising Node's *global* fetch/Headers against the dispatcher bridge +// is the point. See https://github.com/nodejs/undici/issues/5500 +/* eslint-disable no-restricted-globals */ + +require('../..') + +const http = require('node:http') +const { once } = require('node:events') + +async function main () { + const server = http.createServer((req, res) => { + let length = 0 + req.on('data', (chunk) => { length += chunk.length }) + req.on('end', () => res.end(String(length))) + }) + server.listen(0) + await once(server, 'listening') + + try { + const body = 'update=INSERT' + const headers = new Headers() + headers.append('Content-Type', 'application/x-www-form-urlencoded') + headers.append('Content-Length', String(body.length)) + + const url = 'http://127.0.0.1:' + server.address().port + const res = await fetch(url, { method: 'POST', headers, body }) + process.stdout.write(await res.text()) + } finally { + server.close() + } +} + +main().catch((err) => { + console.error(err?.cause?.stack || err?.stack || err) + process.exitCode = 1 +}) diff --git a/test/node-test/global-dispatcher-version.js b/test/node-test/global-dispatcher-version.js index 8fa688043e6..bfbfa24f05e 100644 --- a/test/node-test/global-dispatcher-version.js +++ b/test/node-test/global-dispatcher-version.js @@ -98,6 +98,18 @@ test('setGlobalDispatcher mirrors a v1-compatible dispatcher that Node.js global assert.strictEqual(payload.mirroredV2, true) }) +test('requiring undici does not break Node.js global fetch with a request-set Content-Length', () => { + // See https://github.com/nodejs/undici/issues/5500 + const result = spawnSync( + process.execPath, + [join(__dirname, '../fixtures/global-fetch-content-length.js')], + { cwd, encoding: 'utf8' } + ) + + assert.strictEqual(result.status, 0, result.stderr) + assert.strictEqual(result.stdout, '13') +}) + test('Dispatcher1Wrapper bridges legacy handlers to a new Agent', () => { const script = ` const { Agent, Dispatcher1Wrapper } = require('./index.js')