From 78b3bbe216ff2debbc4f65f35acb55e4dd62cf6f Mon Sep 17 00:00:00 2001 From: Nitzan Tamam Date: Tue, 22 Apr 2025 20:32:28 +0300 Subject: [PATCH 1/4] fix(https-proxy-agent): added missing proxy connection timeout --- packages/https-proxy-agent/src/index.ts | 205 +++++++++++++----------- 1 file changed, 110 insertions(+), 95 deletions(-) diff --git a/packages/https-proxy-agent/src/index.ts b/packages/https-proxy-agent/src/index.ts index 30cb5f2c..f2dfefd7 100644 --- a/packages/https-proxy-agent/src/index.ts +++ b/packages/https-proxy-agent/src/index.ts @@ -1,13 +1,12 @@ import * as net from 'net'; import * as tls from 'tls'; import * as http from 'http'; -import assert from 'assert'; import createDebug from 'debug'; import { Agent, AgentConnectOpts } from 'agent-base'; import { URL } from 'url'; import { parseProxyResponse } from './parse-proxy-response'; import type { OutgoingHttpHeaders } from 'http'; - +import * as assert from 'assert'; const debug = createDebug('https-proxy-agent'); const setServernameFromNonIpHost = < @@ -100,108 +99,124 @@ export class HttpsProxyAgent extends Agent { req: http.ClientRequest, opts: AgentConnectOpts ): Promise { - const { proxy } = this; + return new Promise((resolve, reject) => { + if (this.connectOpts?.timeout) { + setTimeout(() => { + socket?.destroy(); + reject(new Error('Proxy connection timeout')); + }, this.connectOpts.timeout); + } - if (!opts.host) { - throw new TypeError('No "host" provided'); - } + const { proxy } = this; + // Create a socket connection to the proxy server. + let socket: net.Socket; + if (proxy.protocol === 'https:') { + debug('Creating `tls.Socket`: %o', this.connectOpts); + socket = tls.connect( + setServernameFromNonIpHost(this.connectOpts) + ); + } else { + debug('Creating `net.Socket`: %o', this.connectOpts); + socket = net.connect(this.connectOpts); + } - // Create a socket connection to the proxy server. - let socket: net.Socket; - if (proxy.protocol === 'https:') { - debug('Creating `tls.Socket`: %o', this.connectOpts); - socket = tls.connect(setServernameFromNonIpHost(this.connectOpts)); - } else { - debug('Creating `net.Socket`: %o', this.connectOpts); - socket = net.connect(this.connectOpts); - } + const headers: OutgoingHttpHeaders = + typeof this.proxyHeaders === 'function' + ? this.proxyHeaders() + : { ...this.proxyHeaders }; - const headers: OutgoingHttpHeaders = - typeof this.proxyHeaders === 'function' - ? this.proxyHeaders() - : { ...this.proxyHeaders }; - const host = net.isIPv6(opts.host) ? `[${opts.host}]` : opts.host; - let payload = `CONNECT ${host}:${opts.port} HTTP/1.1\r\n`; - - // Inject the `Proxy-Authorization` header if necessary. - if (proxy.username || proxy.password) { - const auth = `${decodeURIComponent( - proxy.username - )}:${decodeURIComponent(proxy.password)}`; - headers['Proxy-Authorization'] = `Basic ${Buffer.from( - auth - ).toString('base64')}`; - } + if (!opts.host) { + reject(new TypeError('No "host" provided')); + } - headers.Host = `${host}:${opts.port}`; + const host = net.isIPv6(opts.host as string) + ? `[${opts.host}]` + : opts.host; + let payload = `CONNECT ${host}:${opts.port} HTTP/1.1\r\n`; + + // Inject the `Proxy-Authorization` header if necessary. + if (proxy.username || proxy.password) { + const auth = `${decodeURIComponent( + proxy.username + )}:${decodeURIComponent(proxy.password)}`; + headers['Proxy-Authorization'] = `Basic ${Buffer.from( + auth + ).toString('base64')}`; + } - if (!headers['Proxy-Connection']) { - headers['Proxy-Connection'] = this.keepAlive - ? 'Keep-Alive' - : 'close'; - } - for (const name of Object.keys(headers)) { - payload += `${name}: ${headers[name]}\r\n`; - } + headers.Host = `${host}:${opts.port}`; - const proxyResponsePromise = parseProxyResponse(socket); - - socket.write(`${payload}\r\n`); - - const { connect, buffered } = await proxyResponsePromise; - req.emit('proxyConnect', connect); - this.emit('proxyConnect', connect, req); - - if (connect.statusCode === 200) { - req.once('socket', resume); - - if (opts.secureEndpoint) { - // The proxy is connecting to a TLS server, so upgrade - // this socket connection to a TLS connection. - debug('Upgrading socket connection to TLS'); - return tls.connect({ - ...omit( - setServernameFromNonIpHost(opts), - 'host', - 'path', - 'port' - ), - socket, - }); + if (!headers['Proxy-Connection']) { + headers['Proxy-Connection'] = this.keepAlive + ? 'Keep-Alive' + : 'close'; + } + for (const name of Object.keys(headers)) { + payload += `${name}: ${headers[name]}\r\n`; } - return socket; - } - - // Some other status code that's not 200... need to re-play the HTTP - // header "data" events onto the socket once the HTTP machinery is - // attached so that the node core `http` can parse and handle the - // error status code. - - // Close the original socket, and a new "fake" socket is returned - // instead, so that the proxy doesn't get the HTTP request - // written to it (which may contain `Authorization` headers or other - // sensitive data). - // - // See: https://hackerone.com/reports/541502 - socket.destroy(); - - const fakeSocket = new net.Socket({ writable: false }); - fakeSocket.readable = true; - - // Need to wait for the "socket" event to re-play the "data" events. - req.once('socket', (s: net.Socket) => { - debug('Replaying proxy buffer for failed request'); - assert(s.listenerCount('data') > 0); - - // Replay the "buffered" Buffer onto the fake `socket`, since at - // this point the HTTP module machinery has been hooked up for - // the user. - s.push(buffered); - s.push(null); + parseProxyResponse(socket) + .then(({ connect, buffered }) => { + req.emit('proxyConnect', connect); + this.emit('proxyConnect', connect, req); + + if (connect.statusCode === 200) { + req.once('socket', resume); + + if (opts.secureEndpoint) { + // The proxy is connecting to a TLS server, so upgrade + // this socket connection to a TLS connection. + debug('Upgrading socket connection to TLS'); + resolve( + tls.connect({ + ...omit( + setServernameFromNonIpHost(opts), + 'host', + 'path', + 'port' + ), + socket, + }) + ); + } + + resolve(socket); + } + + // Some other status code that's not 200... need to re-play the HTTP + // header "data" events onto the socket once the HTTP machinery is + // attached so that the node core `http` can parse and handle the + // error status code. + + // Close the original socket, and a new "fake" socket is returned + // instead, so that the proxy doesn't get the HTTP request + // written to it (which may contain `Authorization` headers or other + // sensitive data). + // + // See: https://hackerone.com/reports/541502 + socket.destroy(); + + const fakeSocket = new net.Socket({ writable: false }); + fakeSocket.readable = true; + + // Need to wait for the "socket" event to re-play the "data" events. + req.once('socket', (s: net.Socket) => { + debug('Replaying proxy buffer for failed request'); + assert.ok(s.listenerCount('data') > 0); + + // Replay the "buffered" Buffer onto the fake `socket`, since at + // this point the HTTP module machinery has been hooked up for + // the user. + s.push(buffered); + s.push(null); + }); + + resolve(fakeSocket); + }) + .catch(reject); + + socket.write(`${payload}\r\n`); }); - - return fakeSocket; } } From 53f3dffcf8806574e30987c7df78bf14960e4f8e Mon Sep 17 00:00:00 2001 From: Nitzan Tamam Date: Tue, 22 Apr 2025 21:01:49 +0300 Subject: [PATCH 2/4] fix(https-proxy-agent): socket write moved before parse proxy response --- packages/https-proxy-agent/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/https-proxy-agent/src/index.ts b/packages/https-proxy-agent/src/index.ts index f2dfefd7..084a4e9c 100644 --- a/packages/https-proxy-agent/src/index.ts +++ b/packages/https-proxy-agent/src/index.ts @@ -155,6 +155,8 @@ export class HttpsProxyAgent extends Agent { payload += `${name}: ${headers[name]}\r\n`; } + socket.write(`${payload}\r\n`); + parseProxyResponse(socket) .then(({ connect, buffered }) => { req.emit('proxyConnect', connect); @@ -214,8 +216,6 @@ export class HttpsProxyAgent extends Agent { resolve(fakeSocket); }) .catch(reject); - - socket.write(`${payload}\r\n`); }); } } From 56e62a27d151472dd96531c22cda14d4a901b108 Mon Sep 17 00:00:00 2001 From: Nitzan Tamam Date: Tue, 22 Apr 2025 21:12:57 +0300 Subject: [PATCH 3/4] fix(https-proxy-agent): return resolve --- packages/https-proxy-agent/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/https-proxy-agent/src/index.ts b/packages/https-proxy-agent/src/index.ts index 084a4e9c..2d76c186 100644 --- a/packages/https-proxy-agent/src/index.ts +++ b/packages/https-proxy-agent/src/index.ts @@ -169,7 +169,7 @@ export class HttpsProxyAgent extends Agent { // The proxy is connecting to a TLS server, so upgrade // this socket connection to a TLS connection. debug('Upgrading socket connection to TLS'); - resolve( + return resolve( tls.connect({ ...omit( setServernameFromNonIpHost(opts), @@ -182,7 +182,7 @@ export class HttpsProxyAgent extends Agent { ); } - resolve(socket); + return resolve(socket); } // Some other status code that's not 200... need to re-play the HTTP @@ -213,7 +213,7 @@ export class HttpsProxyAgent extends Agent { s.push(null); }); - resolve(fakeSocket); + return resolve(fakeSocket); }) .catch(reject); }); From 650ea1b9bdce72ef388d9464ebe1633f67933534 Mon Sep 17 00:00:00 2001 From: Nitzan Tamam Date: Wed, 23 Apr 2025 16:50:38 +0300 Subject: [PATCH 4/4] fix(https-proxy-agent): addedm missing clear timeout --- packages/https-proxy-agent/src/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/https-proxy-agent/src/index.ts b/packages/https-proxy-agent/src/index.ts index 2d76c186..3855c44b 100644 --- a/packages/https-proxy-agent/src/index.ts +++ b/packages/https-proxy-agent/src/index.ts @@ -100,8 +100,9 @@ export class HttpsProxyAgent extends Agent { opts: AgentConnectOpts ): Promise { return new Promise((resolve, reject) => { + let connectionTimeout: NodeJS.Timeout | undefined; if (this.connectOpts?.timeout) { - setTimeout(() => { + connectionTimeout = setTimeout(() => { socket?.destroy(); reject(new Error('Proxy connection timeout')); }, this.connectOpts.timeout); @@ -161,8 +162,11 @@ export class HttpsProxyAgent extends Agent { .then(({ connect, buffered }) => { req.emit('proxyConnect', connect); this.emit('proxyConnect', connect, req); + + clearTimeout(connectionTimeout); if (connect.statusCode === 200) { + req.once('socket', resume); if (opts.secureEndpoint) {