From 7485cd9b4cf9a86cb76b1597df527eba15755bfc Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 27 Mar 2024 10:34:28 +0100 Subject: [PATCH 01/11] 6.10.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 733519703b0..f7d7bfd5b9a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "undici", - "version": "6.10.1", + "version": "6.10.2", "description": "An HTTP/1.1 client, written from scratch for Node.js", "homepage": "https://undici.nodejs.org", "bugs": { From 960dff9847beb57023aada45c9e040a2d387ea25 Mon Sep 17 00:00:00 2001 From: Fatuma Abdullahi <67555014+FatumaA@users.noreply.github.com> Date: Sun, 31 Mar 2024 13:30:37 +0300 Subject: [PATCH 02/11] enhancement: link to the contributing guide from the README (#3003) --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 72c32de1346..476c0280c8f 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,12 @@ An HTTP/1.1 client, written from scratch for Node.js. > Undici means eleven in Italian. 1.1 -> 11 -> Eleven -> Undici. It is also a Stranger Things reference. +## How to get involved + Have a question about using Undici? Open a [Q&A Discussion](https://github.com/nodejs/undici/discussions/new) or join our official OpenJS [Slack](https://openjs-foundation.slack.com/archives/C01QF9Q31QD) channel. +Looking to contribute? Start by reading the [contributing guide](./CONTRIBUTING.md) + ## Install ``` From 98aa9f29ecf18507183ec823f63cb32a5b41b5cc Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Thu, 28 Mar 2024 13:19:53 +0300 Subject: [PATCH 03/11] fix: node:util instead of util (#3007) * fix: node:util instead of util * Update test/fetch/request-inspect-custom.js Co-authored-by: Aras Abbasi * Update test/fetch/response-inspect-custom.js Co-authored-by: Aras Abbasi --------- Co-authored-by: Mert Can Altin Co-authored-by: Aras Abbasi --- lib/web/fetch/headers.js | 2 +- test/fetch/headers-inspect-custom.js | 2 +- test/fetch/request-inspect-custom.js | 4 ++-- test/fetch/response-inspect-custom.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web/fetch/headers.js b/lib/web/fetch/headers.js index b849b0e356c..2235f125c65 100644 --- a/lib/web/fetch/headers.js +++ b/lib/web/fetch/headers.js @@ -12,7 +12,7 @@ const { } = require('./util') const { webidl } = require('./webidl') const assert = require('node:assert') -const util = require('util') +const util = require('node:util') const kHeadersMap = Symbol('headers map') const kHeadersSortedMap = Symbol('headers map sorted') diff --git a/test/fetch/headers-inspect-custom.js b/test/fetch/headers-inspect-custom.js index 1aa3326e98c..8145e9d7536 100644 --- a/test/fetch/headers-inspect-custom.js +++ b/test/fetch/headers-inspect-custom.js @@ -3,7 +3,7 @@ const { Headers } = require('../../lib/web/fetch/headers') const { test } = require('node:test') const assert = require('node:assert') -const util = require('util') +const util = require('node:util') test('Headers class custom inspection', () => { const headers = new Headers() diff --git a/test/fetch/request-inspect-custom.js b/test/fetch/request-inspect-custom.js index 7cb7fc73152..e2e60bdab7b 100644 --- a/test/fetch/request-inspect-custom.js +++ b/test/fetch/request-inspect-custom.js @@ -1,8 +1,8 @@ 'use strict' const { describe, it } = require('node:test') -const assert = require('assert') -const util = require('util') +const assert = require('node:assert') +const util = require('node:util') const { Request } = require('../../') describe('Request custom inspection', () => { diff --git a/test/fetch/response-inspect-custom.js b/test/fetch/response-inspect-custom.js index bf72a053a6e..ca8a5a0fc1a 100644 --- a/test/fetch/response-inspect-custom.js +++ b/test/fetch/response-inspect-custom.js @@ -1,8 +1,8 @@ 'use strict' const { describe, it } = require('node:test') -const assert = require('assert') -const util = require('util') +const assert = require('node:assert') +const util = require('node:util') const { Response } = require('../../') describe('Response custom inspection', () => { From 30cced0b70604bca9c817a7a0977287449242fac Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Thu, 28 Mar 2024 19:05:53 +0100 Subject: [PATCH 04/11] fix(workflows): missing top-level content.read permissions (#3013) --- .github/workflows/nightly.yml | 3 +++ .github/workflows/test.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index deabbc6dc78..56b194771b1 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -5,6 +5,9 @@ on: schedule: - cron: "0 10 * * *" +permissions: + contents: read + jobs: test: if: github.repository == 'nodejs/undici' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a23d5edec71..9dd62828a39 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,9 @@ on: required: true type: string +permissions: + contents: read + jobs: test: name: Test with Node.js ${{ inputs.node-version }} on ${{ inputs.runs-on }} From 8c1804cfd60acc8fb1bce78cf51e04ebd74eed6b Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Sun, 31 Mar 2024 06:26:35 -0400 Subject: [PATCH 05/11] =?UTF-8?q?chore:=20add=20automated=20CI=20testing?= =?UTF-8?q?=20with=20=E2=80=94no-intl=20node=20(#3015)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/nodejs.yml | 72 ++++++++++++++++++++++ lib/mock/pending-interceptors-formatter.js | 5 +- package.json | 8 ++- test/connect-timeout.js | 2 +- test/mock-interceptor-unused-assertions.js | 36 ++++++----- test/node-test/debug.js | 2 +- 6 files changed, 104 insertions(+), 21 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 6373d4ac7e7..ba8b8fdeb6f 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -69,6 +69,77 @@ jobs: runs-on: ${{ matrix.runs-on }} secrets: inherit + test-without-intl: + name: Test with Node.js ${{ matrix.version }} compiled --without-intl + strategy: + fail-fast: false + max-parallel: 0 + matrix: + version: [20, 21] + runs-on: ubuntu-latest + timeout-minutes: 120 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + with: + persist-credentials: false + + # Setup node, install deps, and build undici prior to building icu-less node and testing + - name: Setup Node.js@${{ inputs.version }} + uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2 + with: + node-version: ${{ inputs.version }} + + - name: Install dependencies + run: npm install + + - name: Build undici + run: npm run build:node + + - name: Determine latest release + id: release + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + result-encoding: string + script: | + const req = await fetch('https://nodejs.org/download/release/index.json') + const releases = await req.json() + + const latest = releases.find((r) => r.version.startsWith('v${{ matrix.version }}')) + return latest.version + + - name: Download and extract source + run: curl https://nodejs.org/download/release/${{ steps.release.outputs.result }}/node-${{ steps.release.outputs.result }}.tar.xz | tar xfJ - + + - name: Install ninja + run: sudo apt-get install ninja-build + + - name: ccache + uses: hendrikmuhs/ccache-action@faf867a11c028c0b483fb2ae72b6fc8f7d842714 #v1.2.12 + with: + key: node${{ matrix.version }} + + - name: Build node + working-directory: ./node-${{ steps.release.outputs.result }} + run: | + export CC="ccache gcc" + export CXX="ccache g++" + ./configure --without-intl --ninja --prefix=./final + make + make install + echo "$(pwd)/final/bin" >> $GITHUB_PATH + + - name: Print version information + run: | + echo OS: $(node -p "os.version()") + echo Node.js: $(node --version) + echo npm: $(npm --version) + echo git: $(git --version) + echo icu config: $(node -e "console.log(process.config)" | grep icu) + + - name: Run tests + run: npm run test:javascript:withoutintl + test-types: name: Test TypeScript types timeout-minutes: 15 @@ -97,6 +168,7 @@ jobs: - dependency-review - test - test-types + - test-without-intl - lint runs-on: ubuntu-latest permissions: diff --git a/lib/mock/pending-interceptors-formatter.js b/lib/mock/pending-interceptors-formatter.js index ba6e4ebce1b..ccca951195a 100644 --- a/lib/mock/pending-interceptors-formatter.js +++ b/lib/mock/pending-interceptors-formatter.js @@ -3,6 +3,9 @@ const { Transform } = require('node:stream') const { Console } = require('node:console') +const PERSISTENT = process.versions.icu ? '✅' : 'Y ' +const NOT_PERSISTENT = process.versions.icu ? '❌' : 'N ' + /** * Gets the output of `console.table(…)` as a string. */ @@ -29,7 +32,7 @@ module.exports = class PendingInterceptorsFormatter { Origin: origin, Path: path, 'Status code': statusCode, - Persistent: persist ? '✅' : '❌', + Persistent: persist ? PERSISTENT : NOT_PERSISTENT, Invocations: timesInvoked, Remaining: persist ? Infinity : times - timesInvoked })) diff --git a/package.json b/package.json index a3dc801c4dc..10ecfa3befe 100644 --- a/package.json +++ b/package.json @@ -69,10 +69,13 @@ "lint:fix": "standard --fix | snazzy", "test": "npm run test:javascript && cross-env NODE_V8_COVERAGE= npm run test:typescript", "test:javascript": "node scripts/generate-pem && npm run test:unit && npm run test:node-fetch && npm run test:fetch && npm run test:cookies && npm run test:eventsource && npm run test:wpt && npm run test:websocket && npm run test:node-test && npm run test:jest", + "test:javascript:withoutintl": "node scripts/generate-pem && npm run test:unit && npm run test:node-fetch && npm run test:fetch:nobuild && npm run test:cookies && npm run test:eventsource:nobuild && npm run test:wpt:withoutintl && npm run test:node-test", "test:cookies": "borp -p \"test/cookie/*.js\"", "test:node-fetch": "borp -p \"test/node-fetch/**/*.js\"", - "test:eventsource": "npm run build:node && borp --expose-gc -p \"test/eventsource/*.js\"", - "test:fetch": "npm run build:node && borp --expose-gc -p \"test/fetch/*.js\" && borp -p \"test/webidl/*.js\" && borp -p \"test/busboy/*.js\"", + "test:eventsource": "npm run build:node && npm run test:eventsource:nobuild", + "test:eventsource:nobuild": "borp --expose-gc -p \"test/eventsource/*.js\"", + "test:fetch": "npm run build:node && npm run test:fetch:nobuild", + "test:fetch:nobuild": "borp --expose-gc -p \"test/fetch/*.js\" && borp -p \"test/webidl/*.js\" && borp -p \"test/busboy/*.js\"", "test:jest": "cross-env NODE_V8_COVERAGE= jest", "test:unit": "borp --expose-gc -p \"test/*.js\"", "test:node-test": "borp -p \"test/node-test/**/*.js\"", @@ -81,6 +84,7 @@ "test:typescript": "tsd && tsc --skipLibCheck test/imports/undici-import.ts", "test:websocket": "borp -p \"test/websocket/*.js\"", "test:wpt": "node test/wpt/start-fetch.mjs && node test/wpt/start-FileAPI.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-websockets.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs", + "test:wpt:withoutintl": "node test/wpt/start-fetch.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs", "coverage": "npm run coverage:clean && cross-env NODE_V8_COVERAGE=./coverage/tmp npm run test:javascript && npm run coverage:report", "coverage:ci": "npm run coverage:clean && cross-env NODE_V8_COVERAGE=./coverage/tmp npm run test:javascript && npm run coverage:report:ci", "coverage:clean": "node ./scripts/clean-coverage.js", diff --git a/test/connect-timeout.js b/test/connect-timeout.js index 98f7c466bbf..0a28e1f250d 100644 --- a/test/connect-timeout.js +++ b/test/connect-timeout.js @@ -9,7 +9,7 @@ const assert = require('node:assert') // Using describe instead of test to avoid the timeout describe('prioritize socket errors over timeouts', async () => { const t = tspl({ ...assert, after: () => {} }, { plan: 1 }) - const client = new Pool('http://foobar.bar:1234', { connectTimeout: 1 }) + const client = new Pool('http://foorbar.invalid:1234', { connectTimeout: 1 }) client.request({ method: 'GET', path: '/foobar' }) .then(() => t.fail()) diff --git a/test/mock-interceptor-unused-assertions.js b/test/mock-interceptor-unused-assertions.js index e6f5360a3f9..5be3942d3db 100644 --- a/test/mock-interceptor-unused-assertions.js +++ b/test/mock-interceptor-unused-assertions.js @@ -10,6 +10,10 @@ const util = require('../lib/core/util') // https://github.com/nodejs/node/pull/50135 const tableRowsAlignedToLeft = util.nodeMajor >= 21 || (util.nodeMajor === 20 && util.nodeMinor >= 11) +// `console.table` treats emoji as two character widths for cell width determination +const Y = process.versions.icu ? '✅' : 'Y ' +const N = process.versions.icu ? '❌' : 'N ' + // Avoid colors in the output for inline snapshots. const pendingInterceptorsFormatter = new PendingInterceptorsFormatter({ disableColors: true }) @@ -55,7 +59,7 @@ test('1 pending interceptor', t => { ┌─────────┬────────┬───────────────────────┬──────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼───────────────────────┼──────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ └─────────┴────────┴───────────────────────┴──────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim() : ` @@ -64,7 +68,7 @@ test('1 pending interceptor', t => { ┌─────────┬────────┬───────────────────────┬──────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼───────────────────────┼──────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ └─────────┴────────┴───────────────────────┴──────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim()) } @@ -88,8 +92,8 @@ test('2 pending interceptors', t => { ┌─────────┬────────┬──────────────────────────┬──────────────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼──────────────────────────┼──────────────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ -│ 1 │ 'GET' │ 'https://localhost:9999' │ '/some/path' │ 204 │ '❌' │ 0 │ 1 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ +│ 1 │ 'GET' │ 'https://localhost:9999' │ '/some/path' │ 204 │ '${N}' │ 0 │ 1 │ └─────────┴────────┴──────────────────────────┴──────────────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim() : ` @@ -98,8 +102,8 @@ test('2 pending interceptors', t => { ┌─────────┬────────┬──────────────────────────┬──────────────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼──────────────────────────┼──────────────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ -│ 1 │ 'GET' │ 'https://localhost:9999' │ '/some/path' │ 204 │ '❌' │ 0 │ 1 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ +│ 1 │ 'GET' │ 'https://localhost:9999' │ '/some/path' │ 204 │ '${N}' │ 0 │ 1 │ └─────────┴────────┴──────────────────────────┴──────────────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim()) } @@ -164,10 +168,10 @@ test('Variations of persist(), times(), and pending status', async t => { ┌─────────┬────────┬──────────────────────────┬──────────────────────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼──────────────────────────┼──────────────────────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ -│ 1 │ 'GET' │ 'https://localhost:9999' │ '/persistent/unused' │ 200 │ '✅' │ 0 │ Infinity │ -│ 2 │ 'GET' │ 'https://localhost:9999' │ '/times/partial' │ 200 │ '❌' │ 1 │ 4 │ -│ 3 │ 'GET' │ 'https://localhost:9999' │ '/times/unused' │ 200 │ '❌' │ 0 │ 2 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ +│ 1 │ 'GET' │ 'https://localhost:9999' │ '/persistent/unused' │ 200 │ '${Y}' │ 0 │ Infinity │ +│ 2 │ 'GET' │ 'https://localhost:9999' │ '/times/partial' │ 200 │ '${N}' │ 1 │ 4 │ +│ 3 │ 'GET' │ 'https://localhost:9999' │ '/times/unused' │ 200 │ '${N}' │ 0 │ 2 │ └─────────┴────────┴──────────────────────────┴──────────────────────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim() : ` @@ -176,10 +180,10 @@ test('Variations of persist(), times(), and pending status', async t => { ┌─────────┬────────┬──────────────────────────┬──────────────────────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼──────────────────────────┼──────────────────────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '❌' │ 0 │ 1 │ -│ 1 │ 'GET' │ 'https://localhost:9999' │ '/persistent/unused' │ 200 │ '✅' │ 0 │ Infinity │ -│ 2 │ 'GET' │ 'https://localhost:9999' │ '/times/partial' │ 200 │ '❌' │ 1 │ 4 │ -│ 3 │ 'GET' │ 'https://localhost:9999' │ '/times/unused' │ 200 │ '❌' │ 0 │ 2 │ +│ 0 │ 'GET' │ 'https://example.com' │ '/' │ 200 │ '${N}' │ 0 │ 1 │ +│ 1 │ 'GET' │ 'https://localhost:9999' │ '/persistent/unused' │ 200 │ '${Y}' │ 0 │ Infinity │ +│ 2 │ 'GET' │ 'https://localhost:9999' │ '/times/partial' │ 200 │ '${N}' │ 1 │ 4 │ +│ 3 │ 'GET' │ 'https://localhost:9999' │ '/times/unused' │ 200 │ '${N}' │ 0 │ 2 │ └─────────┴────────┴──────────────────────────┴──────────────────────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim()) } @@ -229,7 +233,7 @@ test('defaults to rendering output with terminal color when process.env.CI is un ┌─────────┬────────┬───────────────────────┬──────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼───────────────────────┼──────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ \u001b[32m'GET'\u001b[39m │ \u001b[32m'https://example.com'\u001b[39m │ \u001b[32m'/'\u001b[39m │ \u001b[33m200\u001b[39m │ \u001b[32m'❌'\u001b[39m │ \u001b[33m0\u001b[39m │ \u001b[33m1\u001b[39m │ +│ 0 │ \u001b[32m'GET'\u001b[39m │ \u001b[32m'https://example.com'\u001b[39m │ \u001b[32m'/'\u001b[39m │ \u001b[33m200\u001b[39m │ \u001b[32m'${N}'\u001b[39m │ \u001b[33m0\u001b[39m │ \u001b[33m1\u001b[39m │ └─────────┴────────┴───────────────────────┴──────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim() : ` @@ -238,7 +242,7 @@ test('defaults to rendering output with terminal color when process.env.CI is un ┌─────────┬────────┬───────────────────────┬──────┬─────────────┬────────────┬─────────────┬───────────┐ │ (index) │ Method │ Origin │ Path │ Status code │ Persistent │ Invocations │ Remaining │ ├─────────┼────────┼───────────────────────┼──────┼─────────────┼────────────┼─────────────┼───────────┤ -│ 0 │ \u001b[32m'GET'\u001b[39m │ \u001b[32m'https://example.com'\u001b[39m │ \u001b[32m'/'\u001b[39m │ \u001b[33m200\u001b[39m │ \u001b[32m'❌'\u001b[39m │ \u001b[33m0\u001b[39m │ \u001b[33m1\u001b[39m │ +│ 0 │ \u001b[32m'GET'\u001b[39m │ \u001b[32m'https://example.com'\u001b[39m │ \u001b[32m'/'\u001b[39m │ \u001b[33m200\u001b[39m │ \u001b[32m'${N}'\u001b[39m │ \u001b[33m0\u001b[39m │ \u001b[33m1\u001b[39m │ └─────────┴────────┴───────────────────────┴──────┴─────────────┴────────────┴─────────────┴───────────┘ `.trim()) diff --git a/test/node-test/debug.js b/test/node-test/debug.js index 3e6ca0bc0ef..d7c462f57ae 100644 --- a/test/node-test/debug.js +++ b/test/node-test/debug.js @@ -8,7 +8,7 @@ const { tspl } = require('@matteo.collina/tspl') // eslint-disable-next-line no-control-regex const removeEscapeColorsRE = /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g -test('debug#websocket', async t => { +test('debug#websocket', { skip: !process.versions.icu }, async t => { const assert = tspl(t, { plan: 8 }) const child = spawn( process.execPath, From 9eae7ce5a8baa0257e8b41051cf76e1e6ec985d9 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Sat, 30 Mar 2024 12:37:41 -0400 Subject: [PATCH 06/11] chore(workflows/nightly.yml): create issue only on all fail (#3020) --- .github/workflows/nightly.yml | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 56b194771b1..28f66097a3e 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -9,25 +9,36 @@ permissions: contents: read jobs: - test: + test-linux: if: github.repository == 'nodejs/undici' - strategy: - fail-fast: false - max-parallel: 0 - matrix: - runs-on: - - ubuntu-latest - - windows-latest - - macos-latest uses: ./.github/workflows/test.yml with: node-version: 22-nightly - runs-on: ${{ matrix.runs-on }} + runs-on: ubuntu-latest + secrets: inherit + + test-windows: + if: github.repository == 'nodejs/undici' + uses: ./.github/workflows/test.yml + with: + node-version: 22-nightly + runs-on: windows-latest + secrets: inherit + + test-macos: + if: github.repository == 'nodejs/undici' + uses: ./.github/workflows/test.yml + with: + node-version: 22-nightly + runs-on: macos-latest secrets: inherit report-failure: - if: failure() - needs: test + if: ${{ always() && (needs.test-linux.result == 'failure' && needs.test-windows.result == 'failure' && needs.test-macos.result == 'failure') }} + needs: + - test-linux + - test-windows + - test-macos runs-on: ubuntu-latest permissions: issues: write From e4cf4409f0fe69369a41a50878318872f08f31bb Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Sun, 31 Mar 2024 12:16:41 +0200 Subject: [PATCH 07/11] chore(automerge): remove unnecessary actions:write permission (#3021) --- .github/workflows/nodejs.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index ba8b8fdeb6f..882134e8b93 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -174,7 +174,6 @@ jobs: permissions: contents: write pull-requests: write - actions: write steps: - name: Merge Dependabot PR uses: fastify/github-action-merge-dependabot@9e7bfb249c69139d7bdcd8d984f9665edd49020b # v3.10.1 From b4ac7a418ed84bafb9143cb8cf780aaceb144489 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 2 Apr 2024 09:48:10 +0200 Subject: [PATCH 08/11] fix(#2364): concurrent aborts (#3005) --- lib/dispatcher/client-h2.js | 653 -------------------------------- lib/dispatcher/client-h2.js.rej | 20 + test/http2.js | 85 +++++ 3 files changed, 105 insertions(+), 653 deletions(-) delete mode 100644 lib/dispatcher/client-h2.js create mode 100644 lib/dispatcher/client-h2.js.rej diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js deleted file mode 100644 index 0a53b75e501..00000000000 --- a/lib/dispatcher/client-h2.js +++ /dev/null @@ -1,653 +0,0 @@ -'use strict' - -const assert = require('node:assert') -const { pipeline } = require('node:stream') -const util = require('../core/util.js') -const { - RequestContentLengthMismatchError, - RequestAbortedError, - SocketError, - InformationalError -} = require('../core/errors.js') -const { - kUrl, - kReset, - kClient, - kRunning, - kPending, - kQueue, - kPendingIdx, - kRunningIdx, - kError, - kSocket, - kStrictContentLength, - kOnError, - // HTTP2 - kMaxConcurrentStreams, - kHTTP2Session, - kResume -} = require('../core/symbols.js') - -const kOpenStreams = Symbol('open streams') - -// Experimental -let h2ExperimentalWarned = false - -/** @type {import('http2')} */ -let http2 -try { - http2 = require('node:http2') -} catch { - // @ts-ignore - http2 = { constants: {} } -} - -const { - constants: { - HTTP2_HEADER_AUTHORITY, - HTTP2_HEADER_METHOD, - HTTP2_HEADER_PATH, - HTTP2_HEADER_SCHEME, - HTTP2_HEADER_CONTENT_LENGTH, - HTTP2_HEADER_EXPECT, - HTTP2_HEADER_STATUS - } -} = http2 - -function parseH2Headers (headers) { - // set-cookie is always an array. Duplicates are added to the array. - // For duplicate cookie headers, the values are joined together with '; '. - headers = Object.entries(headers).flat(2) - - const result = [] - - for (const header of headers) { - result.push(Buffer.from(header)) - } - - return result -} - -async function connectH2 (client, socket) { - client[kSocket] = socket - - if (!h2ExperimentalWarned) { - h2ExperimentalWarned = true - process.emitWarning('H2 support is experimental, expect them to change at any time.', { - code: 'UNDICI-H2' - }) - } - - const session = http2.connect(client[kUrl], { - createConnection: () => socket, - peerMaxConcurrentStreams: client[kMaxConcurrentStreams] - }) - - session[kOpenStreams] = 0 - session[kClient] = client - session[kSocket] = socket - session.on('error', onHttp2SessionError) - session.on('frameError', onHttp2FrameError) - session.on('end', onHttp2SessionEnd) - session.on('goaway', onHTTP2GoAway) - session.on('close', function () { - const { [kClient]: client } = this - - const err = this[kError] || new SocketError('closed', util.getSocketInfo(this)) - - client[kSocket] = null - - assert(client[kPending] === 0) - - // Fail entire queue. - const requests = client[kQueue].splice(client[kRunningIdx]) - for (let i = 0; i < requests.length; i++) { - const request = requests[i] - errorRequest(client, request, err) - } - - client[kPendingIdx] = client[kRunningIdx] - - assert(client[kRunning] === 0) - - client.emit('disconnect', client[kUrl], [client], err) - - client[kResume]() - }) - session.unref() - - client[kHTTP2Session] = session - socket[kHTTP2Session] = session - - socket.on('error', function (err) { - assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') - - this[kError] = err - - this[kClient][kOnError](err) - }) - socket.on('end', function () { - util.destroy(this, new SocketError('other side closed', util.getSocketInfo(this))) - }) - - let closed = false - socket.on('close', () => { - closed = true - }) - - return { - version: 'h2', - defaultPipelining: Infinity, - write (...args) { - // TODO (fix): return - writeH2(client, ...args) - }, - resume () { - - }, - destroy (err, callback) { - session.destroy(err) - if (closed) { - queueMicrotask(callback) - } else { - socket.destroy(err).on('close', callback) - } - }, - get destroyed () { - return socket.destroyed - }, - busy () { - return false - } - } -} - -function onHttp2SessionError (err) { - assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') - - this[kSocket][kError] = err - - this[kClient][kOnError](err) -} - -function onHttp2FrameError (type, code, id) { - const err = new InformationalError(`HTTP/2: "frameError" received - type ${type}, code ${code}`) - - if (id === 0) { - this[kSocket][kError] = err - this[kClient][kOnError](err) - } -} - -function onHttp2SessionEnd () { - this.destroy(new SocketError('other side closed')) - util.destroy(this[kSocket], new SocketError('other side closed')) -} - -function onHTTP2GoAway (code) { - const client = this[kClient] - const err = new InformationalError(`HTTP/2: "GOAWAY" frame received with code ${code}`) - client[kSocket] = null - client[kHTTP2Session] = null - - if (client.destroyed) { - assert(this[kPending] === 0) - - // Fail entire queue. - const requests = client[kQueue].splice(client[kRunningIdx]) - for (let i = 0; i < requests.length; i++) { - const request = requests[i] - errorRequest(this, request, err) - } - } else if (client[kRunning] > 0) { - // Fail head of pipeline. - const request = client[kQueue][client[kRunningIdx]] - client[kQueue][client[kRunningIdx]++] = null - - errorRequest(client, request, err) - } - - client[kPendingIdx] = client[kRunningIdx] - - assert(client[kRunning] === 0) - - client.emit('disconnect', - client[kUrl], - [client], - err - ) - - client[kResume]() -} - -function errorRequest (client, request, err) { - try { - request.onError(err) - assert(request.aborted) - } catch (err) { - client.emit('error', err) - } -} - -// https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 -function shouldSendContentLength (method) { - return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT' -} - -function writeH2 (client, request) { - const session = client[kHTTP2Session] - const { body, method, path, host, upgrade, expectContinue, signal, headers: reqHeaders } = request - - if (upgrade) { - errorRequest(client, request, new Error('Upgrade not supported for H2')) - return false - } - - if (request.aborted) { - return false - } - - const headers = {} - for (let n = 0; n < reqHeaders.length; n += 2) { - const key = reqHeaders[n + 0] - const val = reqHeaders[n + 1] - - if (Array.isArray(val)) { - for (let i = 0; i < val.length; i++) { - if (headers[key]) { - headers[key] += `,${val[i]}` - } else { - headers[key] = val[i] - } - } - } else { - headers[key] = val - } - } - - /** @type {import('node:http2').ClientHttp2Stream} */ - let stream - - const { hostname, port } = client[kUrl] - - headers[HTTP2_HEADER_AUTHORITY] = host || `${hostname}${port ? `:${port}` : ''}` - headers[HTTP2_HEADER_METHOD] = method - - try { - // We are already connected, streams are pending. - // We can call on connect, and wait for abort - request.onConnect((err) => { - if (request.aborted || request.completed) { - return - } - - err = err || new RequestAbortedError() - - if (stream != null) { - util.destroy(stream, err) - - session[kOpenStreams] -= 1 - if (session[kOpenStreams] === 0) { - session.unref() - } - } - - errorRequest(client, request, err) - }) - } catch (err) { - errorRequest(client, request, err) - } - - if (method === 'CONNECT') { - session.ref() - // We are already connected, streams are pending, first request - // will create a new stream. We trigger a request to create the stream and wait until - // `ready` event is triggered - // We disabled endStream to allow the user to write to the stream - stream = session.request(headers, { endStream: false, signal }) - - if (stream.id && !stream.pending) { - request.onUpgrade(null, null, stream) - ++session[kOpenStreams] - } else { - stream.once('ready', () => { - request.onUpgrade(null, null, stream) - ++session[kOpenStreams] - }) - } - - stream.once('close', () => { - session[kOpenStreams] -= 1 - // TODO(HTTP/2): unref only if current streams count is 0 - if (session[kOpenStreams] === 0) session.unref() - }) - - return true - } - - // https://tools.ietf.org/html/rfc7540#section-8.3 - // :path and :scheme headers must be omitted when sending CONNECT - - headers[HTTP2_HEADER_PATH] = path - headers[HTTP2_HEADER_SCHEME] = 'https' - - // https://tools.ietf.org/html/rfc7231#section-4.3.1 - // https://tools.ietf.org/html/rfc7231#section-4.3.2 - // https://tools.ietf.org/html/rfc7231#section-4.3.5 - - // Sending a payload body on a request that does not - // expect it can cause undefined behavior on some - // servers and corrupt connection state. Do not - // re-use the connection for further requests. - - const expectsPayload = ( - method === 'PUT' || - method === 'POST' || - method === 'PATCH' - ) - - if (body && typeof body.read === 'function') { - // Try to read EOF in order to get length. - body.read(0) - } - - let contentLength = util.bodyLength(body) - - if (contentLength == null) { - contentLength = request.contentLength - } - - if (contentLength === 0 || !expectsPayload) { - // https://tools.ietf.org/html/rfc7230#section-3.3.2 - // A user agent SHOULD NOT send a Content-Length header field when - // the request message does not contain a payload body and the method - // semantics do not anticipate such a body. - - contentLength = null - } - - // https://github.com/nodejs/undici/issues/2046 - // A user agent may send a Content-Length header with 0 value, this should be allowed. - if (shouldSendContentLength(method) && contentLength > 0 && request.contentLength != null && request.contentLength !== contentLength) { - if (client[kStrictContentLength]) { - errorRequest(client, request, new RequestContentLengthMismatchError()) - return false - } - - process.emitWarning(new RequestContentLengthMismatchError()) - } - - if (contentLength != null) { - assert(body, 'no body must not have content length') - headers[HTTP2_HEADER_CONTENT_LENGTH] = `${contentLength}` - } - - session.ref() - - const shouldEndStream = method === 'GET' || method === 'HEAD' || body === null - if (expectContinue) { - headers[HTTP2_HEADER_EXPECT] = '100-continue' - stream = session.request(headers, { endStream: shouldEndStream, signal }) - - stream.once('continue', writeBodyH2) - } else { - stream = session.request(headers, { - endStream: shouldEndStream, - signal - }) - writeBodyH2() - } - - // Increment counter as we have new several streams open - ++session[kOpenStreams] - - stream.once('response', headers => { - const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers - request.onResponseStarted() - - if (request.onHeaders(Number(statusCode), parseH2Headers(realHeaders), stream.resume.bind(stream), '') === false) { - stream.pause() - } - - stream.on('data', (chunk) => { - if (request.onData(chunk) === false) { - stream.pause() - } - }) - }) - - stream.once('end', () => { - // When state is null, it means we haven't consumed body and the stream still do not have - // a state. - // Present specially when using pipeline or stream - if (stream.state?.state == null || stream.state.state < 6) { - request.onComplete([]) - return - } - - // Stream is closed or half-closed-remote (6), decrement counter and cleanup - // It does not have sense to continue working with the stream as we do not - // have yet RST_STREAM support on client-side - session[kOpenStreams] -= 1 - if (session[kOpenStreams] === 0) { - session.unref() - } - - const err = new InformationalError('HTTP/2: stream half-closed (remote)') - errorRequest(client, request, err) - util.destroy(stream, err) - }) - - stream.once('close', () => { - session[kOpenStreams] -= 1 - // TODO(HTTP/2): unref only if current streams count is 0 - if (session[kOpenStreams] === 0) { - session.unref() - } - }) - - stream.once('error', function (err) { - if (client[kHTTP2Session] && !client[kHTTP2Session].destroyed && !this.closed && !this.destroyed) { - session[kOpenStreams] -= 1 - util.destroy(stream, err) - } - }) - - stream.once('frameError', (type, code) => { - const err = new InformationalError(`HTTP/2: "frameError" received - type ${type}, code ${code}`) - errorRequest(client, request, err) - - if (client[kHTTP2Session] && !client[kHTTP2Session].destroyed && !this.closed && !this.destroyed) { - session[kOpenStreams] -= 1 - util.destroy(stream, err) - } - }) - - // stream.on('aborted', () => { - // // TODO(HTTP/2): Support aborted - // }) - - // stream.on('timeout', () => { - // // TODO(HTTP/2): Support timeout - // }) - - // stream.on('push', headers => { - // // TODO(HTTP/2): Support push - // }) - - // stream.on('trailers', headers => { - // // TODO(HTTP/2): Support trailers - // }) - - return true - - function writeBodyH2 () { - /* istanbul ignore else: assertion */ - if (!body) { - request.onRequestSent() - } else if (util.isBuffer(body)) { - assert(contentLength === body.byteLength, 'buffer body must have content length') - stream.cork() - stream.write(body) - stream.uncork() - stream.end() - request.onBodySent(body) - request.onRequestSent() - } else if (util.isBlobLike(body)) { - if (typeof body.stream === 'function') { - writeIterable({ - client, - request, - contentLength, - h2stream: stream, - expectsPayload, - body: body.stream(), - socket: client[kSocket], - header: '' - }) - } else { - writeBlob({ - body, - client, - request, - contentLength, - expectsPayload, - h2stream: stream, - header: '', - socket: client[kSocket] - }) - } - } else if (util.isStream(body)) { - writeStream({ - body, - client, - request, - contentLength, - expectsPayload, - socket: client[kSocket], - h2stream: stream, - header: '' - }) - } else if (util.isIterable(body)) { - writeIterable({ - body, - client, - request, - contentLength, - expectsPayload, - header: '', - h2stream: stream, - socket: client[kSocket] - }) - } else { - assert(false) - } - } -} - -function writeStream ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { - assert(contentLength !== 0 || client[kRunning] === 0, 'stream body cannot be pipelined') - - // For HTTP/2, is enough to pipe the stream - const pipe = pipeline( - body, - h2stream, - (err) => { - if (err) { - util.destroy(body, err) - util.destroy(h2stream, err) - } else { - request.onRequestSent() - } - } - ) - - pipe.on('data', onPipeData) - pipe.once('end', () => { - pipe.removeListener('data', onPipeData) - util.destroy(pipe) - }) - - function onPipeData (chunk) { - request.onBodySent(chunk) - } -} - -async function writeBlob ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { - assert(contentLength === body.size, 'blob body must have content length') - - try { - if (contentLength != null && contentLength !== body.size) { - throw new RequestContentLengthMismatchError() - } - - const buffer = Buffer.from(await body.arrayBuffer()) - - h2stream.cork() - h2stream.write(buffer) - h2stream.uncork() - - request.onBodySent(buffer) - request.onRequestSent() - - if (!expectsPayload) { - socket[kReset] = true - } - - client[kResume]() - } catch (err) { - util.destroy(h2stream) - } -} - -async function writeIterable ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { - assert(contentLength !== 0 || client[kRunning] === 0, 'iterator body cannot be pipelined') - - let callback = null - function onDrain () { - if (callback) { - const cb = callback - callback = null - cb() - } - } - - const waitForDrain = () => new Promise((resolve, reject) => { - assert(callback === null) - - if (socket[kError]) { - reject(socket[kError]) - } else { - callback = resolve - } - }) - - h2stream - .on('close', onDrain) - .on('drain', onDrain) - - try { - // It's up to the user to somehow abort the async iterable. - for await (const chunk of body) { - if (socket[kError]) { - throw socket[kError] - } - - const res = h2stream.write(chunk) - request.onBodySent(chunk) - if (!res) { - await waitForDrain() - } - } - } catch (err) { - h2stream.destroy(err) - } finally { - request.onRequestSent() - h2stream.end() - h2stream - .off('close', onDrain) - .off('drain', onDrain) - } -} - -module.exports = connectH2 diff --git a/lib/dispatcher/client-h2.js.rej b/lib/dispatcher/client-h2.js.rej new file mode 100644 index 00000000000..4e49510ed4a --- /dev/null +++ b/lib/dispatcher/client-h2.js.rej @@ -0,0 +1,20 @@ +diff a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js (rejected hunks) +@@ -391,6 +391,18 @@ function writeH2 (client, request) { + const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers + request.onResponseStarted() + ++ // Due to the stream nature, it is possible we face a race condition ++ // where the stream has been assigned, but the request has been aborted ++ // the request remains in-flight and headers hasn't been received yet ++ // for those scenarios, best effort is to destroy the stream immediately ++ // as there's no value to keep it open. ++ if (request.aborted || request.completed) { ++ const err = new RequestAbortedError() ++ errorRequest(client, request, err) ++ util.destroy(stream, err) ++ return ++ } ++ + if (request.onHeaders(Number(statusCode), realHeaders, stream.resume.bind(stream), '') === false) { + stream.pause() + } diff --git a/test/http2.js b/test/http2.js index 849a0cc43ba..a4ecc1ac3e8 100644 --- a/test/http2.js +++ b/test/http2.js @@ -1296,3 +1296,88 @@ test('Should throw informational error on half-closed streams (remote)', async t t.strictEqual(err.code, 'UND_ERR_INFO') }) }) + +test('#2364 - Concurrent aborts', async t => { + const server = createSecureServer(pem) + + server.on('stream', (stream, headers, _flags, rawHeaders) => { + t.strictEqual(headers['x-my-header'], 'foo') + t.strictEqual(headers[':method'], 'GET') + setTimeout(() => { + stream.respond({ + 'content-type': 'text/plain; charset=utf-8', + 'x-custom-h2': 'hello', + ':status': 200 + }) + stream.end('hello h2!') + }, 100) + }) + + server.listen(0) + await once(server, 'listening') + + const client = new Client(`https://localhost:${server.address().port}`, { + connect: { + rejectUnauthorized: false + }, + allowH2: true + }) + + t = tspl(t, { plan: 18 }) + after(() => server.close()) + after(() => client.close()) + const controller = new AbortController() + + client.request({ + path: '/', + method: 'GET', + headers: { + 'x-my-header': 'foo' + } + }, (err, response) => { + t.ifError(err) + t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + t.strictEqual(response.headers['x-custom-h2'], 'hello') + t.strictEqual(response.statusCode, 200) + response.body.dump() + }) + + client.request({ + path: '/', + method: 'GET', + headers: { + 'x-my-header': 'foo' + }, + signal: controller.signal + }, (err, response) => { + t.strictEqual(err.name, 'AbortError') + }) + + client.request({ + path: '/', + method: 'GET', + headers: { + 'x-my-header': 'foo' + } + }, (err, response) => { + t.ifError(err) + t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + t.strictEqual(response.headers['x-custom-h2'], 'hello') + t.strictEqual(response.statusCode, 200) + }) + + client.request({ + path: '/', + method: 'GET', + headers: { + 'x-my-header': 'foo' + }, + signal: controller.signal + }, (err, response) => { + t.strictEqual(err.name, 'AbortError') + }) + + controller.abort() + + await t.completed +}) From 57ff37ca12fe43420c39facf1060390afcd08173 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 2 Apr 2024 11:16:06 +0200 Subject: [PATCH 09/11] fixup Signed-off-by: Matteo Collina --- lib/dispatcher/{client-h2.js.rej => client-h2.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/dispatcher/{client-h2.js.rej => client-h2.js} (100%) diff --git a/lib/dispatcher/client-h2.js.rej b/lib/dispatcher/client-h2.js similarity index 100% rename from lib/dispatcher/client-h2.js.rej rename to lib/dispatcher/client-h2.js From c007adcbe736e1481458169027ab84cb2f75e2c2 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 2 Apr 2024 11:18:58 +0200 Subject: [PATCH 10/11] fixup Signed-off-by: Matteo Collina --- lib/dispatcher/client-h2.js | 673 ++++++++++++++++++++++++++++++++++-- 1 file changed, 653 insertions(+), 20 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 4e49510ed4a..0a53b75e501 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -1,20 +1,653 @@ -diff a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js (rejected hunks) -@@ -391,6 +391,18 @@ function writeH2 (client, request) { - const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers - request.onResponseStarted() - -+ // Due to the stream nature, it is possible we face a race condition -+ // where the stream has been assigned, but the request has been aborted -+ // the request remains in-flight and headers hasn't been received yet -+ // for those scenarios, best effort is to destroy the stream immediately -+ // as there's no value to keep it open. -+ if (request.aborted || request.completed) { -+ const err = new RequestAbortedError() -+ errorRequest(client, request, err) -+ util.destroy(stream, err) -+ return -+ } -+ - if (request.onHeaders(Number(statusCode), realHeaders, stream.resume.bind(stream), '') === false) { - stream.pause() - } +'use strict' + +const assert = require('node:assert') +const { pipeline } = require('node:stream') +const util = require('../core/util.js') +const { + RequestContentLengthMismatchError, + RequestAbortedError, + SocketError, + InformationalError +} = require('../core/errors.js') +const { + kUrl, + kReset, + kClient, + kRunning, + kPending, + kQueue, + kPendingIdx, + kRunningIdx, + kError, + kSocket, + kStrictContentLength, + kOnError, + // HTTP2 + kMaxConcurrentStreams, + kHTTP2Session, + kResume +} = require('../core/symbols.js') + +const kOpenStreams = Symbol('open streams') + +// Experimental +let h2ExperimentalWarned = false + +/** @type {import('http2')} */ +let http2 +try { + http2 = require('node:http2') +} catch { + // @ts-ignore + http2 = { constants: {} } +} + +const { + constants: { + HTTP2_HEADER_AUTHORITY, + HTTP2_HEADER_METHOD, + HTTP2_HEADER_PATH, + HTTP2_HEADER_SCHEME, + HTTP2_HEADER_CONTENT_LENGTH, + HTTP2_HEADER_EXPECT, + HTTP2_HEADER_STATUS + } +} = http2 + +function parseH2Headers (headers) { + // set-cookie is always an array. Duplicates are added to the array. + // For duplicate cookie headers, the values are joined together with '; '. + headers = Object.entries(headers).flat(2) + + const result = [] + + for (const header of headers) { + result.push(Buffer.from(header)) + } + + return result +} + +async function connectH2 (client, socket) { + client[kSocket] = socket + + if (!h2ExperimentalWarned) { + h2ExperimentalWarned = true + process.emitWarning('H2 support is experimental, expect them to change at any time.', { + code: 'UNDICI-H2' + }) + } + + const session = http2.connect(client[kUrl], { + createConnection: () => socket, + peerMaxConcurrentStreams: client[kMaxConcurrentStreams] + }) + + session[kOpenStreams] = 0 + session[kClient] = client + session[kSocket] = socket + session.on('error', onHttp2SessionError) + session.on('frameError', onHttp2FrameError) + session.on('end', onHttp2SessionEnd) + session.on('goaway', onHTTP2GoAway) + session.on('close', function () { + const { [kClient]: client } = this + + const err = this[kError] || new SocketError('closed', util.getSocketInfo(this)) + + client[kSocket] = null + + assert(client[kPending] === 0) + + // Fail entire queue. + const requests = client[kQueue].splice(client[kRunningIdx]) + for (let i = 0; i < requests.length; i++) { + const request = requests[i] + errorRequest(client, request, err) + } + + client[kPendingIdx] = client[kRunningIdx] + + assert(client[kRunning] === 0) + + client.emit('disconnect', client[kUrl], [client], err) + + client[kResume]() + }) + session.unref() + + client[kHTTP2Session] = session + socket[kHTTP2Session] = session + + socket.on('error', function (err) { + assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') + + this[kError] = err + + this[kClient][kOnError](err) + }) + socket.on('end', function () { + util.destroy(this, new SocketError('other side closed', util.getSocketInfo(this))) + }) + + let closed = false + socket.on('close', () => { + closed = true + }) + + return { + version: 'h2', + defaultPipelining: Infinity, + write (...args) { + // TODO (fix): return + writeH2(client, ...args) + }, + resume () { + + }, + destroy (err, callback) { + session.destroy(err) + if (closed) { + queueMicrotask(callback) + } else { + socket.destroy(err).on('close', callback) + } + }, + get destroyed () { + return socket.destroyed + }, + busy () { + return false + } + } +} + +function onHttp2SessionError (err) { + assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') + + this[kSocket][kError] = err + + this[kClient][kOnError](err) +} + +function onHttp2FrameError (type, code, id) { + const err = new InformationalError(`HTTP/2: "frameError" received - type ${type}, code ${code}`) + + if (id === 0) { + this[kSocket][kError] = err + this[kClient][kOnError](err) + } +} + +function onHttp2SessionEnd () { + this.destroy(new SocketError('other side closed')) + util.destroy(this[kSocket], new SocketError('other side closed')) +} + +function onHTTP2GoAway (code) { + const client = this[kClient] + const err = new InformationalError(`HTTP/2: "GOAWAY" frame received with code ${code}`) + client[kSocket] = null + client[kHTTP2Session] = null + + if (client.destroyed) { + assert(this[kPending] === 0) + + // Fail entire queue. + const requests = client[kQueue].splice(client[kRunningIdx]) + for (let i = 0; i < requests.length; i++) { + const request = requests[i] + errorRequest(this, request, err) + } + } else if (client[kRunning] > 0) { + // Fail head of pipeline. + const request = client[kQueue][client[kRunningIdx]] + client[kQueue][client[kRunningIdx]++] = null + + errorRequest(client, request, err) + } + + client[kPendingIdx] = client[kRunningIdx] + + assert(client[kRunning] === 0) + + client.emit('disconnect', + client[kUrl], + [client], + err + ) + + client[kResume]() +} + +function errorRequest (client, request, err) { + try { + request.onError(err) + assert(request.aborted) + } catch (err) { + client.emit('error', err) + } +} + +// https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 +function shouldSendContentLength (method) { + return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT' +} + +function writeH2 (client, request) { + const session = client[kHTTP2Session] + const { body, method, path, host, upgrade, expectContinue, signal, headers: reqHeaders } = request + + if (upgrade) { + errorRequest(client, request, new Error('Upgrade not supported for H2')) + return false + } + + if (request.aborted) { + return false + } + + const headers = {} + for (let n = 0; n < reqHeaders.length; n += 2) { + const key = reqHeaders[n + 0] + const val = reqHeaders[n + 1] + + if (Array.isArray(val)) { + for (let i = 0; i < val.length; i++) { + if (headers[key]) { + headers[key] += `,${val[i]}` + } else { + headers[key] = val[i] + } + } + } else { + headers[key] = val + } + } + + /** @type {import('node:http2').ClientHttp2Stream} */ + let stream + + const { hostname, port } = client[kUrl] + + headers[HTTP2_HEADER_AUTHORITY] = host || `${hostname}${port ? `:${port}` : ''}` + headers[HTTP2_HEADER_METHOD] = method + + try { + // We are already connected, streams are pending. + // We can call on connect, and wait for abort + request.onConnect((err) => { + if (request.aborted || request.completed) { + return + } + + err = err || new RequestAbortedError() + + if (stream != null) { + util.destroy(stream, err) + + session[kOpenStreams] -= 1 + if (session[kOpenStreams] === 0) { + session.unref() + } + } + + errorRequest(client, request, err) + }) + } catch (err) { + errorRequest(client, request, err) + } + + if (method === 'CONNECT') { + session.ref() + // We are already connected, streams are pending, first request + // will create a new stream. We trigger a request to create the stream and wait until + // `ready` event is triggered + // We disabled endStream to allow the user to write to the stream + stream = session.request(headers, { endStream: false, signal }) + + if (stream.id && !stream.pending) { + request.onUpgrade(null, null, stream) + ++session[kOpenStreams] + } else { + stream.once('ready', () => { + request.onUpgrade(null, null, stream) + ++session[kOpenStreams] + }) + } + + stream.once('close', () => { + session[kOpenStreams] -= 1 + // TODO(HTTP/2): unref only if current streams count is 0 + if (session[kOpenStreams] === 0) session.unref() + }) + + return true + } + + // https://tools.ietf.org/html/rfc7540#section-8.3 + // :path and :scheme headers must be omitted when sending CONNECT + + headers[HTTP2_HEADER_PATH] = path + headers[HTTP2_HEADER_SCHEME] = 'https' + + // https://tools.ietf.org/html/rfc7231#section-4.3.1 + // https://tools.ietf.org/html/rfc7231#section-4.3.2 + // https://tools.ietf.org/html/rfc7231#section-4.3.5 + + // Sending a payload body on a request that does not + // expect it can cause undefined behavior on some + // servers and corrupt connection state. Do not + // re-use the connection for further requests. + + const expectsPayload = ( + method === 'PUT' || + method === 'POST' || + method === 'PATCH' + ) + + if (body && typeof body.read === 'function') { + // Try to read EOF in order to get length. + body.read(0) + } + + let contentLength = util.bodyLength(body) + + if (contentLength == null) { + contentLength = request.contentLength + } + + if (contentLength === 0 || !expectsPayload) { + // https://tools.ietf.org/html/rfc7230#section-3.3.2 + // A user agent SHOULD NOT send a Content-Length header field when + // the request message does not contain a payload body and the method + // semantics do not anticipate such a body. + + contentLength = null + } + + // https://github.com/nodejs/undici/issues/2046 + // A user agent may send a Content-Length header with 0 value, this should be allowed. + if (shouldSendContentLength(method) && contentLength > 0 && request.contentLength != null && request.contentLength !== contentLength) { + if (client[kStrictContentLength]) { + errorRequest(client, request, new RequestContentLengthMismatchError()) + return false + } + + process.emitWarning(new RequestContentLengthMismatchError()) + } + + if (contentLength != null) { + assert(body, 'no body must not have content length') + headers[HTTP2_HEADER_CONTENT_LENGTH] = `${contentLength}` + } + + session.ref() + + const shouldEndStream = method === 'GET' || method === 'HEAD' || body === null + if (expectContinue) { + headers[HTTP2_HEADER_EXPECT] = '100-continue' + stream = session.request(headers, { endStream: shouldEndStream, signal }) + + stream.once('continue', writeBodyH2) + } else { + stream = session.request(headers, { + endStream: shouldEndStream, + signal + }) + writeBodyH2() + } + + // Increment counter as we have new several streams open + ++session[kOpenStreams] + + stream.once('response', headers => { + const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers + request.onResponseStarted() + + if (request.onHeaders(Number(statusCode), parseH2Headers(realHeaders), stream.resume.bind(stream), '') === false) { + stream.pause() + } + + stream.on('data', (chunk) => { + if (request.onData(chunk) === false) { + stream.pause() + } + }) + }) + + stream.once('end', () => { + // When state is null, it means we haven't consumed body and the stream still do not have + // a state. + // Present specially when using pipeline or stream + if (stream.state?.state == null || stream.state.state < 6) { + request.onComplete([]) + return + } + + // Stream is closed or half-closed-remote (6), decrement counter and cleanup + // It does not have sense to continue working with the stream as we do not + // have yet RST_STREAM support on client-side + session[kOpenStreams] -= 1 + if (session[kOpenStreams] === 0) { + session.unref() + } + + const err = new InformationalError('HTTP/2: stream half-closed (remote)') + errorRequest(client, request, err) + util.destroy(stream, err) + }) + + stream.once('close', () => { + session[kOpenStreams] -= 1 + // TODO(HTTP/2): unref only if current streams count is 0 + if (session[kOpenStreams] === 0) { + session.unref() + } + }) + + stream.once('error', function (err) { + if (client[kHTTP2Session] && !client[kHTTP2Session].destroyed && !this.closed && !this.destroyed) { + session[kOpenStreams] -= 1 + util.destroy(stream, err) + } + }) + + stream.once('frameError', (type, code) => { + const err = new InformationalError(`HTTP/2: "frameError" received - type ${type}, code ${code}`) + errorRequest(client, request, err) + + if (client[kHTTP2Session] && !client[kHTTP2Session].destroyed && !this.closed && !this.destroyed) { + session[kOpenStreams] -= 1 + util.destroy(stream, err) + } + }) + + // stream.on('aborted', () => { + // // TODO(HTTP/2): Support aborted + // }) + + // stream.on('timeout', () => { + // // TODO(HTTP/2): Support timeout + // }) + + // stream.on('push', headers => { + // // TODO(HTTP/2): Support push + // }) + + // stream.on('trailers', headers => { + // // TODO(HTTP/2): Support trailers + // }) + + return true + + function writeBodyH2 () { + /* istanbul ignore else: assertion */ + if (!body) { + request.onRequestSent() + } else if (util.isBuffer(body)) { + assert(contentLength === body.byteLength, 'buffer body must have content length') + stream.cork() + stream.write(body) + stream.uncork() + stream.end() + request.onBodySent(body) + request.onRequestSent() + } else if (util.isBlobLike(body)) { + if (typeof body.stream === 'function') { + writeIterable({ + client, + request, + contentLength, + h2stream: stream, + expectsPayload, + body: body.stream(), + socket: client[kSocket], + header: '' + }) + } else { + writeBlob({ + body, + client, + request, + contentLength, + expectsPayload, + h2stream: stream, + header: '', + socket: client[kSocket] + }) + } + } else if (util.isStream(body)) { + writeStream({ + body, + client, + request, + contentLength, + expectsPayload, + socket: client[kSocket], + h2stream: stream, + header: '' + }) + } else if (util.isIterable(body)) { + writeIterable({ + body, + client, + request, + contentLength, + expectsPayload, + header: '', + h2stream: stream, + socket: client[kSocket] + }) + } else { + assert(false) + } + } +} + +function writeStream ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { + assert(contentLength !== 0 || client[kRunning] === 0, 'stream body cannot be pipelined') + + // For HTTP/2, is enough to pipe the stream + const pipe = pipeline( + body, + h2stream, + (err) => { + if (err) { + util.destroy(body, err) + util.destroy(h2stream, err) + } else { + request.onRequestSent() + } + } + ) + + pipe.on('data', onPipeData) + pipe.once('end', () => { + pipe.removeListener('data', onPipeData) + util.destroy(pipe) + }) + + function onPipeData (chunk) { + request.onBodySent(chunk) + } +} + +async function writeBlob ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { + assert(contentLength === body.size, 'blob body must have content length') + + try { + if (contentLength != null && contentLength !== body.size) { + throw new RequestContentLengthMismatchError() + } + + const buffer = Buffer.from(await body.arrayBuffer()) + + h2stream.cork() + h2stream.write(buffer) + h2stream.uncork() + + request.onBodySent(buffer) + request.onRequestSent() + + if (!expectsPayload) { + socket[kReset] = true + } + + client[kResume]() + } catch (err) { + util.destroy(h2stream) + } +} + +async function writeIterable ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) { + assert(contentLength !== 0 || client[kRunning] === 0, 'iterator body cannot be pipelined') + + let callback = null + function onDrain () { + if (callback) { + const cb = callback + callback = null + cb() + } + } + + const waitForDrain = () => new Promise((resolve, reject) => { + assert(callback === null) + + if (socket[kError]) { + reject(socket[kError]) + } else { + callback = resolve + } + }) + + h2stream + .on('close', onDrain) + .on('drain', onDrain) + + try { + // It's up to the user to somehow abort the async iterable. + for await (const chunk of body) { + if (socket[kError]) { + throw socket[kError] + } + + const res = h2stream.write(chunk) + request.onBodySent(chunk) + if (!res) { + await waitForDrain() + } + } + } catch (err) { + h2stream.destroy(err) + } finally { + request.onRequestSent() + h2stream.end() + h2stream + .off('close', onDrain) + .off('drain', onDrain) + } +} + +module.exports = connectH2 From b5b2669f531c18259c3d87df2937488e58ea5c4b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 2 Apr 2024 11:21:11 +0200 Subject: [PATCH 11/11] fixup Signed-off-by: Matteo Collina --- lib/dispatcher/client-h2.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 0a53b75e501..6b48ab9904e 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -405,6 +405,18 @@ function writeH2 (client, request) { const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers request.onResponseStarted() + // Due to the stream nature, it is possible we face a race condition + // where the stream has been assigned, but the request has been aborted + // the request remains in-flight and headers hasn't been received yet + // for those scenarios, best effort is to destroy the stream immediately + // as there's no value to keep it open. + if (request.aborted || request.completed) { + const err = new RequestAbortedError() + errorRequest(client, request, err) + util.destroy(stream, err) + return + } + if (request.onHeaders(Number(statusCode), parseH2Headers(realHeaders), stream.resume.bind(stream), '') === false) { stream.pause() }