diff --git a/lib/api/readable.js b/lib/api/readable.js index d106568cd4b..89913eaa621 100644 --- a/lib/api/readable.js +++ b/lib/api/readable.js @@ -16,6 +16,8 @@ const kBody = Symbol('kBody') const kAbort = Symbol('abort') const kContentType = Symbol('kContentType') +const noop = () => {} + module.exports = class BodyReadable extends Readable { constructor ({ resume, @@ -149,37 +151,50 @@ module.exports = class BodyReadable extends Readable { return this[kBody] } - async dump (opts) { + dump (opts) { let limit = opts && Number.isFinite(opts.limit) ? opts.limit : 262144 const signal = opts && opts.signal - const abortFn = () => { - this.destroy() - } - let signalListenerCleanup + if (signal) { - if (typeof signal !== 'object' || !('aborted' in signal)) { - throw new InvalidArgumentError('signal must be an AbortSignal') - } - util.throwIfAborted(signal) - signalListenerCleanup = util.addAbortListener(signal, abortFn) - } - try { - for await (const chunk of this) { - util.throwIfAborted(signal) - limit -= Buffer.byteLength(chunk) - if (limit < 0) { - return + try { + if (typeof signal !== 'object' || !('aborted' in signal)) { + throw new InvalidArgumentError('signal must be an AbortSignal') } + util.throwIfAborted(signal) + } catch (err) { + return Promise.reject(err) } - } catch { - util.throwIfAborted(signal) - } finally { - if (typeof signalListenerCleanup === 'function') { - signalListenerCleanup() - } else if (signalListenerCleanup) { - signalListenerCleanup[Symbol.dispose]() - } } + + if (this.closed) { + return Promise.resolve(null) + } + + return new Promise((resolve, reject) => { + const signalListenerCleanup = signal + ? util.addAbortListener(signal, () => { + this.destroy() + }) + : noop + + this + .on('close', function () { + signalListenerCleanup() + if (signal?.aborted) { + reject(signal.reason || Object.assign(new Error('The operation was aborted'), { name: 'AbortError' })) + } else { + resolve(null) + } + }) + .on('error', noop) + .on('data', function (chunk) { + limit -= chunk.length + if (limit <= 0) { + this.destroy() + } + }) + .resume() + }) } } diff --git a/lib/client.js b/lib/client.js index 0ac23f3ee99..70001247553 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1183,7 +1183,7 @@ async function connect (client) { const idx = hostname.indexOf(']') assert(idx !== -1) - const ip = hostname.substr(1, idx - 1) + const ip = hostname.substring(1, idx) assert(net.isIP(ip)) hostname = ip @@ -1462,23 +1462,7 @@ function _resume (client, sync) { return } - if (util.isStream(request.body) && util.bodyLength(request.body) === 0) { - request.body - .on('data', /* istanbul ignore next */ function () { - /* istanbul ignore next */ - assert(false) - }) - .on('error', function (err) { - errorRequest(client, request, err) - }) - .on('end', function () { - util.destroy(this) - }) - - request.body = null - } - - if (client[kRunning] > 0 && + if (client[kRunning] > 0 && util.bodyLength(request.body) !== 0 && (util.isStream(request.body) || util.isAsyncIterable(request.body))) { // Request with stream or iterator body can error while other requests // are inflight and indirectly error those as well. @@ -1532,7 +1516,9 @@ function write (client, request) { body.read(0) } - let contentLength = util.bodyLength(body) + const bodyLength = util.bodyLength(body) + + let contentLength = bodyLength if (contentLength === null) { contentLength = request.contentLength @@ -1630,7 +1616,7 @@ function write (client, request) { } /* istanbul ignore else: assertion */ - if (!body) { + if (!body || bodyLength === 0) { if (contentLength === 0) { socket.write(`${header}content-length: 0\r\n\r\n`, 'latin1') } else { @@ -1986,7 +1972,11 @@ function writeStream ({ h2stream, body, client, request, socket, contentLength, } } const onAbort = function () { - onFinished(new RequestAbortedError()) + if (finished) { + return + } + const err = new RequestAbortedError() + queueMicrotask(() => onFinished(err)) } const onFinished = function (err) { if (finished) { diff --git a/lib/core/request.js b/lib/core/request.js index 43973884f1a..7db05ce65ae 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -112,10 +112,29 @@ class Request { this.method = method + this.abort = null + if (body == null) { this.body = null } else if (util.isStream(body)) { this.body = body + + const rState = this.body._readableState + if (!rState || !rState.autoDestroy) { + this.endHandler = function autoDestroy () { + util.destroy(this) + } + this.body.on('end', this.endHandler) + } + + this.errorHandler = err => { + if (this.abort) { + this.abort(err) + } else { + this.error = err + } + } + this.body.on('error', this.errorHandler) } else if (util.isBuffer(body)) { this.body = body.byteLength ? body : null } else if (ArrayBuffer.isView(body)) { @@ -236,7 +255,12 @@ class Request { assert(!this.aborted) assert(!this.completed) - return this[kHandler].onConnect(abort) + if (this.error) { + abort(this.error) + } else { + this.abort = abort + return this[kHandler].onConnect(abort) + } } onHeaders (statusCode, headers, resume, statusText) { @@ -265,6 +289,8 @@ class Request { } onComplete (trailers) { + this.onFinally() + assert(!this.aborted) this.completed = true @@ -275,6 +301,8 @@ class Request { } onError (error) { + this.onFinally() + if (channels.error.hasSubscribers) { channels.error.publish({ request: this, error }) } @@ -286,6 +314,18 @@ class Request { return this[kHandler].onError(error) } + onFinally () { + if (this.errorHandler) { + this.body.off('error', this.errorHandler) + this.errorHandler = null + } + + if (this.endHandler) { + this.body.off('end', this.endHandler) + this.endHandler = null + } + } + // TODO: adjust to support H2 addHeader (key, value) { processHeader(this, key, value) diff --git a/lib/core/util.js b/lib/core/util.js index 48d24fd9a80..8d5450ba0c0 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -125,13 +125,13 @@ function getHostname (host) { const idx = host.indexOf(']') assert(idx !== -1) - return host.substr(1, idx - 1) + return host.substring(1, idx) } const idx = host.indexOf(':') if (idx === -1) return host - return host.substr(0, idx) + return host.substring(0, idx) } // IP addresses are not valid server names per RFC6066 @@ -190,7 +190,7 @@ function isReadableAborted (stream) { } function destroy (stream, err) { - if (!isStream(stream) || isDestroyed(stream)) { + if (stream == null || !isStream(stream) || isDestroyed(stream)) { return } @@ -228,7 +228,7 @@ function parseHeaders (headers, obj = {}) { if (!val) { if (Array.isArray(headers[i + 1])) { - obj[key] = headers[i + 1] + obj[key] = headers[i + 1].map(x => x.toString('utf8')) } else { obj[key] = headers[i + 1].toString('utf8') } @@ -431,16 +431,7 @@ function throwIfAborted (signal) { } } -let events function addAbortListener (signal, listener) { - if (typeof Symbol.dispose === 'symbol') { - if (!events) { - events = require('events') - } - if (typeof events.addAbortListener === 'function' && 'aborted' in signal) { - return events.addAbortListener(signal, listener) - } - } if ('addEventListener' in signal) { signal.addEventListener('abort', listener, { once: true }) return () => signal.removeEventListener('abort', listener) diff --git a/lib/fetch/headers.js b/lib/fetch/headers.js index aa5e73e5d27..bf17d690a91 100644 --- a/lib/fetch/headers.js +++ b/lib/fetch/headers.js @@ -16,6 +16,13 @@ const assert = require('assert') const kHeadersMap = Symbol('headers map') const kHeadersSortedMap = Symbol('headers map sorted') +/** + * @param {number} code + */ +function isHTTPWhiteSpaceCharCode (code) { + return code === 0x00a || code === 0x00d || code === 0x009 || code === 0x020 +} + /** * @see https://fetch.spec.whatwg.org/#concept-header-value-normalize * @param {string} potentialValue @@ -24,12 +31,12 @@ function headerValueNormalize (potentialValue) { // To normalize a byte sequence potentialValue, remove // any leading and trailing HTTP whitespace bytes from // potentialValue. + let i = 0; let j = potentialValue.length + + while (j > i && isHTTPWhiteSpaceCharCode(potentialValue.charCodeAt(j - 1))) --j + while (j > i && isHTTPWhiteSpaceCharCode(potentialValue.charCodeAt(i))) ++i - // Trimming the end with `.replace()` and a RegExp is typically subject to - // ReDoS. This is safer and faster. - let i = potentialValue.length - while (/[\r\n\t ]/.test(potentialValue.charAt(--i))); - return potentialValue.slice(0, i + 1).replace(/^[\r\n\t ]+/, '') + return i === 0 && j === potentialValue.length ? potentialValue : potentialValue.substring(i, j) } function fill (headers, object) { @@ -38,7 +45,8 @@ function fill (headers, object) { // 1. If object is a sequence, then for each header in object: // Note: webidl conversion to array has already been done. if (Array.isArray(object)) { - for (const header of object) { + for (let i = 0; i < object.length; ++i) { + const header = object[i] // 1. If header does not contain exactly two items, then throw a TypeError. if (header.length !== 2) { throw webidl.errors.exception({ @@ -48,15 +56,16 @@ function fill (headers, object) { } // 2. Append (header’s first item, header’s second item) to headers. - headers.append(header[0], header[1]) + appendHeader(headers, header[0], header[1]) } } else if (typeof object === 'object' && object !== null) { // Note: null should throw // 2. Otherwise, object is a record, then for each key → value in object, // append (key, value) to headers - for (const [key, value] of Object.entries(object)) { - headers.append(key, value) + const keys = Object.keys(object) + for (let i = 0; i < keys.length; ++i) { + appendHeader(headers, keys[i], object[keys[i]]) } } else { throw webidl.errors.conversionFailed({ @@ -67,6 +76,50 @@ function fill (headers, object) { } } +/** + * @see https://fetch.spec.whatwg.org/#concept-headers-append + */ +function appendHeader (headers, name, value) { + // 1. Normalize value. + value = headerValueNormalize(value) + + // 2. If name is not a header name or value is not a + // header value, then throw a TypeError. + if (!isValidHeaderName(name)) { + throw webidl.errors.invalidArgument({ + prefix: 'Headers.append', + value: name, + type: 'header name' + }) + } else if (!isValidHeaderValue(value)) { + throw webidl.errors.invalidArgument({ + prefix: 'Headers.append', + value, + type: 'header value' + }) + } + + // 3. If headers’s guard is "immutable", then throw a TypeError. + // 4. Otherwise, if headers’s guard is "request" and name is a + // forbidden header name, return. + // Note: undici does not implement forbidden header names + if (headers[kGuard] === 'immutable') { + throw new TypeError('immutable') + } else if (headers[kGuard] === 'request-no-cors') { + // 5. Otherwise, if headers’s guard is "request-no-cors": + // TODO + } + + // 6. Otherwise, if headers’s guard is "response" and name is a + // forbidden response-header name, return. + + // 7. Append (name, value) to headers’s header list. + return headers[kHeadersList].append(name, value) + + // 8. If headers’s guard is "request-no-cors", then remove + // privileged no-CORS request headers from headers +} + class HeadersList { /** @type {[string, string][]|null} */ cookies = null @@ -212,43 +265,7 @@ class Headers { name = webidl.converters.ByteString(name) value = webidl.converters.ByteString(value) - // 1. Normalize value. - value = headerValueNormalize(value) - - // 2. If name is not a header name or value is not a - // header value, then throw a TypeError. - if (!isValidHeaderName(name)) { - throw webidl.errors.invalidArgument({ - prefix: 'Headers.append', - value: name, - type: 'header name' - }) - } else if (!isValidHeaderValue(value)) { - throw webidl.errors.invalidArgument({ - prefix: 'Headers.append', - value, - type: 'header value' - }) - } - - // 3. If headers’s guard is "immutable", then throw a TypeError. - // 4. Otherwise, if headers’s guard is "request" and name is a - // forbidden header name, return. - // Note: undici does not implement forbidden header names - if (this[kGuard] === 'immutable') { - throw new TypeError('immutable') - } else if (this[kGuard] === 'request-no-cors') { - // 5. Otherwise, if headers’s guard is "request-no-cors": - // TODO - } - - // 6. Otherwise, if headers’s guard is "response" and name is a - // forbidden response-header name, return. - - // 7. Append (name, value) to headers’s header list. - // 8. If headers’s guard is "request-no-cors", then remove - // privileged no-CORS request headers from headers - return this[kHeadersList].append(name, value) + return appendHeader(this, name, value) } // https://fetch.spec.whatwg.org/#dom-headers-delete @@ -422,7 +439,8 @@ class Headers { const cookies = this[kHeadersList].cookies // 3. For each name of names: - for (const [name, value] of names) { + for (let i = 0; i < names.length; ++i) { + const [name, value] = names[i] // 1. If name is `set-cookie`, then: if (name === 'set-cookie') { // 1. Let values be a list of all values of headers in list whose name @@ -430,8 +448,8 @@ class Headers { // 2. For each value of values: // 1. Append (name, value) to headers. - for (const value of cookies) { - headers.push([name, value]) + for (let j = 0; j < cookies.length; ++j) { + headers.push([name, cookies[j]]) } } else { // 2. Otherwise: diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 033fa206aed..bc6fd50c68a 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -103,52 +103,57 @@ function isValidReasonPhrase (statusText) { return true } -function isTokenChar (c) { - return !( - c >= 0x7f || - c <= 0x20 || - c === '(' || - c === ')' || - c === '<' || - c === '>' || - c === '@' || - c === ',' || - c === ';' || - c === ':' || - c === '\\' || - c === '"' || - c === '/' || - c === '[' || - c === ']' || - c === '?' || - c === '=' || - c === '{' || - c === '}' - ) +/** + * @see https://tools.ietf.org/html/rfc7230#section-3.2.6 + * @param {number} c + */ +function isTokenCharCode (c) { + switch (c) { + case 0x22: + case 0x28: + case 0x29: + case 0x2c: + case 0x2f: + case 0x3a: + case 0x3b: + case 0x3c: + case 0x3d: + case 0x3e: + case 0x3f: + case 0x40: + case 0x5b: + case 0x5c: + case 0x5d: + case 0x7b: + case 0x7d: + // DQUOTE and "(),/:;<=>?@[\]{}" + return false + default: + // VCHAR %x21-7E + return c >= 0x21 && c <= 0x7e + } } -// See RFC 7230, Section 3.2.6. -// https://github.com/chromium/chromium/blob/d7da0240cae77824d1eda25745c4022757499131/third_party/blink/renderer/platform/network/http_parsers.cc#L321 +/** + * @param {string} characters + */ function isValidHTTPToken (characters) { - if (!characters || typeof characters !== 'string') { + if (characters.length === 0) { return false } for (let i = 0; i < characters.length; ++i) { - const c = characters.charCodeAt(i) - if (c > 0x7f || !isTokenChar(c)) { + if (!isTokenCharCode(characters.charCodeAt(i))) { return false } } return true } -// https://fetch.spec.whatwg.org/#header-name -// https://github.com/chromium/chromium/blob/b3d37e6f94f87d59e44662d6078f6a12de845d17/net/http/http_util.cc#L342 +/** + * @see https://fetch.spec.whatwg.org/#header-name + * @param {string} potentialValue + */ function isValidHeaderName (potentialValue) { - if (potentialValue.length === 0) { - return false - } - return isValidHTTPToken(potentialValue) } diff --git a/lib/fetch/webidl.js b/lib/fetch/webidl.js index 38a05e65759..6fcf2ab6ad4 100644 --- a/lib/fetch/webidl.js +++ b/lib/fetch/webidl.js @@ -427,12 +427,10 @@ webidl.converters.ByteString = function (V) { // 2. If the value of any element of x is greater than // 255, then throw a TypeError. for (let index = 0; index < x.length; index++) { - const charCode = x.charCodeAt(index) - - if (charCode > 255) { + if (x.charCodeAt(index) > 255) { throw new TypeError( 'Cannot convert argument to a ByteString because the character at ' + - `index ${index} has a value of ${charCode} which is greater than 255.` + `index ${index} has a value of ${x.charCodeAt(index)} which is greater than 255.` ) } } diff --git a/package.json b/package.json index 55021927723..91f71169a91 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "undici", - "version": "5.27.0", + "version": "5.27.2", "description": "An HTTP/1.1 client, written from scratch for Node.js", "homepage": "https://undici.nodejs.org", "bugs": { diff --git a/scripts/generate-undici-types-package-json.js b/scripts/generate-undici-types-package-json.js index 250cfc08aa2..78095ae6d5e 100644 --- a/scripts/generate-undici-types-package-json.js +++ b/scripts/generate-undici-types-package-json.js @@ -5,6 +5,9 @@ const packageJSONPath = path.join(__dirname, '..', 'package.json') const packageJSONRaw = fs.readFileSync(packageJSONPath, 'utf-8') const packageJSON = JSON.parse(packageJSONRaw) +const licensePath = path.join(__dirname, '..', 'LICENSE') +const licenseRaw = fs.readFileSync(licensePath, 'utf-8') + const packageTypesJSON = { name: 'undici-types', version: packageJSON.version, @@ -19,5 +22,7 @@ const packageTypesJSON = { } const packageTypesPath = path.join(__dirname, '..', 'types', 'package.json') +const licenseTypesPath = path.join(__dirname, '..', 'types', 'LICENSE') fs.writeFileSync(packageTypesPath, JSON.stringify(packageTypesJSON, null, 2)) +fs.writeFileSync(licenseTypesPath, licenseRaw) diff --git a/test/fetch/headers.js b/test/fetch/headers.js index b91609dc383..c4b4e03d297 100644 --- a/test/fetch/headers.js +++ b/test/fetch/headers.js @@ -638,6 +638,14 @@ tap.test('request-no-cors guard', (t) => { }) tap.test('invalid headers', (t) => { + t.doesNotThrow(() => new Headers({ "abcdefghijklmnopqrstuvwxyz0123456789!#$%&'*+-.^_`|~": 'test' })) + + const chars = '"(),/:;<=>?@[\\]{}'.split('') + + for (const char of chars) { + t.throws(() => new Headers({ [char]: 'test' }), TypeError, `The string "${char}" should throw an error.`) + } + for (const byte of ['\r', '\n', '\t', ' ', String.fromCharCode(128), '']) { t.throws(() => { new Headers().set(byte, 'test') diff --git a/test/fetch/http2.js b/test/fetch/http2.js index 32fb33e14eb..521c97bea97 100644 --- a/test/fetch/http2.js +++ b/test/fetch/http2.js @@ -6,13 +6,16 @@ const { once } = require('node:events') const { Blob } = require('node:buffer') const { Readable } = require('node:stream') -const { test, plan } = require('tap') +const { test, plan, skip } = require('tap') const pem = require('https-pem') const { Client, fetch } = require('../..') const nodeVersion = Number(process.version.split('v')[1].split('.')[0]) +skip('Skip H2 test due to pseudo-header issue.') +process.exit(0) + plan(6) test('[Fetch] Issue#2311', async t => { @@ -84,7 +87,7 @@ test('[Fetch] Simple GET with h2', async t => { stream.end(expectedRequestBody) }) - t.plan(3) + t.plan(4) server.listen() await once(server, 'listening') @@ -117,6 +120,9 @@ test('[Fetch] Simple GET with h2', async t => { t.equal(responseBody, expectedRequestBody) t.equal(response.headers.get('x-method'), 'GET') t.equal(response.headers.get('x-custom-h2'), 'foo') + + // See https://fetch.spec.whatwg.org/#concept-response-status-message + t.equal(response.statusText, '') }) test('[Fetch] Should handle h2 request with body (string or buffer)', async t => { diff --git a/test/mock-agent.js b/test/mock-agent.js index 4145432c2c6..d96a511a167 100644 --- a/test/mock-agent.js +++ b/test/mock-agent.js @@ -2594,5 +2594,9 @@ test('MockAgent - headers should be array of strings', async (t) => { method: 'GET' }) - t.equal(headers['set-cookie'].length, 3) + t.same(headers['set-cookie'], [ + 'foo=bar', + 'bar=baz', + 'baz=qux' + ]) }) diff --git a/test/util.js b/test/util.js index 48a21a1141f..794c68e3f77 100644 --- a/test/util.js +++ b/test/util.js @@ -83,12 +83,13 @@ test('validateHandler', (t) => { }) test('parseHeaders', (t) => { - t.plan(5) + t.plan(6) t.same(util.parseHeaders(['key', 'value']), { key: 'value' }) t.same(util.parseHeaders([Buffer.from('key'), Buffer.from('value')]), { key: 'value' }) t.same(util.parseHeaders(['Key', 'Value']), { key: 'Value' }) t.same(util.parseHeaders(['Key', 'value', 'key', 'Value']), { key: ['value', 'Value'] }) t.same(util.parseHeaders(['key', ['value1', 'value2', 'value3']]), { key: ['value1', 'value2', 'value3'] }) + t.same(util.parseHeaders([Buffer.from('key'), [Buffer.from('value1'), Buffer.from('value2'), Buffer.from('value3')]]), { key: ['value1', 'value2', 'value3'] }) }) test('parseRawHeaders', (t) => { diff --git a/test/wpt/start-websockets.mjs b/test/wpt/start-websockets.mjs index ee364ecf6c5..616584d0f97 100644 --- a/test/wpt/start-websockets.mjs +++ b/test/wpt/start-websockets.mjs @@ -11,7 +11,10 @@ function isGlobalAvailable () { return true } - return process.execArgv.includes('--experimental-websocket') + const [nodeMajor] = process.versions.node.split('.').map(v => Number(v)) + + // TODO: keep this up to date when backports to earlier majors happen + return nodeMajor >= 21 } if (process.env.CI) {