From 8e025d1cf5a453ff158cb1bd40ff6929e1b27bb1 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 16 Oct 2024 20:23:47 +0200 Subject: [PATCH 1/8] fix(#3736): leaked error event on response body (#3740) --- lib/api/api-request.js | 2 +- test/client-request.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 2c86dd07c58..1bf07b392aa 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -65,7 +65,7 @@ class RequestHandler extends AsyncResource { this.removeAbortListener = util.addAbortListener(signal, () => { this.reason = signal.reason ?? new RequestAbortedError() if (this.res) { - util.destroy(this.res, this.reason) + util.destroy(this.res.on('error', noop), this.reason) } else if (this.abort) { this.abort(this.reason) } diff --git a/test/client-request.js b/test/client-request.js index 27555d5186a..9712820b3a3 100644 --- a/test/client-request.js +++ b/test/client-request.js @@ -1337,3 +1337,39 @@ test('request multibyte text with setEncoding', async (t) => { await t.completed }) + +test('#3736 - Aborted Response (without consuming body)', async (t) => { + const plan = tspl(t, { plan: 1 }) + + const controller = new AbortController() + const server = createServer((req, res) => { + setTimeout(() => { + res.writeHead(200, 'ok', { + 'content-type': 'text/plain' + }) + res.write('hello from server') + res.end() + }, 100) + }) + + server.listen(0) + + await EE.once(server, 'listening') + const client = new Client(`http://localhost:${server.address().port}`) + + after(server.close.bind(server)) + after(client.destroy.bind(client)) + + const { signal } = controller + const promise = client.request({ + path: '/', + method: 'GET', + signal + }) + + controller.abort() + + await plan.rejects(promise, { message: 'This operation was aborted' }) + + await plan.completed +}) From 30305063937a87e70561d2aeb5795788ed628ca2 Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Fri, 18 Oct 2024 17:28:16 -0700 Subject: [PATCH 2/8] fix: unsafe methods not causing cache purge (#3739) * fix: unsafe methods not causing cache purge Fixes bug introduced in #3733 where unsafe methods no longer make it to the cache handler and cause a cache purge for an origin. Also removes a duplicate test file. Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> * Update cache.js * extend test to check that safe methods we're not caching don't purge the cache Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> * code review Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --------- Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- lib/handler/cache-handler.js | 10 +- lib/interceptor/cache.js | 4 +- test/cache-interceptor/interceptor.js | 240 -------------------------- test/interceptors/cache.js | 57 +++++- 4 files changed, 61 insertions(+), 250 deletions(-) delete mode 100644 test/cache-interceptor/interceptor.js diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 3c4044a653e..d48675c3760 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -16,11 +16,6 @@ class CacheHandler extends DecoratorHandler { */ #store - /** - * @type {import('../../types/cache-interceptor.d.ts').default.CacheMethods} - */ - #methods - /** * @type {import('../../types/dispatcher.d.ts').default.RequestOptions} */ @@ -42,14 +37,13 @@ class CacheHandler extends DecoratorHandler { * @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler */ constructor (opts, requestOptions, handler) { - const { store, methods } = opts + const { store } = opts super(handler) this.#store = store this.#requestOptions = requestOptions this.#handler = handler - this.#methods = methods } /** @@ -75,7 +69,7 @@ class CacheHandler extends DecoratorHandler { ) if ( - !this.#methods.includes(this.#requestOptions.method) && + !util.safeHTTPMethods.includes(this.#requestOptions.method) && statusCode >= 200 && statusCode <= 399 ) { diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index b03734aa886..63d56b6880b 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -30,9 +30,11 @@ module.exports = (opts = {}) => { methods } + const safeMethodsToNotCache = util.safeHTTPMethods.filter(method => methods.includes(method) === false) + return dispatch => { return (opts, handler) => { - if (!opts.origin || !methods.includes(opts.method)) { + if (!opts.origin || safeMethodsToNotCache.includes(opts.method)) { // Not a method we want to cache or we don't have the origin, skip return dispatch(opts, handler) } diff --git a/test/cache-interceptor/interceptor.js b/test/cache-interceptor/interceptor.js deleted file mode 100644 index 40cad118bba..00000000000 --- a/test/cache-interceptor/interceptor.js +++ /dev/null @@ -1,240 +0,0 @@ -'use strict' - -const { describe, test, after } = require('node:test') -const { strictEqual, notEqual, fail } = require('node:assert') -const { createServer } = require('node:http') -const { once } = require('node:events') -const { Client, interceptors, cacheStores } = require('../../index') - -describe('Cache Interceptor', () => { - test('doesn\'t cache request w/ no cache-control header', async () => { - let requestsToOrigin = 0 - - const server = createServer((_, res) => { - requestsToOrigin++ - res.end('asd') - }).listen(0) - - after(() => server.close()) - await once(server, 'listening') - - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache()) - - strictEqual(requestsToOrigin, 0) - - // Send initial request. This should reach the origin - let response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/' - }) - strictEqual(requestsToOrigin, 1) - strictEqual(await response.body.text(), 'asd') - - // Send second request that should be handled by cache - response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/' - }) - strictEqual(requestsToOrigin, 2) - strictEqual(await response.body.text(), 'asd') - }) - - test('caches request successfully', async () => { - let requestsToOrigin = 0 - - const server = createServer((_, res) => { - requestsToOrigin++ - res.setHeader('cache-control', 'public, s-maxage=10') - res.end('asd') - }).listen(0) - - after(() => server.close()) - await once(server, 'listening') - - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache()) - - strictEqual(requestsToOrigin, 0) - - // Send initial request. This should reach the origin - let response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/' - }) - strictEqual(requestsToOrigin, 1) - strictEqual(await response.body.text(), 'asd') - - // Send second request that should be handled by cache - response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/' - }) - strictEqual(requestsToOrigin, 1) - strictEqual(await response.body.text(), 'asd') - strictEqual(response.headers.age, '0') - }) - - test('respects vary header', async () => { - let requestsToOrigin = 0 - - const server = createServer((req, res) => { - requestsToOrigin++ - res.setHeader('cache-control', 'public, s-maxage=10') - res.setHeader('vary', 'some-header, another-header') - - if (req.headers['some-header'] === 'abc123') { - res.end('asd') - } else { - res.end('dsa') - } - }).listen(0) - - after(() => server.close()) - await once(server, 'listening') - - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache()) - - strictEqual(requestsToOrigin, 0) - - // Send initial request. This should reach the origin - let response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/', - headers: { - 'some-header': 'abc123', - 'another-header': '123abc' - } - }) - strictEqual(requestsToOrigin, 1) - strictEqual(await response.body.text(), 'asd') - - // Make another request with changed headers, this should miss - const secondResponse = await client.request({ - method: 'GET', - path: '/', - headers: { - 'some-header': 'qwerty', - 'another-header': 'asdfg' - } - }) - strictEqual(requestsToOrigin, 2) - strictEqual(await secondResponse.body.text(), 'dsa') - - // Resend the first request again which should still be cahced - response = await client.request({ - origin: 'localhost', - method: 'GET', - path: '/', - headers: { - 'some-header': 'abc123', - 'another-header': '123abc' - } - }) - strictEqual(requestsToOrigin, 2) - strictEqual(await response.body.text(), 'asd') - }) - - test('revalidates request when needed', async () => { - let requestsToOrigin = 0 - - const server = createServer((req, res) => { - res.setHeader('cache-control', 'public, s-maxage=1, stale-while-revalidate=10') - - requestsToOrigin++ - - if (requestsToOrigin > 1) { - notEqual(req.headers['if-modified-since'], undefined) - - if (requestsToOrigin === 3) { - res.end('asd123') - } else { - res.statusCode = 304 - res.end() - } - } else { - res.end('asd') - } - }).listen(0) - - after(() => server.close()) - await once(server, 'listening') - - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache()) - - strictEqual(requestsToOrigin, 0) - - const request = { - origin: 'localhost', - method: 'GET', - path: '/' - } - - // Send initial request. This should reach the origin - let response = await client.request(request) - strictEqual(requestsToOrigin, 1) - strictEqual(await response.body.text(), 'asd') - - // Now we send two more requests. Both of these should reach the origin, - // but now with a conditional header asking if the resource has been - // updated. These need to be ran after the response is stale. - const completed = new Promise((resolve, reject) => { - setTimeout(async () => { - try { - // No update for the second request - response = await client.request(request) - strictEqual(requestsToOrigin, 2) - strictEqual(await response.body.text(), 'asd') - - // This should be updated, even though the value isn't expired. - response = await client.request(request) - strictEqual(requestsToOrigin, 3) - strictEqual(await response.body.text(), 'asd123') - - resolve() - } catch (e) { - reject(e) - } - }, 1500) - }) - await completed - }) - - test('respects cache store\'s isFull property', async () => { - const server = createServer((_, res) => { - res.end('asd') - }).listen(0) - - after(() => server.close()) - await once(server, 'listening') - - const store = new cacheStores.MemoryCacheStore() - Object.defineProperty(store, 'isFull', { - value: true - }) - - store.createWriteStream = (...args) => { - fail('shouln\'t have reached this') - } - - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache({ store })) - - await client.request({ - origin: 'localhost', - method: 'GET', - path: '/', - headers: { - 'some-header': 'abc123', - 'another-header': '123abc' - } - }) - }) -}) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 302fc734b4b..c99cb52279f 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1,7 +1,7 @@ 'use strict' const { describe, test, after } = require('node:test') -const { strictEqual, notEqual, fail } = require('node:assert') +const { strictEqual, notEqual, fail, equal } = require('node:assert') const { createServer } = require('node:http') const { once } = require('node:events') const FakeTimers = require('@sinonjs/fake-timers') @@ -250,4 +250,59 @@ describe('Cache Interceptor', () => { } }) }) + + test('unsafe methods call the store\'s deleteByOrigin function', async () => { + const server = createServer((_, res) => { + res.end('asd') + }).listen(0) + + after(() => server.close()) + await once(server, 'listening') + + let deleteByOriginCalled = false + const store = new cacheStores.MemoryCacheStore() + + const originalDeleteByOrigin = store.deleteByOrigin.bind(store) + store.deleteByOrigin = (origin) => { + deleteByOriginCalled = true + originalDeleteByOrigin(origin) + } + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache({ + store, + methods: ['GET'] // explicitly only cache GET methods + })) + + // Make sure safe methods that we want to cache don't cause a cache purge + await client.request({ + origin: 'localhost', + method: 'GET', + path: '/' + }) + + equal(deleteByOriginCalled, false) + + // Make sure other safe methods that we don't want to cache don't cause a cache purge + await client.request({ + origin: 'localhost', + method: 'HEAD', + path: '/' + }) + + strictEqual(deleteByOriginCalled, false) + + // Make sure the common unsafe methods cause cache purges + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + deleteByOriginCalled = false + + await client.request({ + origin: 'localhost', + method, + path: '/' + }) + + equal(deleteByOriginCalled, true, method) + } + }) }) From 7bb663e073ba9ee573e1f86c51b5c59ba76b6380 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 23 Oct 2024 22:15:39 +0200 Subject: [PATCH 3/8] build(deps): bump node from `83b4d7b` to `f1b4315` in /build (#3756) Bumps node from `83b4d7b` to `f1b4315`. --- updated-dependencies: - dependency-name: node dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Dockerfile b/build/Dockerfile index cabe308dd88..bd330c61c81 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -1,4 +1,4 @@ -FROM node:22-alpine3.19@sha256:83b4d7bcfc3d4a40faac3e73a59bc3b0f4b3cc72b9a19e036d340746ebfeaecb +FROM node:22-alpine3.19@sha256:f1b43157ce277feaed97088f4d1bbf6b209148d49d98cea592e0af6637657baf ARG UID=1000 ARG GID=1000 From 02c61d2612953ab810457340762579aef7f78b0f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 24 Oct 2024 13:19:54 +0200 Subject: [PATCH 4/8] Bumped v7.0.0-alpha.3 Signed-off-by: Matteo Collina --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ab4dcf422f1..fb7b4181fa9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "undici", - "version": "7.0.0-alpha.2", + "version": "7.0.0-alpha.3", "description": "An HTTP/1.1 client, written from scratch for Node.js", "homepage": "https://undici.nodejs.org", "bugs": { From 211cc27824f00b8174bd84a4f464727e1bd7e585 Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 25 Oct 2024 09:45:52 -0400 Subject: [PATCH 5/8] fix filename* parsing (#3768) * fix filename* parsing * fixup --- lib/web/fetch/body.js | 6 +- lib/web/fetch/formdata-parser.js | 113 +++++++++++++++++++------------ test/busboy/issue-3760.js | 59 ++++++++++++++++ 3 files changed, 130 insertions(+), 48 deletions(-) create mode 100644 test/busboy/issue-3760.js diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js index 99355d32e78..b092b3c83db 100644 --- a/lib/web/fetch/body.js +++ b/lib/web/fetch/body.js @@ -364,12 +364,8 @@ function bodyMixinMethods (instance, getInternalState) { switch (mimeType.essence) { case 'multipart/form-data': { // 1. ... [long step] - const parsed = multipartFormDataParser(value, mimeType) - // 2. If that fails for some reason, then throw a TypeError. - if (parsed === 'failure') { - throw new TypeError('Failed to parse body as FormData.') - } + const parsed = multipartFormDataParser(value, mimeType) // 3. Return a new FormData object, appending each entry, // resulting from the parsing operation, to its entry list. diff --git a/lib/web/fetch/formdata-parser.js b/lib/web/fetch/formdata-parser.js index 4bbc2c909fe..f43b5fd98e5 100644 --- a/lib/web/fetch/formdata-parser.js +++ b/lib/web/fetch/formdata-parser.js @@ -11,7 +11,7 @@ const { File: NodeFile } = require('node:buffer') const File = globalThis.File ?? NodeFile const formDataNameBuffer = Buffer.from('form-data; name="') -const filenameBuffer = Buffer.from('; filename') +const filenameBuffer = Buffer.from('filename') const dd = Buffer.from('--') const ddcrlf = Buffer.from('--\r\n') @@ -75,7 +75,7 @@ function multipartFormDataParser (input, mimeType) { // Otherwise, let boundary be the result of UTF-8 decoding mimeType’s // parameters["boundary"]. if (boundaryString === undefined) { - return 'failure' + throw parsingError('missing boundary in content-type header') } const boundary = Buffer.from(`--${boundaryString}`, 'utf8') @@ -111,7 +111,7 @@ function multipartFormDataParser (input, mimeType) { if (input.subarray(position.position, position.position + boundary.length).equals(boundary)) { position.position += boundary.length } else { - return 'failure' + throw parsingError('expected a value starting with -- and the boundary') } // 5.2. If position points to the sequence of bytes 0x2D 0x2D 0x0D 0x0A @@ -127,7 +127,7 @@ function multipartFormDataParser (input, mimeType) { // 5.3. If position does not point to a sequence of bytes starting with 0x0D // 0x0A (CR LF), return failure. if (input[position.position] !== 0x0d || input[position.position + 1] !== 0x0a) { - return 'failure' + throw parsingError('expected CRLF') } // 5.4. Advance position by 2. (This skips past the newline.) @@ -138,10 +138,6 @@ function multipartFormDataParser (input, mimeType) { // is not failure. Otherwise, return failure. const result = parseMultipartFormDataHeaders(input, position) - if (result === 'failure') { - return 'failure' - } - let { name, filename, contentType, encoding } = result // 5.6. Advance position by 2. (This skips past the empty line that marks @@ -157,7 +153,7 @@ function multipartFormDataParser (input, mimeType) { const boundaryIndex = input.indexOf(boundary.subarray(2), position.position) if (boundaryIndex === -1) { - return 'failure' + throw parsingError('expected boundary after body') } body = input.subarray(position.position, boundaryIndex - 4) @@ -174,7 +170,7 @@ function multipartFormDataParser (input, mimeType) { // 5.9. If position does not point to a sequence of bytes starting with // 0x0D 0x0A (CR LF), return failure. Otherwise, advance position by 2. if (input[position.position] !== 0x0d || input[position.position + 1] !== 0x0a) { - return 'failure' + throw parsingError('expected CRLF') } else { position.position += 2 } @@ -230,7 +226,7 @@ function parseMultipartFormDataHeaders (input, position) { if (input[position.position] === 0x0d && input[position.position + 1] === 0x0a) { // 2.1.1. If name is null, return failure. if (name === null) { - return 'failure' + throw parsingError('header name is null') } // 2.1.2. Return name, filename and contentType. @@ -250,12 +246,12 @@ function parseMultipartFormDataHeaders (input, position) { // 2.4. If header name does not match the field-name token production, return failure. if (!HTTP_TOKEN_CODEPOINTS.test(headerName.toString())) { - return 'failure' + throw parsingError('header name does not match the field-name token production') } // 2.5. If the byte at position is not 0x3A (:), return failure. if (input[position.position] !== 0x3a) { - return 'failure' + throw parsingError('expected :') } // 2.6. Advance position by 1. @@ -278,7 +274,7 @@ function parseMultipartFormDataHeaders (input, position) { // 2. If position does not point to a sequence of bytes starting with // `form-data; name="`, return failure. if (!bufferStartsWith(input, formDataNameBuffer, position)) { - return 'failure' + throw parsingError('expected form-data; name=" for content-disposition header') } // 3. Advance position so it points at the byte after the next 0x22 (") @@ -290,34 +286,61 @@ function parseMultipartFormDataHeaders (input, position) { // failure. name = parseMultipartFormDataName(input, position) - if (name === null) { - return 'failure' - } - // 5. If position points to a sequence of bytes starting with `; filename="`: - if (bufferStartsWith(input, filenameBuffer, position)) { - // Note: undici also handles filename* - let check = position.position + filenameBuffer.length - - if (input[check] === 0x2a) { - position.position += 1 - check += 1 - } - - if (input[check] !== 0x3d || input[check + 1] !== 0x22) { // =" - return 'failure' - } - - // 1. Advance position so it points at the byte after the next 0x22 (") byte - // (the one in the sequence of bytes matched above). - position.position += 12 - - // 2. Set filename to the result of parsing a multipart/form-data name given - // input and position, if the result is not failure. Otherwise, return failure. - filename = parseMultipartFormDataName(input, position) - - if (filename === null) { - return 'failure' + if (input[position.position] === 0x3b /* ; */ && input[position.position + 1] === 0x20 /* ' ' */) { + const at = { position: position.position + 2 } + + if (bufferStartsWith(input, filenameBuffer, at)) { + if (input[at.position + 8] === 0x2a /* '*' */) { + at.position += 10 // skip past filename*= + + // Remove leading http tab and spaces. See RFC for examples. + // https://datatracker.ietf.org/doc/html/rfc6266#section-5 + collectASequenceOfBytes( + (char) => char === 0x20 || char === 0x09, + input, + at + ) + + const headerValue = collectASequenceOfBytes( + (char) => char !== 0x20 && char !== 0x0d && char !== 0x0a, // ' ' or CRLF + input, + at + ) + + if ( + (headerValue[0] !== 0x75 && headerValue[0] !== 0x55) || // u or U + (headerValue[1] !== 0x74 && headerValue[1] !== 0x54) || // t or T + (headerValue[2] !== 0x66 && headerValue[2] !== 0x46) || // f or F + headerValue[3] !== 0x2d || // - + headerValue[4] !== 0x38 // 8 + ) { + throw parsingError('unknown encoding, expected utf-8\'\'') + } + + // skip utf-8'' + filename = decodeURIComponent(new TextDecoder().decode(headerValue.subarray(7))) + + position.position = at.position + } else { + // 1. Advance position so it points at the byte after the next 0x22 (") byte + // (the one in the sequence of bytes matched above). + position.position += 11 + + // Remove leading http tab and spaces. See RFC for examples. + // https://datatracker.ietf.org/doc/html/rfc6266#section-5 + collectASequenceOfBytes( + (char) => char === 0x20 || char === 0x09, + input, + position + ) + + position.position++ // skip past " after removing whitespace + + // 2. Set filename to the result of parsing a multipart/form-data name given + // input and position, if the result is not failure. Otherwise, return failure. + filename = parseMultipartFormDataName(input, position) + } } } @@ -367,7 +390,7 @@ function parseMultipartFormDataHeaders (input, position) { // 2.9. If position does not point to a sequence of bytes starting with 0x0D 0x0A // (CR LF), return failure. Otherwise, advance position by 2 (past the newline). if (input[position.position] !== 0x0d && input[position.position + 1] !== 0x0a) { - return 'failure' + throw parsingError('expected CRLF') } else { position.position += 2 } @@ -393,7 +416,7 @@ function parseMultipartFormDataName (input, position) { // 3. If the byte at position is not 0x22 ("), return failure. Otherwise, advance position by 1. if (input[position.position] !== 0x22) { - return null // name could be 'failure' + throw parsingError('expected "') } else { position.position++ } @@ -468,6 +491,10 @@ function bufferStartsWith (buffer, start, position) { return true } +function parsingError (cause) { + return new TypeError('Failed to parse body as FormData.', { cause: new TypeError(cause) }) +} + module.exports = { multipartFormDataParser, validateBoundary diff --git a/test/busboy/issue-3760.js b/test/busboy/issue-3760.js new file mode 100644 index 00000000000..d37852c8fac --- /dev/null +++ b/test/busboy/issue-3760.js @@ -0,0 +1,59 @@ +'use strict' + +const { test } = require('node:test') +const assert = require('node:assert') +const { Response } = require('../..') + +// https://github.com/nodejs/undici/issues/3760 +test('filename* parameter is parsed properly', async (t) => { + const response = new Response([ + '--83d82e0d-9ced-44c0-ac79-4e66a827415b\r\n' + + 'Content-Type: text/plain\r\n' + + 'Content-Disposition: form-data; name="file"; filename*=UTF-8\'\'%e2%82%ac%20rates\r\n' + + '\r\n' + + 'testabc\r\n' + + '--83d82e0d-9ced-44c0-ac79-4e66a827415b--\r\n' + + '\r\n' + ].join(''), { + headers: { + 'content-type': 'multipart/form-data; boundary="83d82e0d-9ced-44c0-ac79-4e66a827415b"' + } + }) + + const fd = await response.formData() + assert.deepEqual(fd.get('file').name, '€ rates') +}) + +test('whitespace after filename[*]= is ignored', async () => { + for (const response of [ + new Response([ + '--83d82e0d-9ced-44c0-ac79-4e66a827415b\r\n' + + 'Content-Type: text/plain\r\n' + + 'Content-Disposition: form-data; name="file"; filename*= utf-8\'\'hello\r\n' + + '\r\n' + + 'testabc\r\n' + + '--83d82e0d-9ced-44c0-ac79-4e66a827415b--\r\n' + + '\r\n' + ].join(''), { + headers: { + 'content-type': 'multipart/form-data; boundary="83d82e0d-9ced-44c0-ac79-4e66a827415b"' + } + }), + new Response([ + '--83d82e0d-9ced-44c0-ac79-4e66a827415b\r\n' + + 'Content-Type: text/plain\r\n' + + 'Content-Disposition: form-data; name="file"; filename= "hello"\r\n' + + '\r\n' + + 'testabc\r\n' + + '--83d82e0d-9ced-44c0-ac79-4e66a827415b--\r\n' + + '\r\n' + ].join(''), { + headers: { + 'content-type': 'multipart/form-data; boundary="83d82e0d-9ced-44c0-ac79-4e66a827415b"' + } + }) + ]) { + const fd = await response.formData() + assert.deepEqual(fd.get('file').name, 'hello') + } +}) From c8dc113d22e2f622189091ef987a58964f513dee Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 25 Oct 2024 09:56:33 -0400 Subject: [PATCH 6/8] fix http2 test (#3769) --- test/http2.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/test/http2.js b/test/http2.js index d6840a1bd15..3f7ab3deb24 100644 --- a/test/http2.js +++ b/test/http2.js @@ -334,23 +334,15 @@ test( after(() => server.close()) after(() => client.close()) - t = tspl(t, { plan: 2 }) + t = tspl(t, { plan: 1 }) - try { - await client.request({ - path: '/', - method: 'GET', - headers: { - 'x-my-header': 'foo' - } - }) - } catch (error) { - t.strictEqual( - error.message, - 'Client network socket disconnected before secure TLS connection was established' - ) - t.strictEqual(error.code, 'ECONNRESET') - } + await t.rejects(client.request({ + path: '/', + method: 'GET', + headers: { + 'x-my-header': 'foo' + } + })) } ) From eac8ed48c5365a2f7c5a18d03d8fc2bcbe5f011e Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 25 Oct 2024 14:47:54 -0400 Subject: [PATCH 7/8] add unsafe-url referrerPolicy test (#3772) --- test/fetch/issue-3767.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 test/fetch/issue-3767.js diff --git a/test/fetch/issue-3767.js b/test/fetch/issue-3767.js new file mode 100644 index 00000000000..fec7c71dbe6 --- /dev/null +++ b/test/fetch/issue-3767.js @@ -0,0 +1,30 @@ +'use strict' + +const { once } = require('node:events') +const { createServer } = require('node:http') +const { test } = require('node:test') +const { fetch } = require('../..') +const { tspl } = require('@matteo.collina/tspl') + +// https://github.com/nodejs/undici/issues/3767 +test('referrerPolicy unsafe-url is respected', async (t) => { + const { completed, deepEqual } = tspl(t, { plan: 1 }) + + const referrer = 'https://google.com/hello/world' + + const server = createServer((req, res) => { + deepEqual(req.headers.referer, referrer) + + res.end() + }).listen(0) + + t.after(server.close.bind(server)) + await once(server, 'listening') + + await fetch(`http://localhost:${server.address().port}`, { + referrer, + referrerPolicy: 'unsafe-url' + }) + + await completed +}) From 2e79b622874ede53471d4f82ee53b1bafc03436f Mon Sep 17 00:00:00 2001 From: Steven Date: Sat, 26 Oct 2024 06:34:55 -0400 Subject: [PATCH 8/8] chore(docs): add request() example for conditionally reading the body (#3743) --- docs/docs/api/Dispatcher.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/docs/api/Dispatcher.md b/docs/docs/api/Dispatcher.md index 3903eca3d3c..d531efec07c 100644 --- a/docs/docs/api/Dispatcher.md +++ b/docs/docs/api/Dispatcher.md @@ -527,6 +527,7 @@ try { console.log('headers', headers) body.setEncoding('utf8') body.on('data', console.log) + body.on('error', console.error) body.on('end', () => { console.log('trailers', trailers) }) @@ -630,6 +631,25 @@ try { } ``` +#### Example 3 - Conditionally reading the body + +Remember to fully consume the body even in the case when it is not read. + +```js +const { body, statusCode } = await client.request({ + path: '/', + method: 'GET' +}) + +if (statusCode === 200) { + return await body.arrayBuffer() +} + +await body.dump() + +return null +``` + ### `Dispatcher.stream(options, factory[, callback])` A faster version of `Dispatcher.request`. This method expects the second argument `factory` to return a [`stream.Writable`](https://nodejs.org/api/stream.html#stream_class_stream_writable) stream which the response will be written to. This improves performance by avoiding creating an intermediate [`stream.Readable`](https://nodejs.org/api/stream.html#stream_readable_streams) stream when the user expects to directly pipe the response body to a [`stream.Writable`](https://nodejs.org/api/stream.html#stream_class_stream_writable) stream.