diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 74b8c5bc769..e856b98b148 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -50,7 +50,7 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@012739e5082ff0c22ca6d6ab32e07c36df03c4a4 # v2.3.3 + uses: github/codeql-action/init@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # v2.3.3 with: languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. @@ -60,7 +60,7 @@ jobs: # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild - uses: github/codeql-action/autobuild@012739e5082ff0c22ca6d6ab32e07c36df03c4a4 # v2.3.3 + uses: github/codeql-action/autobuild@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # v2.3.3 # ℹ️ Command-line programs to run using the OS shell. # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun @@ -73,6 +73,6 @@ jobs: # ./location_of_script_within_repo/buildscript.sh - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@012739e5082ff0c22ca6d6ab32e07c36df03c4a4 # v2.3.3 + uses: github/codeql-action/analyze@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # v2.3.3 with: category: "/language:${{matrix.language}}" diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 0e356c7c203..1a55ae4ddce 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -24,4 +24,4 @@ jobs: - name: 'Checkout Repository' uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 - name: 'Dependency Review' - uses: actions/dependency-review-action@6c5ccdad469c9f8a2996bfecaec55a631a347034 # v3.1.0 + uses: actions/dependency-review-action@4901385134134e04cec5fbe5ddfe3b2c5bd5d976 # v4.0.0 diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index cf613911706..750339a1349 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -22,7 +22,7 @@ jobs: timeout-minutes: 15 post-test-steps: | - name: Coverage Report - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 automerge: if: > github.event_name == 'pull_request' && github.event.pull_request.user.login == 'dependabot[bot]' diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index bd29fdf44eb..75af3b5aca4 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -51,6 +51,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@012739e5082ff0c22ca6d6ab32e07c36df03c4a4 # v3.22.12 + uses: github/codeql-action/upload-sarif@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # v3.23.2 with: sarif_file: results.sarif diff --git a/lib/fetch/formdata.js b/lib/fetch/formdata.js index ea5163f4e37..b519f890b18 100644 --- a/lib/fetch/formdata.js +++ b/lib/fetch/formdata.js @@ -2,9 +2,10 @@ const { isBlobLike, toUSVString, makeIterator } = require('./util') const { kState } = require('./symbols') +const { kEnumerableProperty } = require('../core/util') const { File: UndiciFile, FileLike, isFileLike } = require('./file') const { webidl } = require('./webidl') -const { Blob, File: NativeFile } = require('node:buffer') +const { File: NativeFile } = require('node:buffer') /** @type {globalThis['File']} */ const File = NativeFile ?? UndiciFile @@ -158,9 +159,10 @@ class FormData { webidl.brandCheck(this, FormData) return makeIterator( - () => this[kState].map(pair => [pair.name, pair.value]), + () => this[kState], 'FormData', - 'key+value' + 'key+value', + 'name', 'value' ) } @@ -168,9 +170,10 @@ class FormData { webidl.brandCheck(this, FormData) return makeIterator( - () => this[kState].map(pair => [pair.name, pair.value]), + () => this[kState], 'FormData', - 'key' + 'key', + 'name', 'value' ) } @@ -178,9 +181,10 @@ class FormData { webidl.brandCheck(this, FormData) return makeIterator( - () => this[kState].map(pair => [pair.name, pair.value]), + () => this[kState], 'FormData', - 'value' + 'value', + 'name', 'value' ) } @@ -200,7 +204,7 @@ class FormData { } for (const [key, value] of this) { - callbackFn.apply(thisArg, [value, key, this]) + callbackFn.call(thisArg, value, key, this) } } } @@ -208,6 +212,17 @@ class FormData { FormData.prototype[Symbol.iterator] = FormData.prototype.entries Object.defineProperties(FormData.prototype, { + append: kEnumerableProperty, + delete: kEnumerableProperty, + get: kEnumerableProperty, + getAll: kEnumerableProperty, + has: kEnumerableProperty, + set: kEnumerableProperty, + entries: kEnumerableProperty, + keys: kEnumerableProperty, + values: kEnumerableProperty, + forEach: kEnumerableProperty, + [Symbol.iterator]: { enumerable: false }, [Symbol.toStringTag]: { value: 'FormData', configurable: true @@ -225,7 +240,7 @@ function makeEntry (name, value, filename) { // 1. Set name to the result of converting name into a scalar value string. // "To convert a string into a scalar value string, replace any surrogates // with U+FFFD." - // see: https://nodejs.org/dist/latest-v18.x/docs/api/buffer.html#buftostringencoding-start-end + // see: https://nodejs.org/dist/latest-v20.x/docs/api/buffer.html#buftostringencoding-start-end name = Buffer.from(name).toString('utf8') // 2. If value is a string, then set value to the result of converting diff --git a/lib/fetch/headers.js b/lib/fetch/headers.js index e1619faf008..3c022a78d77 100644 --- a/lib/fetch/headers.js +++ b/lib/fetch/headers.js @@ -492,48 +492,33 @@ class Headers { keys () { webidl.brandCheck(this, Headers) - if (this[kGuard] === 'immutable') { - const value = this[kHeadersSortedMap] - return makeIterator(() => value, 'Headers', - 'key') - } - return makeIterator( - () => [...this[kHeadersSortedMap].values()], + () => this[kHeadersSortedMap], 'Headers', - 'key' + 'key', + 0, 1 ) } values () { webidl.brandCheck(this, Headers) - if (this[kGuard] === 'immutable') { - const value = this[kHeadersSortedMap] - return makeIterator(() => value, 'Headers', - 'value') - } - return makeIterator( - () => [...this[kHeadersSortedMap].values()], + () => this[kHeadersSortedMap], 'Headers', - 'value' + 'value', + 0, 1 ) } entries () { webidl.brandCheck(this, Headers) - if (this[kGuard] === 'immutable') { - const value = this[kHeadersSortedMap] - return makeIterator(() => value, 'Headers', - 'key+value') - } - return makeIterator( - () => [...this[kHeadersSortedMap].values()], + () => this[kHeadersSortedMap], 'Headers', - 'key+value' + 'key+value', + 0, 1 ) } @@ -553,7 +538,7 @@ class Headers { } for (const [key, value] of this) { - callbackFn.apply(thisArg, [value, key, this]) + callbackFn.call(thisArg, value, key, this) } } diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 9693782552f..6fe6844776e 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1099,10 +1099,10 @@ function fetchFinale (fetchParams, response) { const byteStream = new ReadableStream({ readableStream: transformStream.readable, - async start (controller) { + async pull (controller) { const reader = this.readableStream.getReader() - while (true) { + while (controller.desiredSize >= 0) { const { done, value } = await reader.read() if (done) { @@ -1113,6 +1113,7 @@ function fetchFinale (fetchParams, response) { controller.enqueue(value) } }, + queuingStrategy: new ByteLengthQueuingStrategy({ highWaterMark: 16384 }), type: 'bytes' }) @@ -1326,6 +1327,9 @@ function httpRedirectFetch (fetchParams, response) { // https://fetch.spec.whatwg.org/#cors-non-wildcard-request-header-name request.headersList.delete('authorization', true) + // https://fetch.spec.whatwg.org/#authentication-entries + request.headersList.delete('proxy-authorization', true) + // "Cookie" and "Host" are forbidden request-headers, which undici doesn't implement. request.headersList.delete('cookie', true) request.headersList.delete('host', true) @@ -1924,6 +1928,7 @@ async function httpNetworkFetch ( // cancelAlgorithm set to cancelAlgorithm. const stream = new ReadableStream( { + highWaterMark: 16384, async start (controller) { fetchParams.controller.controller = controller }, @@ -1933,7 +1938,8 @@ async function httpNetworkFetch ( async cancel (reason) { await cancelAlgorithm(reason) }, - type: 'bytes' + type: 'bytes', + queuingStrategy: new ByteLengthQueuingStrategy({ highWaterMark: 16384 }) } ) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index a306bdbbfe4..8812895bf4d 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -739,19 +739,23 @@ const esIteratorPrototype = Object.getPrototypeOf(Object.getPrototypeOf([][Symbo /** * @see https://webidl.spec.whatwg.org/#dfn-iterator-prototype-object - * @param {() => unknown[]} iterator + * @param {() => unknown} iterator * @param {string} name name of the instance * @param {'key'|'value'|'key+value'} kind + * @param {string | number} [keyIndex] + * @param {string | number} [valueIndex] */ -function makeIterator (iterator, name, kind) { +function makeIterator (iterator, name, kind, keyIndex = 0, valueIndex = 1) { const object = { index: 0, kind, target: iterator } + // The [[Prototype]] internal slot of an iterator prototype object must be %IteratorPrototype%. + const iteratorObject = Object.create(esIteratorPrototype) - const i = { - next () { + Object.defineProperty(iteratorObject, 'next', { + value: function next () { // 1. Let interface be the interface for which the iterator prototype object exists. // 2. Let thisValue be the this value. @@ -763,7 +767,7 @@ function makeIterator (iterator, name, kind) { // 5. If object is not a default iterator object for interface, // then throw a TypeError. - if (Object.getPrototypeOf(this) !== i) { + if (Object.getPrototypeOf(this) !== iteratorObject) { throw new TypeError( `'next' called on an object that does not implement interface ${name} Iterator.` ) @@ -783,68 +787,66 @@ function makeIterator (iterator, name, kind) { if (index >= len) { return { value: undefined, done: true } } - // 11. Let pair be the entry in values at index index. - const pair = values[index] - + const { [keyIndex]: key, [valueIndex]: value } = values[index] // 12. Set object’s index to index + 1. object.index = index + 1 - // 13. Return the iterator result for pair and kind. - return iteratorResult(pair, kind) + // https://webidl.spec.whatwg.org/#iterator-result + // 1. Let result be a value determined by the value of kind: + let result + switch (kind) { + case 'key': + // 1. Let idlKey be pair’s key. + // 2. Let key be the result of converting idlKey to an + // ECMAScript value. + // 3. result is key. + result = key + break + case 'value': + // 1. Let idlValue be pair’s value. + // 2. Let value be the result of converting idlValue to + // an ECMAScript value. + // 3. result is value. + result = value + break + case 'key+value': + // 1. Let idlKey be pair’s key. + // 2. Let idlValue be pair’s value. + // 3. Let key be the result of converting idlKey to an + // ECMAScript value. + // 4. Let value be the result of converting idlValue to + // an ECMAScript value. + // 5. Let array be ! ArrayCreate(2). + // 6. Call ! CreateDataProperty(array, "0", key). + // 7. Call ! CreateDataProperty(array, "1", value). + // 8. result is array. + result = [key, value] + break + } + // 2. Return CreateIterResultObject(result, false). + return { + value: result, + done: false + } }, - // The class string of an iterator prototype object for a given interface is the - // result of concatenating the identifier of the interface and the string " Iterator". - [Symbol.toStringTag]: `${name} Iterator` - } - - // The [[Prototype]] internal slot of an iterator prototype object must be %IteratorPrototype%. - Object.setPrototypeOf(i, esIteratorPrototype) - // esIteratorPrototype needs to be the prototype of i - // which is the prototype of an empty object. Yes, it's confusing. - return Object.setPrototypeOf({}, i) -} + writable: true, + enumerable: true, + configurable: true + }) -// https://webidl.spec.whatwg.org/#iterator-result -function iteratorResult (pair, kind) { - let result - - // 1. Let result be a value determined by the value of kind: - switch (kind) { - case 'key': { - // 1. Let idlKey be pair’s key. - // 2. Let key be the result of converting idlKey to an - // ECMAScript value. - // 3. result is key. - result = pair[0] - break - } - case 'value': { - // 1. Let idlValue be pair’s value. - // 2. Let value be the result of converting idlValue to - // an ECMAScript value. - // 3. result is value. - result = pair[1] - break - } - case 'key+value': { - // 1. Let idlKey be pair’s key. - // 2. Let idlValue be pair’s value. - // 3. Let key be the result of converting idlKey to an - // ECMAScript value. - // 4. Let value be the result of converting idlValue to - // an ECMAScript value. - // 5. Let array be ! ArrayCreate(2). - // 6. Call ! CreateDataProperty(array, "0", key). - // 7. Call ! CreateDataProperty(array, "1", value). - // 8. result is array. - result = pair - break - } - } + // The class string of an iterator prototype object for a given interface is the + // result of concatenating the identifier of the interface and the string " Iterator". + Object.defineProperty(iteratorObject, Symbol.toStringTag, { + value: `${name} Iterator`, + writable: false, + enumerable: false, + configurable: true + }) - // 2. Return CreateIterResultObject(result, false). - return { value: result, done: false } + // esIteratorPrototype needs to be the prototype of iteratorObject + // which is the prototype of an empty object. Yes, it's confusing. + return Object.create(iteratorObject) } /** diff --git a/package.json b/package.json index f6cbe7c8aa6..d9a39009785 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "undici", - "version": "6.6.0", + "version": "6.6.1", "description": "An HTTP/1.1 client, written from scratch for Node.js", "homepage": "https://undici.nodejs.org", "bugs": { @@ -121,7 +121,7 @@ "jest": "^29.0.2", "jsdom": "^24.0.0", "jsfuzz": "^1.0.15", - "mitata": "^0.1.6", + "mitata": "^0.1.8", "mocha": "^10.0.0", "p-timeout": "^3.2.0", "pre-commit": "^1.2.2", diff --git a/test/fetch/pull-dont-push.js b/test/fetch/pull-dont-push.js new file mode 100644 index 00000000000..24bad7c3923 --- /dev/null +++ b/test/fetch/pull-dont-push.js @@ -0,0 +1,53 @@ +'use strict' + +const { test } = require('node:test') +const assert = require('node:assert') +const { fetch } = require('../..') +const { createServer } = require('http') +const { once } = require('events') +const { Readable, pipeline } = require('stream') +const { setTimeout: sleep } = require('timers/promises') + +const { closeServerAsPromise } = require('../utils/node-http') + +test('Allow the usage of custom implementation of AbortController', async (t) => { + let count = 0 + let socket + const server = createServer((req, res) => { + res.statusCode = 200 + socket = res.socket + + // infinite stream + const stream = new Readable({ + read () { + this.push('a') + if (count++ > 1000000) { + this.push(null) + } + } + }) + + pipeline(stream, res, () => {}) + }) + + t.after(closeServerAsPromise(server)) + + server.listen(0) + await once(server, 'listening') + + t.diagnostic('server listening on port %d', server.address().port) + const res = await fetch(`http://localhost:${server.address().port}`) + t.diagnostic('fetched') + + // Some time is needed to fill the buffer + await sleep(1000) + + assert.strictEqual(socket.bytesWritten < 1024 * 1024, true) // 1 MB + socket.destroy() + + // consume the stream + try { + /* eslint-disable-next-line no-empty, no-unused-vars */ + for await (const chunk of res.body) {} + } catch {} +}) diff --git a/test/fetch/redirect-cross-origin-header.js b/test/fetch/redirect-cross-origin-header.js index 5c1d91c9924..3756c22d417 100644 --- a/test/fetch/redirect-cross-origin-header.js +++ b/test/fetch/redirect-cross-origin-header.js @@ -7,11 +7,12 @@ const { once } = require('node:events') const { fetch } = require('../..') test('Cross-origin redirects clear forbidden headers', async (t) => { - const { strictEqual } = tspl(t, { plan: 5 }) + const { strictEqual } = tspl(t, { plan: 6 }) const server1 = createServer((req, res) => { strictEqual(req.headers.cookie, undefined) strictEqual(req.headers.authorization, undefined) + strictEqual(req.headers['proxy-authorization'], undefined) res.end('redirected') }).listen(0) @@ -40,7 +41,8 @@ test('Cross-origin redirects clear forbidden headers', async (t) => { const res = await fetch(`http://localhost:${server2.address().port}`, { headers: { Authorization: 'test', - Cookie: 'ddd=dddd' + Cookie: 'ddd=dddd', + 'Proxy-Authorization': 'test' } }) diff --git a/test/node-test/debug.js b/test/node-test/debug.js index 53eec7aa774..f60500da2b2 100644 --- a/test/node-test/debug.js +++ b/test/node-test/debug.js @@ -6,7 +6,7 @@ const { join } = require('node:path') const { tspl } = require('@matteo.collina/tspl') test('debug#websocket', async t => { - const assert = tspl(t, { plan: 5 }) + const assert = tspl(t, { plan: 6 }) const child = spawn( process.execPath, [join(__dirname, '../fixtures/websocket.js')], @@ -27,25 +27,23 @@ test('debug#websocket', async t => { /(WEBSOCKET [0-9]+:) (closed connection to)/ ] - t.after(() => { - child.kill() - }) - child.stderr.setEncoding('utf8') child.stderr.on('data', chunk => { chunks.push(chunk) }) child.stderr.on('end', () => { + assert.strictEqual(chunks.length, assertions.length) for (let i = 1; i < chunks.length; i++) { assert.match(chunks[i], assertions[i]) } }) await assert.completed + child.kill() }) test('debug#fetch', async t => { - const assert = tspl(t, { plan: 5 }) + const assert = tspl(t, { plan: 6 }) const child = spawn( process.execPath, [join(__dirname, '../fixtures/fetch.js')], @@ -62,26 +60,24 @@ test('debug#fetch', async t => { /(FETCH [0-9]+:) (trailers received)/ ] - t.after(() => { - child.kill() - }) - child.stderr.setEncoding('utf8') child.stderr.on('data', chunk => { chunks.push(chunk) }) child.stderr.on('end', () => { + assert.strictEqual(chunks.length, assertions.length) for (let i = 0; i < chunks.length; i++) { assert.match(chunks[i], assertions[i]) } }) await assert.completed + child.kill() }) test('debug#undici', async t => { // Due to Node.js webpage redirect - const assert = tspl(t, { plan: 5 }) + const assert = tspl(t, { plan: 6 }) const child = spawn( process.execPath, [join(__dirname, '../fixtures/undici.js')], @@ -100,19 +96,17 @@ test('debug#undici', async t => { /(UNDICI [0-9]+:) (trailers received)/ ] - t.after(() => { - child.kill() - }) - child.stderr.setEncoding('utf8') child.stderr.on('data', chunk => { chunks.push(chunk) }) child.stderr.on('end', () => { + assert.strictEqual(chunks.length, assertions.length) for (let i = 0; i < chunks.length; i++) { assert.match(chunks[i], assertions[i]) } }) await assert.completed + child.kill() })