From 37e6dad9f0f6834eac056807ffbd8f9c852a01ee Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Wed, 6 Apr 2022 06:49:45 -0400 Subject: [PATCH 1/6] Fix mock bugs (#1323) * fix(MockAgent): allow lowercase `method` * fix(MockAgent): set statusText properly * add test for bad statusCode * skip test on v14 and lower * code cov --- lib/mock/mock-interceptor.js | 3 ++ lib/mock/mock-utils.js | 77 +++++++++++++++++++++++++++++++++++- test/mock-agent.js | 36 +++++++++++++++++ test/mock-client.js | 18 ++++++++- test/mock-utils.js | 22 ++++++++++- 5 files changed, 153 insertions(+), 3 deletions(-) diff --git a/lib/mock/mock-interceptor.js b/lib/mock/mock-interceptor.js index 699bec41287..5af8b1083f0 100644 --- a/lib/mock/mock-interceptor.js +++ b/lib/mock/mock-interceptor.js @@ -74,6 +74,9 @@ class MockInterceptor { const parsedURL = new URL(opts.path, 'data://') opts.path = parsedURL.pathname + parsedURL.search } + if (typeof opts.method === 'string') { + opts.method = opts.method.toUpperCase() + } this[kDispatchKey] = buildKey(opts) this[kDispatches] = mockDispatches diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 8bd4df51a09..65e21c6bf5c 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -140,6 +140,80 @@ function generateKeyValues (data) { return Object.entries(data).reduce((keyValuePairs, [key, value]) => [...keyValuePairs, key, value], []) } +/** + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status + * @param {number} statusCode + */ +function getStatusText (statusCode) { + switch (statusCode) { + case 100: return 'Continue' + case 101: return 'Switching Protocols' + case 102: return 'Processing' + case 103: return 'Early Hints' + case 200: return 'OK' + case 201: return 'Created' + case 202: return 'Accepted' + case 203: return 'Non-Authoritative Information' + case 204: return 'No Content' + case 205: return 'Reset Content' + case 206: return 'Partial Content' + case 207: return 'Multi-Status' + case 208: return 'Already Reported' + case 226: return 'IM Used' + case 300: return 'Multiple Choice' + case 301: return 'Moved Permanently' + case 302: return 'Found' + case 303: return 'See Other' + case 304: return 'Not Modified' + case 305: return 'Use Proxy' + case 306: return 'unused' + case 307: return 'Temporary Redirect' + case 308: return 'Permanent Redirect' + case 400: return 'Bad Request' + case 401: return 'Unauthorized' + case 402: return 'Payment Required' + case 403: return 'Forbidden' + case 404: return 'Not Found' + case 405: return 'Method Not Allowed' + case 406: return 'Not Acceptable' + case 407: return 'Proxy Authentication Required' + case 408: return 'Request Timeout' + case 409: return 'Conflict' + case 410: return 'Gone' + case 411: return 'Length Required' + case 412: return 'Precondition Failed' + case 413: return 'Payload Too Large' + case 414: return 'URI Too Large' + case 415: return 'Unsupported Media Type' + case 416: return 'Range Not Satisfiable' + case 417: return 'Expectation Failed' + case 418: return 'I\'m a teapot' + case 421: return 'Misdirected Request' + case 422: return 'Unprocessable Entity' + case 423: return 'Locked' + case 424: return 'Failed Dependency' + case 425: return 'Too Early' + case 426: return 'Upgrade Required' + case 428: return 'Precondition Required' + case 429: return 'Too Many Requests' + case 431: return 'Request Header Fields Too Large' + case 451: return 'Unavailable For Legal Reasons' + case 500: return 'Internal Server Error' + case 501: return 'Not Implemented' + case 502: return 'Bad Gateway' + case 503: return 'Service Unavailable' + case 504: return 'Gateway Timeout' + case 505: return 'HTTP Version Not Supported' + case 506: return 'Variant Also Negotiates' + case 507: return 'Insufficient Storage' + case 508: return 'Loop Detected' + case 510: return 'Not Extended' + case 511: return 'Network Authentication Required' + default: + throw new ReferenceError(`Unknown status code "${statusCode}"!`) + } +} + async function getResponse (body) { const buffers = [] for await (const data of body) { @@ -197,7 +271,7 @@ function mockDispatch (opts, handler) { const responseHeaders = generateKeyValues(headers) const responseTrailers = generateKeyValues(trailers) - handler.onHeaders(statusCode, responseHeaders, resume) + handler.onHeaders(statusCode, responseHeaders, resume, getStatusText(statusCode)) handler.onData(Buffer.from(responseData)) handler.onComplete(responseTrailers) deleteMockDispatch(mockDispatches, key) @@ -264,6 +338,7 @@ module.exports = { generateKeyValues, matchValue, getResponse, + getStatusText, mockDispatch, buildMockDispatch, checkNetConnect, diff --git a/test/mock-agent.js b/test/mock-agent.js index 3c4edbf81b9..00e6f1e693d 100644 --- a/test/mock-agent.js +++ b/test/mock-agent.js @@ -13,6 +13,8 @@ const { kAgent } = require('../lib/mock/mock-symbols') const Dispatcher = require('../lib/dispatcher') const { MockNotMatchedError } = require('../lib/mock/mock-errors') +const nodeMajor = Number(process.versions.node.split('.')[0]) + test('MockAgent - constructor', t => { t.plan(5) @@ -2396,3 +2398,37 @@ test('MockAgent - clients are not garbage collected', async (t) => { t.equal(results.size, 1) t.ok(results.has(200)) }) + +// https://github.com/nodejs/undici/issues/1321 +test('MockAgent - using fetch yields correct statusText', { skip: nodeMajor < 16 }, async (t) => { + const { fetch } = require('..') + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + setGlobalDispatcher(mockAgent) + t.teardown(mockAgent.close.bind(mockAgent)) + + const mockPool = mockAgent.get('http://localhost:3000') + + mockPool.intercept({ + path: '/statusText', + method: 'GET' + }).reply(200, 'Body') + + const { status, statusText } = await fetch('http://localhost:3000/statusText') + + t.equal(status, 200) + t.equal(statusText, 'OK') + + mockPool.intercept({ + path: '/badStatusText', + method: 'GET' + }).reply(420, 'Everyday') + + await t.rejects( + fetch('http://localhost:3000/badStatusText'), + TypeError + ) + + t.end() +}) diff --git a/test/mock-client.js b/test/mock-client.js index e5a89af3d4c..3700954c5c2 100644 --- a/test/mock-client.js +++ b/test/mock-client.js @@ -125,7 +125,7 @@ test('MockClient - intercept should return a MockInterceptor', (t) => { }) test('MockClient - intercept validation', (t) => { - t.plan(3) + t.plan(4) t.test('it should error if no options specified in the intercept', t => { t.plan(1) @@ -155,6 +155,22 @@ test('MockClient - intercept validation', (t) => { const mockClient = mockAgent.get('http://localhost:9999') t.doesNotThrow(() => mockClient.intercept({ path: '/foo' })) }) + + t.test('it should uppercase the method - https://github.com/nodejs/undici/issues/1320', t => { + t.plan(1) + + const mockAgent = new MockAgent() + const mockClient = mockAgent.get('http://localhost:3000') + + t.teardown(mockAgent.close.bind(mockAgent)) + + mockClient.intercept({ + path: '/test', + method: 'patch' + }).reply(200, 'Hello!') + + t.equal(mockClient[kDispatches][0].method, 'PATCH') + }) }) test('MockClient - close should run without error', async (t) => { diff --git a/test/mock-utils.js b/test/mock-utils.js index 296143aa319..3e6637e7989 100644 --- a/test/mock-utils.js +++ b/test/mock-utils.js @@ -5,7 +5,8 @@ const { MockNotMatchedError } = require('../lib/mock/mock-errors') const { deleteMockDispatch, getMockDispatch, - getResponseData + getResponseData, + getStatusText } = require('../lib/mock/mock-utils') test('deleteMockDispatch - should do nothing if not able to find mock dispatch', (t) => { @@ -109,3 +110,22 @@ test('getResponseData', (t) => { t.ok(Buffer.isBuffer(responseData)) }) }) + +test('getStatusText', (t) => { + for (const statusCode of [ + 100, 101, 102, 103, 200, 201, 202, 203, + 204, 205, 206, 207, 208, 226, 300, 301, + 302, 303, 304, 305, 306, 307, 308, 400, + 401, 402, 403, 404, 405, 406, 407, 408, + 409, 410, 411, 412, 413, 414, 415, 416, + 417, 418, 421, 422, 423, 424, 425, 426, + 428, 429, 431, 451, 500, 501, 502, 503, + 504, 505, 506, 507, 508, 510, 511 + ]) { + t.ok(getStatusText(statusCode)) + } + + t.throws(() => getStatusText(420), ReferenceError) + + t.end() +}) From 89411abba88d71bb0341360166f07ea1e0e539e3 Mon Sep 17 00:00:00 2001 From: Andrew Powell Date: Wed, 6 Apr 2022 12:54:56 -0400 Subject: [PATCH 2/6] docs: expand mixins information, notate body reuse (#1209) * docs: expand mixins information, notate body reuse * chore: apply suggestions from code review Co-authored-by: Robert Nagy * docs: remove ref to web stream. Co-authored-by: Matteo Collina * docs: remove stream clone info * Update README.md Co-authored-by: Robert Nagy Co-authored-by: Matteo Collina --- README.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a4cc09a0373..60c2b6fbc48 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,15 @@ for await (const data of body) { console.log('trailers', trailers) ``` -Using [the body mixin from the Fetch Standard](https://fetch.spec.whatwg.org/#body-mixin). +## Body Mixins + +The `body` mixins are the most common way to format the request/response body. Mixins include: + +- [`.formData()`](https://fetch.spec.whatwg.org/#dom-body-formdata) +- [`.json()`](https://fetch.spec.whatwg.org/#dom-body-json) +- [`.text()`](https://fetch.spec.whatwg.org/#dom-body-text) + +Example usage: ```js import { request } from 'undici' @@ -83,6 +91,12 @@ console.log('data', await body.json()) console.log('trailers', trailers) ``` +_Note: Once a mixin has been called then the body cannot be reused, thus calling additional mixins on `.body`, e.g. `.body.json(); .body.text()` will result in an error `TypeError: unusable` being thrown and returned through the `Promise` rejection._ + +Should you need to access the `body` in plain-text after using a mixin, the best practice is to use the `.text()` mixin first, and manually parse the text to the desired format. + +For more information about their behavior, please reference the body mixin from the [Fetch Standard](https://fetch.spec.whatwg.org/#body-mixin). + ## Common API Methods This section documents our most commonly used API methods. Additional APIs are documented in their own files within the [docs](./docs/) folder and are accessible via the navigation list on the left side of the docs site. From c0961f4c7ef5dd6e64dbb7ffbd3f6cb1f78f106a Mon Sep 17 00:00:00 2001 From: Rafael Silva Date: Sat, 9 Apr 2022 10:27:21 -0300 Subject: [PATCH 3/6] fetch: replace HeadersList Array inheritance to Map (#1309) * update: uses Map over Array in HeadersList * fix: create empty HeaderList * update(fetch/headers): sort keys before returning * update: remove binarySearch method * perf: use headersList.set to bypass validation * update: remove comment * fix: lint * update(headers): create a new headers map for sorting * update: remove kSorted * update: replace .set to .append * fixes * fix: lint Co-authored-by: Robert Nagy --- lib/fetch/headers.js | 148 ++++++++++++++++++++-------------------- lib/fetch/index.js | 6 +- lib/fetch/request.js | 13 +++- lib/fetch/response.js | 19 +++--- test/fetch/headers.js | 79 +++++---------------- test/node-fetch/mock.js | 5 +- 6 files changed, 115 insertions(+), 155 deletions(-) diff --git a/lib/fetch/headers.js b/lib/fetch/headers.js index a5dd5b7a413..e0295be2250 100644 --- a/lib/fetch/headers.js +++ b/lib/fetch/headers.js @@ -11,22 +11,8 @@ const { forbiddenResponseHeaderNames } = require('./constants') -function binarySearch (arr, val) { - let low = 0 - let high = Math.floor(arr.length / 2) - - while (high > low) { - const mid = (high + low) >>> 1 - - if (val.localeCompare(arr[mid * 2]) > 0) { - low = mid + 1 - } else { - high = mid - } - } - - return low * 2 -} +const kHeadersMap = Symbol('headers map') +const kHeadersSortedMap = Symbol('headers map sorted') function normalizeAndValidateHeaderName (name) { if (name === undefined) { @@ -91,64 +77,74 @@ function fill (headers, object) { } } -// TODO: Composition over inheritence? Or helper methods? -class HeadersList extends Array { +class HeadersList { + constructor (init) { + if (init instanceof HeadersList) { + this[kHeadersMap] = new Map(init[kHeadersMap]) + this[kHeadersSortedMap] = init[kHeadersSortedMap] + } else { + this[kHeadersMap] = new Map(init) + this[kHeadersSortedMap] = null + } + } + append (name, value) { + this[kHeadersSortedMap] = null + const normalizedName = normalizeAndValidateHeaderName(name) const normalizedValue = normalizeAndValidateHeaderValue(name, value) - const index = binarySearch(this, normalizedName) + const exists = this[kHeadersMap].get(normalizedName) - if (this[index] === normalizedName) { - this[index + 1] += `, ${normalizedValue}` + if (exists) { + this[kHeadersMap].set(normalizedName, `${exists}, ${normalizedValue}`) } else { - this.splice(index, 0, normalizedName, normalizedValue) + this[kHeadersMap].set(normalizedName, `${normalizedValue}`) } } - delete (name) { + set (name, value) { + this[kHeadersSortedMap] = null + const normalizedName = normalizeAndValidateHeaderName(name) + return this[kHeadersMap].set(normalizedName, value) + } - const index = binarySearch(this, normalizedName) + delete (name) { + this[kHeadersSortedMap] = null - if (this[index] === normalizedName) { - this.splice(index, 2) - } + const normalizedName = normalizeAndValidateHeaderName(name) + return this[kHeadersMap].delete(normalizedName) } get (name) { const normalizedName = normalizeAndValidateHeaderName(name) - - const index = binarySearch(this, normalizedName) - - if (this[index] === normalizedName) { - return this[index + 1] - } - - return null + return this[kHeadersMap].get(normalizedName) ?? null } has (name) { const normalizedName = normalizeAndValidateHeaderName(name) + return this[kHeadersMap].has(normalizedName) + } - const index = binarySearch(this, normalizedName) + keys () { + return this[kHeadersMap].keys() + } - return this[index] === normalizedName + values () { + return this[kHeadersMap].values() } - set (name, value) { - const normalizedName = normalizeAndValidateHeaderName(name) - const normalizedValue = normalizeAndValidateHeaderValue(name, value) + entries () { + return this[kHeadersMap].entries() + } - const index = binarySearch(this, normalizedName) - if (this[index] === normalizedName) { - this[index + 1] = normalizedValue - } else { - this.splice(index, 0, normalizedName, normalizedValue) - } + [Symbol.iterator] () { + return this[kHeadersMap][Symbol.iterator]() } } +// https://fetch.spec.whatwg.org/#headers-class class Headers { constructor (...args) { if ( @@ -161,7 +157,6 @@ class Headers { ) } const init = args.length >= 1 ? args[0] ?? {} : {} - this[kHeadersList] = new HeadersList() // The new Headers(init) constructor steps are: @@ -287,20 +282,18 @@ class Headers { ) } - const normalizedName = normalizeAndValidateHeaderName(String(args[0])) - if (this[kGuard] === 'immutable') { throw new TypeError('immutable') } else if ( this[kGuard] === 'request' && - forbiddenHeaderNames.includes(normalizedName) + forbiddenHeaderNames.includes(String(args[0]).toLocaleLowerCase()) ) { return } else if (this[kGuard] === 'request-no-cors') { // TODO } else if ( this[kGuard] === 'response' && - forbiddenResponseHeaderNames.includes(normalizedName) + forbiddenResponseHeaderNames.includes(String(args[0]).toLocaleLowerCase()) ) { return } @@ -308,25 +301,41 @@ class Headers { return this[kHeadersList].set(String(args[0]), String(args[1])) } - * keys () { - const clone = this[kHeadersList].slice() - for (let index = 0; index < clone.length; index += 2) { - yield clone[index] + get [kHeadersSortedMap] () { + this[kHeadersList][kHeadersSortedMap] ??= new Map([...this[kHeadersList]].sort((a, b) => a[0] < b[0] ? -1 : 1)) + return this[kHeadersList][kHeadersSortedMap] + } + + keys () { + if (!(this instanceof Headers)) { + throw new TypeError('Illegal invocation') } + + return this[kHeadersSortedMap].keys() } - * values () { - const clone = this[kHeadersList].slice() - for (let index = 1; index < clone.length; index += 2) { - yield clone[index] + values () { + if (!(this instanceof Headers)) { + throw new TypeError('Illegal invocation') } + + return this[kHeadersSortedMap].values() } - * entries () { - const clone = this[kHeadersList].slice() - for (let index = 0; index < clone.length; index += 2) { - yield [clone[index], clone[index + 1]] + entries () { + if (!(this instanceof Headers)) { + throw new TypeError('Illegal invocation') } + + return this[kHeadersSortedMap].entries() + } + + [Symbol.iterator] () { + if (!(this instanceof Headers)) { + throw new TypeError('Illegal invocation') + } + + return this[kHeadersSortedMap] } forEach (...args) { @@ -346,15 +355,9 @@ class Headers { const callback = args[0] const thisArg = args[1] - const clone = this[kHeadersList].slice() - for (let index = 0; index < clone.length; index += 2) { - callback.call( - thisArg, - clone[index + 1], - clone[index], - this - ) - } + this[kHeadersSortedMap].forEach((value, index) => { + callback.apply(thisArg, [value, index, this]) + }) } [Symbol.for('nodejs.util.inspect.custom')] () { @@ -384,7 +387,6 @@ module.exports = { fill, Headers, HeadersList, - binarySearch, normalizeAndValidateHeaderName, normalizeAndValidateHeaderValue } diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 90479764eae..1fbf29b9db6 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -780,7 +780,7 @@ async function schemeFetch (fetchParams) { const resp = makeResponse({ statusText: 'OK', headersList: [ - 'content-type', 'text/html;charset=utf-8' + ['content-type', 'text/html;charset=utf-8'] ] }) @@ -871,7 +871,7 @@ async function schemeFetch (fetchParams) { return makeResponse({ statusText: 'OK', headersList: [ - 'content-type', contentType + ['content-type', contentType] ], body: extractBody(dataURLStruct.body)[0] }) @@ -1919,7 +1919,7 @@ async function httpNetworkFetch ( origin: url.origin, method: request.method, body: fetchParams.controller.dispatcher[kIsMockActive] ? request.body && request.body.source : body, - headers: request.headersList, + headers: [...request.headersList].flat(), maxRedirections: 0 }, { diff --git a/lib/fetch/request.js b/lib/fetch/request.js index 151dc8e4411..0f10e678894 100644 --- a/lib/fetch/request.js +++ b/lib/fetch/request.js @@ -420,7 +420,15 @@ class Request { // 4. If headers is a Headers object, then for each header in its header // list, append header’s name/header’s value to this’s headers. if (headers instanceof Headers) { - this[kState].headersList.push(...headers[kHeadersList]) + // TODO (fix): Why doesn't this work? + // for (const [key, val] of headers[kHeadersList]) { + // this[kHeaders].append(key, val) + // } + + this[kState].headersList = new HeadersList([ + ...this[kState].headersList, + ...headers[kHeadersList] + ]) } else { // 5. Otherwise, fill this’s headers with headers. fillHeaders(this[kState].headersList, headers) @@ -460,6 +468,7 @@ class Request { // this’s headers. if (contentType && !this[kHeaders].has('content-type')) { this[kHeaders].append('content-type', contentType) + this[kState].headersList.append('content-type', contentType) } } @@ -793,7 +802,7 @@ function makeRequest (init) { timingAllowFailed: false, ...init, headersList: init.headersList - ? new HeadersList(...init.headersList) + ? new HeadersList(init.headersList) : new HeadersList(), urlList: init.urlList ? [...init.urlList.map((url) => new URL(url))] : [] } diff --git a/lib/fetch/response.js b/lib/fetch/response.js index 64fc3170dac..d1ce6ce95bd 100644 --- a/lib/fetch/response.js +++ b/lib/fetch/response.js @@ -81,7 +81,7 @@ class Response { const value = parsedURL.toString() // 7. Append `Location`/value to responseObject’s response’s header list. - responseObject[kState].headersList.push('location', value) + responseObject[kState].headersList.append('location', value) // 8. Return responseObject. return responseObject @@ -172,7 +172,7 @@ class Response { // not contain `Content-Type`, then append `Content-Type`/Content-Type // to this’s response’s header list. if (contentType && !this.headers.has('content-type')) { - this.headers.set('content-type', contentType) + this.headers.append('content-type', contentType) } } } @@ -350,7 +350,7 @@ function makeResponse (init) { statusText: '', ...init, headersList: init.headersList - ? new HeadersList(...init.headersList) + ? new HeadersList(init.headersList) : new HeadersList(), urlList: init.urlList ? [...init.urlList] : [] } @@ -394,16 +394,13 @@ function makeFilteredHeadersList (headersList, filter) { // Override methods used by Headers class. if (prop === 'get' || prop === 'has') { return (name) => filter(name) ? target[prop](name) : undefined - } else if (prop === 'slice') { - return (...args) => { - assert(args.length === 0) - const arr = [] - for (let index = 0; index < target.length; index += 2) { - if (filter(target[index])) { - arr.push(target[index], target[index + 1]) + } else if (prop === Symbol.iterator) { + return function * () { + for (const entry of target) { + if (filter(entry[0])) { + yield entry } } - return arr } } else { return target[prop] diff --git a/test/fetch/headers.js b/test/fetch/headers.js index 044e4d1508b..4c4e2171915 100644 --- a/test/fetch/headers.js +++ b/test/fetch/headers.js @@ -1,15 +1,12 @@ 'use strict' -const util = require('util') const tap = require('tap') const { Headers, - binarySearch, normalizeAndValidateHeaderName, normalizeAndValidateHeaderValue, fill } = require('../../lib/fetch/headers') -const { kHeadersList } = require('../../lib/core/symbols') const { kGuard } = require('../../lib/fetch/symbols') const { forbiddenHeaderNames, @@ -101,11 +98,12 @@ tap.test('Headers initialization', t => { t.test('accepts headers as objects with array values', t => { t.plan(1) const headers = new Headers({ - a: ['1', '2'], + c: '5', b: ['3', '4'], - c: '5' + a: ['1', '2'] }) - t.same(headers.entries(), [ + + t.same([...headers.entries()], [ ['a', '1,2'], ['b', '3,4'], ['c', '5'] @@ -322,7 +320,7 @@ tap.test('Headers as Iterable', t => { const order = [] for (const [key] of headers) { order.push(key) - headers.set(key + key, 1) + headers.append(key + key, 1) } t.strictSame(order, ['x', 'y', 'z']) @@ -333,7 +331,7 @@ tap.test('Headers as Iterable', t => { }) t.test('returns combined and sorted entries using .forEach()', t => { - t.plan(12) + t.plan(8) const init = [ ['a', '1'], ['b', '2'], @@ -352,7 +350,6 @@ tap.test('Headers as Iterable', t => { let i = 0 headers.forEach(function (value, key, _headers) { t.strictSame(expected[i++], [key, value]) - t.equal(headers, _headers) t.equal(this, that) }, that) }) @@ -445,53 +442,17 @@ tap.test('Headers as Iterable', t => { headers.append('c', '7') headers.append('abc', '8') - const expected = [ - 'a', '1', - 'abc', '8', - 'b', '2', - 'c', '3, 7', - 'd', '4', - 'e', '5', - 'f', '6' - ] - - t.same(headers[kHeadersList], expected) - }) -}) + const expected = [...new Map([ + ['a', '1'], + ['abc', '8'], + ['b', '2'], + ['c', '3, 7'], + ['d', '4'], + ['e', '5'], + ['f', '6'] + ])] -tap.test('binary search', t => { - // 0 1 2 3 4 5 6 7 - const l1 = ['b', 1, 'c', 2, 'd', 3, 'f', 4] - // 0 1 2 3 4 5 6 7 8 9 - const l2 = ['b', 1, 'c', 2, 'd', 3, 'e', 4, 'g', 5] - // 0 1 2 3 4 5 6 7 - const l3 = ['a', 1, 'b', 2, 'bcd', 3, 'c', 4] - // 0 1 2 3 4 5 6 7 8 9 - const l4 = ['a', 1, 'b', 2, 'c', 3, 'cde', 4, 'f', 5] - - const tests = [ - { input: [l1, 'c'], expected: 2, message: 'find item in n=even array' }, - { input: [l1, 'f'], expected: 6, message: 'find item at end of n=even array' }, - { input: [l1, 'b'], expected: 0, message: 'find item at beg of n=even array' }, - { input: [l1, 'e'], expected: 6, message: 'find new item position in n=even array' }, - { input: [l1, 'g'], expected: 8, message: 'find new item position at end of n=even array' }, - { input: [l1, 'a'], expected: 0, message: 'find new item position at beg of n=even array' }, - { input: [l2, 'c'], expected: 2, message: 'find item in n=odd array' }, - { input: [l2, 'g'], expected: 8, message: 'find item at end of n=odd array' }, - { input: [l2, 'b'], expected: 0, message: 'find item at beg of n=odd array' }, - { input: [l2, 'f'], expected: 8, message: 'find new item position in n=odd array' }, - { input: [l2, 'h'], expected: 10, message: 'find new item position at end of n=odd array' }, - { input: [l2, 'a'], expected: 0, message: 'find new item position at beg of n=odd array' }, - { input: [l3, 'b'], expected: 2, message: 'find item with similarity in n=odd array' }, - { input: [l3, 'bcd'], expected: 4, message: 'find item with similarity in n=odd array' }, - { input: [l4, 'c'], expected: 4, message: 'find item with similarity in n=odd array' }, - { input: [l4, 'cde'], expected: 6, message: 'find item with similarity in n=odd array' } - ] - - t.plan(tests.length) - - tests.forEach(({ input: [list, target], expected, message }) => { - t.equal(expected, binarySearch(list, target), message) + t.same([...headers], expected) }) }) @@ -599,14 +560,6 @@ tap.test('various init paths of Headers', (t) => { t.end() }) -tap.test('node inspect', (t) => { - const headers = new Headers() - headers.set('k1', 'v1') - headers.set('k2', 'v2') - t.equal(util.inspect(headers), "HeadersList(4) [ 'k1', 'v1', 'k2', 'v2' ]") - t.end() -}) - tap.test('immutable guard', (t) => { const headers = new Headers() headers.set('key', 'val') diff --git a/test/node-fetch/mock.js b/test/node-fetch/mock.js index d4881eabc35..a53f464a1a9 100644 --- a/test/node-fetch/mock.js +++ b/test/node-fetch/mock.js @@ -68,8 +68,8 @@ describe('node-fetch with MockAgent', () => { .intercept({ path: '/test', method: 'GET', - headers: { - 'User-Agent': /^undici$/ + headers: (h) => { + return h['user-agent'] === 'undici' } }) .reply(200, { success: true }) @@ -79,7 +79,6 @@ describe('node-fetch with MockAgent', () => { method: 'GET', headers: new Headers({ 'User-Agent': 'undici' }) }) - expect(res.status).to.equal(200) expect(await res.json()).to.deep.equal({ success: true }) }) From 2ea8f0375dfa83e71d14e68fd90af51af2b868a4 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Wed, 13 Apr 2022 00:06:52 -0400 Subject: [PATCH 4/6] feat: allow `FormData` bodies in `request` (#1332) * feat: allow `FormData` bodies in `request` * add test for FormData body on node <16 * fix: bad version check * test: add busboy formdata parsing --- lib/client.js | 24 ++++++++++++- lib/core/util.js | 7 +++- test/client-request.js | 66 +++++++++++++++++++++++++++++++++++ test/types/formdata.test-d.ts | 7 +++- test/utils/formdata.js | 49 ++++++++++++++++++++++++++ types/dispatcher.d.ts | 3 +- 6 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 test/utils/formdata.js diff --git a/lib/client.js b/lib/client.js index d3d4cfc705d..5c1ca952031 100644 --- a/lib/client.js +++ b/lib/client.js @@ -67,6 +67,12 @@ const { const kClosedResolve = Symbol('kClosedResolve') const channels = {} +const [nodeMajor, nodeMinor] = process.version + .slice(1) // remove 'v' + .split('.', 2) + .map(v => Number(v)) + +let extractBody try { const diagnosticsChannel = require('diagnostics_channel') @@ -1580,10 +1586,26 @@ async function writeIterable ({ body, client, request, socket, contentLength, he .on('close', onDrain) .on('drain', onDrain) + let bodyAsyncIterable = body + + if (util.isFormDataLike(body)) { + if (nodeMajor < 16 || (nodeMajor === 16 && nodeMinor < 5)) { + throw new InvalidArgumentError('Form-Data bodies are only supported in node v16.5 and newer.') + } + + if (!extractBody) { + extractBody = require('./fetch/body.js').extractBody + } + + const [bodyStream, contentType] = extractBody(body) + header += `content-type: ${contentType}\r\n` + bodyAsyncIterable = bodyStream.stream + } + const writer = new AsyncWriter({ socket, request, contentLength, client, expectsPayload, header }) try { // It's up to the user to somehow abort the async iterable. - for await (const chunk of body) { + for await (const chunk of bodyAsyncIterable) { if (socket[kError]) { throw socket[kError] } diff --git a/lib/core/util.js b/lib/core/util.js index 9d028cadc11..0cb60b0d2a4 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -324,6 +324,10 @@ function ReadableStreamFrom (iterable) { ) } +function isFormDataLike (chunk) { + return chunk && chunk.constructor && chunk.constructor.name === 'FormData' +} + const kEnumerableProperty = Object.create(null) kEnumerableProperty.enumerable = true @@ -352,5 +356,6 @@ module.exports = { ReadableStreamFrom, isBuffer, validateHandler, - getSocketInfo + getSocketInfo, + isFormDataLike } diff --git a/test/client-request.js b/test/client-request.js index 1956dc61bb1..8e81c20715d 100644 --- a/test/client-request.js +++ b/test/client-request.js @@ -9,6 +9,7 @@ const { Readable } = require('stream') const net = require('net') const { promisify } = require('util') const { NotSupportedError } = require('../lib/core/errors') +const { parseFormDataString } = require('./utils/formdata') const nodeMajor = Number(process.versions.node.split('.')[0]) @@ -657,3 +658,68 @@ test('request text2', (t) => { t.strictSame(JSON.stringify(obj), await p) }) }) + +test('request with FormData body', { skip: nodeMajor < 16 }, (t) => { + const { FormData } = require('../') + const { Blob } = require('buffer') + + const fd = new FormData() + fd.set('key', 'value') + fd.set('file', new Blob(['Hello, world!']), 'hello_world.txt') + + const server = createServer(async (req, res) => { + const contentType = req.headers['content-type'] + // ensure we received a multipart/form-data header + t.ok(/^multipart\/form-data; boundary=-+formdata-undici-0.\d+$/.test(contentType)) + + const chunks = [] + + for await (const chunk of req) { + chunks.push(chunk) + } + + const { fileMap, fields } = await parseFormDataString( + Buffer.concat(chunks), + contentType + ) + + t.same(fields[0], { key: 'key', value: 'value' }) + t.ok(fileMap.has('file')) + t.equal(fileMap.get('file').data.toString(), 'Hello, world!') + t.equal(fileMap.get('file').info, 'hello_world.txt') + + return res.end() + }) + t.teardown(server.close.bind(server)) + + server.listen(0, async () => { + const client = new Client(`http://localhost:${server.address().port}`) + t.teardown(client.destroy.bind(client)) + + await client.request({ + path: '/', + method: 'POST', + body: fd + }) + + t.end() + }) +}) + +test('request with FormData body on node < 16', { skip: nodeMajor >= 16 }, async (t) => { + t.plan(1) + + // a FormData polyfill, for example + class FormData {} + + const fd = new FormData() + + const client = new Client('http://localhost:3000') + t.teardown(client.destroy.bind(client)) + + await t.rejects(client.request({ + path: '/', + method: 'POST', + body: fd + }), errors.InvalidArgumentError) +}) diff --git a/test/types/formdata.test-d.ts b/test/types/formdata.test-d.ts index db8b11e3e88..d17ff6c923f 100644 --- a/test/types/formdata.test-d.ts +++ b/test/types/formdata.test-d.ts @@ -1,6 +1,10 @@ import { Blob } from 'buffer' +import { Readable } from 'stream' import { expectAssignable, expectType } from 'tsd' -import { File, FormData } from "../.." +import { File, FormData } from '../..' +import { DispatchOptions } from '../../types/dispatcher' + +declare const dispatcherOptions: DispatchOptions declare const blob: Blob const formData = new FormData() @@ -20,3 +24,4 @@ expectAssignable>(formData.keys()) expectAssignable>(formData.values()) expectAssignable>(formData.entries()) expectAssignable>(formData[Symbol.iterator]()) +expectAssignable(dispatcherOptions.body) diff --git a/test/utils/formdata.js b/test/utils/formdata.js new file mode 100644 index 00000000000..19dac3e84e5 --- /dev/null +++ b/test/utils/formdata.js @@ -0,0 +1,49 @@ +const busboy = require('busboy') + +function parseFormDataString ( + body, + contentType +) { + const cache = { + fileMap: new Map(), + fields: [] + } + + const bb = busboy({ + headers: { + 'content-type': contentType + } + }) + + return new Promise((resolve, reject) => { + bb.on('file', (name, file, info) => { + cache.fileMap.set(name, { data: [], info }) + + file.on('data', (data) => { + const old = cache.fileMap.get(name) + + cache.fileMap.set(name, { + data: [...old.data, data], + info: old.info + }) + }).on('end', () => { + const old = cache.fileMap.get(name) + + cache.fileMap.set(name, { + data: Buffer.concat(old.data), + info: old.info + }) + }) + }) + + bb.on('field', (key, value) => cache.fields.push({ key, value })) + bb.on('close', () => resolve(cache)) + bb.on('error', (e) => reject(e)) + + bb.end(body) + }) +} + +module.exports = { + parseFormDataString +} diff --git a/types/dispatcher.d.ts b/types/dispatcher.d.ts index 0fe6cbfb9d4..9b2af26e6d2 100644 --- a/types/dispatcher.d.ts +++ b/types/dispatcher.d.ts @@ -4,6 +4,7 @@ import { EventEmitter } from 'events' import { IncomingHttpHeaders } from 'http' import { Blob } from 'buffer' import BodyReadable from './readable' +import { FormData } from './formdata' type AbortSignal = unknown; @@ -43,7 +44,7 @@ declare namespace Dispatcher { path: string; method: HttpMethod; /** Default: `null` */ - body?: string | Buffer | Uint8Array | Readable | null; + body?: string | Buffer | Uint8Array | Readable | null | FormData; /** Default: `null` */ headers?: IncomingHttpHeaders | string[] | null; /** Whether the requests can be safely retried or not. If `false` the request won't be sent until all preceding requests in the pipeline have completed. Default: `true` if `method` is `HEAD` or `GET`. */ From 63d435a23e5a057e00660135c087df7ac66bf186 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Wed, 13 Apr 2022 12:57:12 -0400 Subject: [PATCH 5/6] fix(Headers): filter out forbidden headers & `get` should return null on unknown header name (#1337) * fix: headers * re-run ci --- lib/fetch/response.js | 8 ++++++-- test/fetch/headers.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/fetch/response.js b/lib/fetch/response.js index d1ce6ce95bd..b8aa4897c1b 100644 --- a/lib/fetch/response.js +++ b/lib/fetch/response.js @@ -393,7 +393,8 @@ function makeFilteredHeadersList (headersList, filter) { get (target, prop) { // Override methods used by Headers class. if (prop === 'get' || prop === 'has') { - return (name) => filter(name) ? target[prop](name) : undefined + const defaultReturn = prop === 'has' ? false : null + return (name) => filter(name) ? target[prop](name) : defaultReturn } else if (prop === Symbol.iterator) { return function * () { for (const entry of target) { @@ -420,7 +421,10 @@ function filterResponse (response, type) { return makeFilteredResponse(response, { type: 'basic', - headersList: makeFilteredHeadersList(response.headersList, (name) => !forbiddenResponseHeaderNames.includes(name)) + headersList: makeFilteredHeadersList( + response.headersList, + (name) => !forbiddenResponseHeaderNames.includes(name.toLowerCase()) + ) }) } else if (type === 'cors') { // A CORS filtered response is a filtered response whose type is "cors" diff --git a/test/fetch/headers.js b/test/fetch/headers.js index 4c4e2171915..779c1e04f00 100644 --- a/test/fetch/headers.js +++ b/test/fetch/headers.js @@ -12,6 +12,8 @@ const { forbiddenHeaderNames, forbiddenResponseHeaderNames } = require('../../lib/fetch/constants') +const { createServer } = require('http') +const { fetch } = require('../../index') tap.test('Headers initialization', t => { t.plan(7) @@ -627,3 +629,37 @@ tap.test('response guard', (t) => { t.end() }) + +tap.test('set-cookie[2] in Headers constructor', (t) => { + const headers = new Headers(forbiddenResponseHeaderNames.map(k => [k, 'v'])) + + for (const header of forbiddenResponseHeaderNames) { + t.ok(headers.has(header)) + t.equal(headers.get(header), 'v') + } + + t.end() +}) + +// https://github.com/nodejs/undici/issues/1328 +tap.test('set-cookie[2] received from server - issue #1328', (t) => { + const server = createServer((req, res) => { + res.setHeader('set-cookie', 'my-cookie; wow') + res.end('Goodbye!') + }).unref() + t.teardown(server.close.bind(server)) + + server.listen(0, async () => { + const { headers } = await fetch(`http://localhost:${server.address().port}`) + + t.notOk(headers.has('set-cookie')) + t.notOk(headers.has('Set-cookie')) + t.notOk(headers.has('sEt-CoOkIe')) + + t.equal(headers.get('set-cookie'), null) + t.equal(headers.get('Set-cookie'), null) + t.equal(headers.get('sEt-CoOkIe'), null) + + t.end() + }) +}) From 4b5205309f308836885576aa2c6954faf305d50b Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 13 Apr 2022 19:00:03 +0200 Subject: [PATCH 6/6] fix: formdata cleanup (#1336) * fix: formdata cleanup * Update lib/core/request.js --- lib/client.js | 24 +----------------------- lib/core/request.js | 26 ++++++++++++++++++++++++-- lib/fetch/body.js | 2 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/client.js b/lib/client.js index 5c1ca952031..d3d4cfc705d 100644 --- a/lib/client.js +++ b/lib/client.js @@ -67,12 +67,6 @@ const { const kClosedResolve = Symbol('kClosedResolve') const channels = {} -const [nodeMajor, nodeMinor] = process.version - .slice(1) // remove 'v' - .split('.', 2) - .map(v => Number(v)) - -let extractBody try { const diagnosticsChannel = require('diagnostics_channel') @@ -1586,26 +1580,10 @@ async function writeIterable ({ body, client, request, socket, contentLength, he .on('close', onDrain) .on('drain', onDrain) - let bodyAsyncIterable = body - - if (util.isFormDataLike(body)) { - if (nodeMajor < 16 || (nodeMajor === 16 && nodeMinor < 5)) { - throw new InvalidArgumentError('Form-Data bodies are only supported in node v16.5 and newer.') - } - - if (!extractBody) { - extractBody = require('./fetch/body.js').extractBody - } - - const [bodyStream, contentType] = extractBody(body) - header += `content-type: ${contentType}\r\n` - bodyAsyncIterable = bodyStream.stream - } - const writer = new AsyncWriter({ socket, request, contentLength, client, expectsPayload, header }) try { // It's up to the user to somehow abort the async iterable. - for await (const chunk of bodyAsyncIterable) { + for await (const chunk of body) { if (socket[kError]) { throw socket[kError] } diff --git a/lib/core/request.js b/lib/core/request.js index 0d98987d9d1..80ff607ca27 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -11,6 +11,13 @@ const kHandler = Symbol('handler') const channels = {} +let extractBody + +const [nodeMajor, nodeMinor] = process.version + .slice(1) // remove 'v' + .split('.', 2) + .map(v => Number(v)) + try { const diagnosticsChannel = require('diagnostics_channel') channels.create = diagnosticsChannel.channel('undici:request:create') @@ -79,7 +86,7 @@ class Request { this.body = body.byteLength ? body : null } else if (typeof body === 'string') { this.body = body.length ? Buffer.from(body) : null - } else if (util.isIterable(body) || util.isBlobLike(body)) { + } else if (util.isFormDataLike(body) || util.isIterable(body) || util.isBlobLike(body)) { this.body = body } else { throw new InvalidArgumentError('body must be a string, a Buffer, a Readable stream, an iterable, or an async iterable') @@ -126,7 +133,22 @@ class Request { throw new InvalidArgumentError('headers must be an object or an array') } - if (util.isBlobLike(body) && this.contentType == null && body.type) { + if (util.isFormDataLike(this.body)) { + if (nodeMajor < 16 || (nodeMajor === 16 && nodeMinor < 5)) { + throw new InvalidArgumentError('Form-Data bodies are only supported in node v16.5 and newer.') + } + + if (!extractBody) { + extractBody = require('../fetch/body.js').extractBody + } + + const [bodyStream, contentType] = extractBody(body) + if (this.contentType == null) { + this.contentType = contentType + this.headers += `content-type: ${contentType}\r\n` + } + this.body = bodyStream.stream + } else if (util.isBlobLike(body) && this.contentType == null && body.type) { this.contentType = body.type this.headers += `content-type: ${body.type}\r\n` } diff --git a/lib/fetch/body.js b/lib/fetch/body.js index 7da6eb988b0..fe8c4333b5e 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -71,7 +71,7 @@ function extractBody (object, keepalive = false) { // Set source to a copy of the bytes held by object. source = new Uint8Array(object) - } else if (object instanceof FormData) { + } else if (util.isFormDataLike(object)) { const boundary = '----formdata-undici-' + Math.random() const prefix = `--${boundary}\r\nContent-Disposition: form-data`