diff --git a/__tests__/http-client-timeout.test.ts b/__tests__/http-client-timeout.test.ts new file mode 100644 index 000000000..e63e5b27c --- /dev/null +++ b/__tests__/http-client-timeout.test.ts @@ -0,0 +1,55 @@ +import {afterEach, describe, expect, it} from '@jest/globals'; +import {HttpClient} from '@actions/http-client'; +import net, {AddressInfo, Server, Socket} from 'node:net'; + +describe('@actions/http-client timeout cleanup', () => { + let server: Server | undefined; + const sockets = new Set(); + + afterEach(async () => { + for (const socket of sockets) { + socket.destroy(); + } + sockets.clear(); + if (server) { + await new Promise((resolve, reject) => { + server?.close(error => (error ? reject(error) : resolve())); + }); + server = undefined; + } + }); + + it('destroys a request whose socket times out', async () => { + server = net.createServer({allowHalfOpen: true}, socket => { + sockets.add(socket); + socket.on('data', () => undefined); + socket.on('close', () => sockets.delete(socket)); + }); + await new Promise((resolve, reject) => { + server?.listen(0, '127.0.0.1', () => resolve()); + server?.once('error', reject); + }); + + const {port} = server.address() as AddressInfo; + const client = new HttpClient('setup-java-timeout-test', [], { + socketTimeout: 50 + }); + + await expect(client.get(`http://127.0.0.1:${port}/jdk`)).rejects.toThrow( + 'Request timeout: /jdk' + ); + await new Promise(resolve => setTimeout(resolve, 250)); + const activeHandles = ( + process as typeof process & { + _getActiveHandles(): unknown[]; + } + )._getActiveHandles(); + const openClientSockets = activeHandles.filter( + (handle): handle is Socket => + handle instanceof Socket && + handle.remotePort === port && + !handle.destroyed + ); + expect(openClientSockets).toHaveLength(0); + }); +}); diff --git a/dist/cleanup/index.js b/dist/cleanup/index.js index 03ab6fe1f..4c15c6a4f 100644 --- a/dist/cleanup/index.js +++ b/dist/cleanup/index.js @@ -34202,10 +34202,9 @@ class HttpClient { }); // If we ever get disconnected, we want the socket to timeout eventually req.setTimeout(this._socketTimeout || 3 * 60000, () => { - if (socket) { - socket.end(); - } - handleResult(new Error(`Request timeout: ${info.options.path}`)); + const timeoutError = new Error(`Request timeout: ${info.options.path}`); + req.destroy(timeoutError); + handleResult(timeoutError); }); req.on('error', function (err) { // err has statusCode property diff --git a/dist/setup/index.js b/dist/setup/index.js index 5c0536646..91d60c2a6 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -34679,10 +34679,9 @@ class HttpClient { }); // If we ever get disconnected, we want the socket to timeout eventually req.setTimeout(this._socketTimeout || 3 * 60000, () => { - if (socket) { - socket.end(); - } - handleResult(new Error(`Request timeout: ${info.options.path}`)); + const timeoutError = new Error(`Request timeout: ${info.options.path}`); + req.destroy(timeoutError); + handleResult(timeoutError); }); req.on('error', function (err) { // err has statusCode property diff --git a/package.json b/package.json index 500787181..cc88f6a94 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "node": ">=24.0.0" }, "scripts": { - "build": "node scripts/patch-is-unsafe.mjs && ncc build -o dist/setup src/setup-java.ts && ncc build -o dist/cleanup src/cleanup-java.ts", + "build": "node scripts/patch-is-unsafe.mjs && node scripts/patch-http-client-timeout.mjs && ncc build -o dist/setup src/setup-java.ts && ncc build -o dist/cleanup src/cleanup-java.ts", "format": "prettier --no-error-on-unmatched-pattern --write \"**/*.{ts,yml,yaml}\"", "format-check": "prettier --no-error-on-unmatched-pattern --check \"**/*.{ts,yml,yaml}\"", "lint": "eslint \"**/*.ts\"", @@ -19,6 +19,7 @@ "prepare": "husky install", "prerelease": "npm run-script build", "release": "git add -f dist/setup/*.js dist/setup/package.json dist/cleanup/*.js", + "pretest": "node scripts/patch-http-client-timeout.mjs", "test": "node --experimental-vm-modules ./node_modules/jest/bin/jest.js --runInBand --coverage" }, "lint-staged": { diff --git a/scripts/patch-http-client-timeout.mjs b/scripts/patch-http-client-timeout.mjs new file mode 100644 index 000000000..2f51c3a57 --- /dev/null +++ b/scripts/patch-http-client-timeout.mjs @@ -0,0 +1,31 @@ +import {readFile, writeFile} from 'node:fs/promises'; + +const sourcePath = new URL( + '../node_modules/@actions/http-client/lib/index.js', + import.meta.url +); +const vulnerablePattern = `req.setTimeout(this._socketTimeout || 3 * 60000, () => { + if (socket) { + socket.end(); + } + handleResult(new Error(\`Request timeout: \${info.options.path}\`)); + });`; +const safePattern = `req.setTimeout(this._socketTimeout || 3 * 60000, () => { + const timeoutError = new Error(\`Request timeout: \${info.options.path}\`); + req.destroy(timeoutError); + handleResult(timeoutError); + });`; +const source = await readFile(sourcePath, 'utf8'); + +if (source.includes(safePattern)) { + process.exit(0); +} + +const occurrences = source.split(vulnerablePattern).length - 1; +if (occurrences !== 1) { + throw new Error( + `Expected one vulnerable timeout handler in ${sourcePath.pathname}, found ${occurrences}` + ); +} + +await writeFile(sourcePath, source.replace(vulnerablePattern, safePattern));