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); }); });