From 80d32d1a1688946c959e5a11b8669c2405789521 Mon Sep 17 00:00:00 2001 From: NUAAwqy <146524024+NUAAwqy@users.noreply.github.com> Date: Tue, 13 Oct 2026 12:52:24 +0800 Subject: [PATCH] fix(sign): Fixed proxy routing with native fetch Replace the ignored agent option with an Undici proxy dispatcher. Close each dispatcher gracefully without blocking response body consumption. Add real CONNECT proxy regressions for uploads, polling, downloads, credentials and failure behavior. Fixes #3413 Signed-off-by: NUAAwqy <146524024+NUAAwqy@users.noreply.github.com> --- package-lock.json | 41 ++-- package.json | 2 +- src/util/submit-addon.js | 22 ++- .../unit/test-util/test.submit-addon-proxy.js | 177 ++++++++++++++++++ tests/unit/test-util/test.submit-addon.js | 8 +- 5 files changed, 212 insertions(+), 38 deletions(-) create mode 100644 tests/unit/test-util/test.submit-addon-proxy.js diff --git a/package-lock.json b/package-lock.json index 64e4a6d4e0..430c5a09ea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,6 @@ "es6-error": "4.1.1", "firefox-profile": "4.7.1", "fx-runner": "1.7.0", - "https-proxy-agent": "^7.0.0", "jose": "5.9.6", "jszip": "3.10.2", "multimatch": "6.0.0", @@ -31,6 +30,7 @@ "strip-bom": "5.0.0", "strip-json-comments": "5.0.3", "tmp": "0.2.7", + "undici": "^6.29.0", "update-notifier": "7.3.1", "watchpack": "2.5.2", "yargs": "17.7.3", @@ -2815,15 +2815,6 @@ "node": ">=14.0" } }, - "node_modules/agent-base": { - "version": "7.1.4", - "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-7.1.4.tgz", - "integrity": "sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==", - "license": "MIT", - "engines": { - "node": ">= 14" - } - }, "node_modules/aggregate-error": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/aggregate-error/-/aggregate-error-3.1.0.tgz", @@ -3564,6 +3555,15 @@ "url": "https://github.com/sponsors/fb55" } }, + "node_modules/cheerio/node_modules/undici": { + "version": "7.30.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.0.tgz", + "integrity": "sha512-dkrQXeHSaoamnItlYbmzG0wFYrM0ZwDxCIg0A7aKjTyyhh9svRzCNFEzV+Vm05/yehjCzjDZ31KXfGEjYSztDQ==", + "license": "MIT", + "engines": { + "node": ">=20.18.1" + } + }, "node_modules/chokidar": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-5.0.0.tgz", @@ -5738,19 +5738,6 @@ "url": "https://github.com/fb55/entities?sponsor=1" } }, - "node_modules/https-proxy-agent": { - "version": "7.0.6", - "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz", - "integrity": "sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==", - "license": "MIT", - "dependencies": { - "agent-base": "^7.1.2", - "debug": "4" - }, - "engines": { - "node": ">= 14" - } - }, "node_modules/iconv-lite": { "version": "0.6.3", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.6.3.tgz", @@ -10214,12 +10201,12 @@ } }, "node_modules/undici": { - "version": "7.30.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.0.tgz", - "integrity": "sha512-dkrQXeHSaoamnItlYbmzG0wFYrM0ZwDxCIg0A7aKjTyyhh9svRzCNFEzV+Vm05/yehjCzjDZ31KXfGEjYSztDQ==", + "version": "6.29.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.29.0.tgz", + "integrity": "sha512-R+RODBqp6i2pPflGdq+xIOUkl+RNfGgHwoinecKu/JCuf2uO06cOKoDbI2P7Dn6KcswdKwrczbU6IYJ6K8X+wg==", "license": "MIT", "engines": { - "node": ">=20.18.1" + "node": ">=18.17" } }, "node_modules/undici-types": { diff --git a/package.json b/package.json index cd5225f093..14bec77c7c 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "es6-error": "4.1.1", "firefox-profile": "4.7.1", "fx-runner": "1.7.0", - "https-proxy-agent": "^7.0.0", "jose": "5.9.6", "jszip": "3.10.2", "multimatch": "6.0.0", @@ -78,6 +77,7 @@ "strip-bom": "5.0.0", "strip-json-comments": "5.0.3", "tmp": "0.2.7", + "undici": "^6.29.0", "update-notifier": "7.3.1", "watchpack": "2.5.2", "yargs": "17.7.3", diff --git a/src/util/submit-addon.js b/src/util/submit-addon.js index 32b01f98d2..e8e0a523d5 100644 --- a/src/util/submit-addon.js +++ b/src/util/submit-addon.js @@ -5,7 +5,7 @@ import { promises as streamPromises } from 'stream'; import { SignJWT } from 'jose'; import JSZip from 'jszip'; -import { HttpsProxyAgent } from 'https-proxy-agent'; +import { ProxyAgent } from 'undici'; import { isErrorWithCode } from '../errors.js'; import { createLogger } from './../util/logger.js'; @@ -110,8 +110,18 @@ export default class Client { return new File([fileData], basename(filePath)); } - nodeFetch(url, { method, headers, body, agent }) { - return fetch(url, { method, headers, body, agent }); + async nodeFetch(url, { method, headers, body, dispatcher }) { + try { + return await fetch(url, { method, headers, body, dispatcher }); + } finally { + // Close gracefully after the response body has been consumed. Awaiting + // close here would prevent the caller from receiving and reading it. + if (dispatcher) { + dispatcher.close().catch((error) => { + log.debug(`Closing proxy dispatcher failed: ${error}`); + }); + } + } } async doUploadSubmit(xpiPath, channel) { @@ -339,11 +349,11 @@ export default class Client { 'Content-Type': 'application/json', }; } - let agent; + let dispatcher; if (this.apiProxy) { - agent = new HttpsProxyAgent(this.apiProxy); + dispatcher = new ProxyAgent(this.apiProxy); } - return this.nodeFetch(url, { method, body, headers, agent }); + return this.nodeFetch(url, { method, body, headers, dispatcher }); } returnResult(addonId, downloadedFiles) { diff --git a/tests/unit/test-util/test.submit-addon-proxy.js b/tests/unit/test-util/test.submit-addon-proxy.js new file mode 100644 index 0000000000..72f0f0c88a --- /dev/null +++ b/tests/unit/test-util/test.submit-addon-proxy.js @@ -0,0 +1,177 @@ +import { once } from 'node:events'; +import { createServer } from 'node:http'; +import { connect } from 'node:net'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; + +import { assert } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import Client from '../../../src/util/submit-addon.js'; +import { withTempDir } from '../../../src/util/temp-dir.js'; + +describe('util.submit-addon proxy transport', () => { + let origin; + let proxy; + let baseUrl; + let apiProxy; + let requests; + let tunnels; + let sockets; + let rejectProxy; + + beforeEach(async () => { + requests = []; + tunnels = []; + sockets = new Set(); + rejectProxy = false; + origin = createServer(async (request, response) => { + const chunks = []; + for await (const chunk of request) { + chunks.push(chunk); + } + requests.push({ + method: request.method, + url: request.url, + headers: request.headers, + body: Buffer.concat(chunks).toString(), + }); + if (request.url === '/signed.xpi') { + response.write(Buffer.from([0x50, 0x4b])); + setTimeout(() => response.end(Buffer.from([0, 0xff])), 20); + } else { + response.setHeader('Content-Type', 'application/json'); + response.end(JSON.stringify({ received: true })); + } + }); + origin.listen(0, '127.0.0.1'); + await once(origin, 'listening'); + baseUrl = new URL(`http://127.0.0.1:${origin.address().port}/`); + + proxy = createServer(); + proxy.on('connect', (request, socket, head) => { + tunnels.push({ url: request.url, headers: request.headers }); + sockets.add(socket); + if (rejectProxy) { + socket.end('HTTP/1.1 502 Bad Gateway\r\n\r\n'); + return; + } + const upstream = connect(origin.address().port, '127.0.0.1'); + sockets.add(upstream); + upstream.on('error', () => socket.destroy()); + socket.on('error', () => upstream.destroy()); + upstream.on('connect', () => { + socket.write('HTTP/1.1 200 Connection Established\r\n\r\n'); + upstream.write(head); + socket.pipe(upstream).pipe(socket); + }); + }); + proxy.listen(0, '127.0.0.1'); + await once(proxy, 'listening'); + apiProxy = `http://127.0.0.1:${proxy.address().port}`; + }); + + afterEach(async () => { + for (const socket of sockets) { + socket.destroy(); + } + origin.closeAllConnections(); + proxy.closeAllConnections(); + await Promise.all([ + new Promise((resolve) => origin.close(resolve)), + new Promise((resolve) => proxy.close(resolve)), + ]); + }); + + const makeClient = (options = {}) => + new Client({ + baseUrl, + apiProxy, + apiAuth: { getAuthHeader: async () => 'JWT test-token' }, + userAgentString: 'web-ext-proxy-test', + ...options, + }); + + const checkProxy = () => { + assert.lengthOf(tunnels, 1); + assert.equal(tunnels[0].url, baseUrl.host); + assert.lengthOf(requests, 1); + assert.equal(requests[0].headers.authorization, 'JWT test-token'); + assert.equal(requests[0].headers['user-agent'], 'web-ext-proxy-test'); + }; + + it('routes JSON polling requests through the configured proxy', async () => { + assert.deepEqual(await makeClient().fetchJson(baseUrl), { received: true }); + checkProxy(); + assert.equal(requests[0].method, 'GET'); + }); + + it('preserves JSON version submissions through the proxy', async () => { + const body = JSON.stringify({ version: { upload: 'test-upload' } }); + await makeClient().fetchJson(baseUrl, 'PUT', body); + checkProxy(); + assert.equal(requests[0].method, 'PUT'); + assert.equal(requests[0].body, body); + assert.equal(requests[0].headers['content-type'], 'application/json'); + }); + + for (const method of ['POST', 'PATCH']) { + it(`preserves multipart ${method} uploads through the proxy`, async () => { + const body = new FormData(); + body.set('channel', 'unlisted'); + body.set('upload', new Blob(['extension-bytes']), 'extension.xpi'); + await makeClient().fetchJson(baseUrl, method, body); + checkProxy(); + assert.equal(requests[0].method, method); + assert.match( + requests[0].headers['content-type'], + /^multipart\/form-data; boundary=/, + ); + assert.include(requests[0].body, 'filename="extension.xpi"'); + assert.include(requests[0].body, 'extension-bytes'); + assert.include(requests[0].body, 'unlisted'); + }); + } + + it('streams signed file downloads through the proxy', () => + withTempDir(async (tmpDir) => { + const result = await makeClient({ + downloadDir: tmpDir.path(), + }).downloadSignedFile(new URL('signed.xpi', baseUrl), 'test-addon'); + checkProxy(); + assert.deepEqual(result.downloadedFiles, ['signed.xpi']); + assert.deepEqual( + readFileSync(path.join(tmpDir.path(), 'signed.xpi')), + Buffer.from([0x50, 0x4b, 0, 0xff]), + ); + })); + + it('supports URL-encoded proxy credentials', async () => { + const proxyUrl = new URL(apiProxy); + proxyUrl.username = 'user'; + proxyUrl.password = 'p%40ss'; + await makeClient({ apiProxy: proxyUrl.href }).fetchJson(baseUrl); + checkProxy(); + assert.equal( + tunnels[0].headers['proxy-authorization'], + `Basic ${Buffer.from('user:p@ss').toString('base64')}`, + ); + assert.notProperty(requests[0].headers, 'proxy-authorization'); + }); + + it('does not bypass a proxy that rejects the connection', async () => { + rejectProxy = true; + await assert.isRejected(makeClient().fetchJson(baseUrl), 'fetch failed'); + assert.lengthOf(tunnels, 1); + assert.lengthOf(requests, 0); + }); + + it('keeps direct requests working when no proxy is configured', async () => { + assert.deepEqual( + await makeClient({ apiProxy: undefined }).fetchJson(baseUrl), + { received: true }, + ); + assert.lengthOf(tunnels, 0); + assert.lengthOf(requests, 1); + }); +}); diff --git a/tests/unit/test-util/test.submit-addon.js b/tests/unit/test-util/test.submit-addon.js index 0d4e5d5fea..9f8d2af789 100644 --- a/tests/unit/test-util/test.submit-addon.js +++ b/tests/unit/test-util/test.submit-addon.js @@ -11,6 +11,7 @@ import { File } from 'node:buffer'; import CRC32 from 'crc-32'; import { assert, expect } from 'chai'; import JSZip from 'jszip'; +import { ProxyAgent } from 'undici'; import { afterEach, before, beforeEach, describe, it } from 'mocha'; import * as sinon from 'sinon'; @@ -1364,10 +1365,9 @@ describe('util.submit-addon', () => { await client.fetch(baseUrl, 'POST'); - assert.equal( - nodeFetchStub.firstCall.args[1].agent.proxy.host, - apiProxyHost, - ); + const { dispatcher } = nodeFetchStub.firstCall.args[1]; + assert.instanceOf(dispatcher, ProxyAgent); + await dispatcher.close(); sinon.assert.calledOnce(nodeFetchStub); }); });