Fix keepAlive for http-proxy-agent

.changeset/selfish-parrots-return.md

@@ -0,0 +1,5 @@
+---
+'http-proxy-agent': patch
+---
+
+Fixed the keepAlive option for HttpProxyAgent

packages/agent-base/test/test.ts

@@ -462,6 +462,7 @@ describe('Agent (TypeScript)', () => {
 				assert(gotReq);
 				expect(connectCount).toEqual(1);
 				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
 				res.resume();
 				const s1 = res.socket;
 
@@ -473,6 +474,7 @@ describe('Agent (TypeScript)', () => {
 				});
 				expect(connectCount).toEqual(1);
 				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
 				assert(res2.socket === s1);
 
 				res2.resume();

packages/http-proxy-agent/src/index.ts

@@ -29,6 +29,7 @@ interface HttpProxyAgentClientRequest extends http.ClientRequest {
 	}[];
 	_header?: string | null;
 	_implicitHeader(): void;
+	_updatedPropsForProxyAgent?: boolean;
 }
 
 function isHTTPS(protocol?: string | null): boolean {
@@ -71,15 +72,9 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		};
 	}
 
-	async connect(
-		req: HttpProxyAgentClientRequest,
-		opts: AgentConnectOpts
-	): Promise<net.Socket> {
-		const { proxy } = this;
-
-		const protocol = opts.secureEndpoint ? 'https:' : 'http:';
+	setRequestProps(req: HttpProxyAgentClientRequest, opts: AgentConnectOpts): void {
 		const hostname = req.getHeader('host') || 'localhost';
-		const base = `${protocol}//${hostname}`;
+		const base = `http://${hostname}`;
 		const url = new URL(req.path, base);
 		if (opts.port !== 80) {
 			url.port = String(opts.port);
@@ -89,6 +84,8 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		// to the absolute path of the URL that will be requested.
 		req.path = String(url);
 
+		const { proxy } = this;
+
 		// Inject the `Proxy-Authorization` header if necessary.
 		req._header = null;
 		if (proxy.username || proxy.password) {
@@ -108,6 +105,26 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 			);
 		}
 
+		req._updatedPropsForProxyAgent = true;
+	}
+
+	addRequest(req: HttpProxyAgentClientRequest, opts: AgentConnectOpts): void {
+		this.setRequestProps(req, opts);
+		// @ts-expect-error `addRequest()` isn't defined in `@types/node`
+		super.addRequest(req, opts);
+	}
+
+	async connect(
+		req: HttpProxyAgentClientRequest,
+		opts: AgentConnectOpts
+	): Promise<net.Socket> {
+		if (!req._updatedPropsForProxyAgent) {
+			// Maintain backwards compatibility with ProxyAgent and
+			// PacProxyAgent, which both rely on HttpProxyAgent to set request
+			// props inside the connect() function.
+			this.setRequestProps(req, opts);
+		}
+
 		// Create a socket connection to the proxy server.
 		let socket: net.Socket;
 		if (this.secureProxy) {
@@ -124,7 +141,9 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		let first: string;
 		let endOfHeaders: number;
 		debug('Regenerating stored HTTP header string for request');
-		req._implicitHeader();
+		if (!req._header) {
+			req._implicitHeader();
+		}
 		if (req.outputData && req.outputData.length > 0) {
 			// Node >= 12
 			debug(

packages/http-proxy-agent/test/test.ts

@@ -2,6 +2,7 @@ import * as fs from 'fs';
 import * as http from 'http';
 import * as https from 'https';
 import assert from 'assert';
+import { once } from 'events';
 import { createProxy, ProxyServer } from 'proxy';
 import { listen } from 'async-listen';
 import { json, req } from 'agent-base';
@@ -187,5 +188,32 @@ describe('HttpProxyAgent', () => {
 			const body = await json(res);
 			expect(body.host).toEqual(httpServerUrl.hostname);
 		});
+
+		it('should work with `keepAlive: true`', async () => {
+			httpServer.on('request', (req, res) => {
+				res.end(JSON.stringify(req.headers));
+			});
+
+			const agent = new HttpProxyAgent(proxyUrl, { keepAlive: true });
+
+			try {
+				const res = await req(httpServerUrl, { agent });
+				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
+				res.resume();
+				const s1 = res.socket;
+				await once(s1, 'free');
+
+				const res2 = await req(httpServerUrl, { agent });
+				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
+				res2.resume();
+				const s2 = res2.socket;
+				assert(s1 === s2);
+				await once(s2, 'free');
+			} finally {
+				agent.destroy();
+			}
+		});
 	});
 });

packages/https-proxy-agent/test/test.ts

@@ -215,12 +215,14 @@ describe('HttpsProxyAgent', () => {
 			try {
 				const res = await req(serverUrl, { agent });
 				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
 				res.resume();
 				const s1 = res.socket;
 				await once(s1, 'free');
 
 				const res2 = await req(serverUrl, { agent });
 				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
 				res2.resume();
 				const s2 = res2.socket;
 				assert(s1 === s2);
@@ -332,6 +334,7 @@ describe('HttpsProxyAgent', () => {
 					rejectUnauthorized: false,
 				});
 				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
 				res.resume();
 				const s1 = res.socket;
 				await once(s1, 'free');
@@ -341,6 +344,7 @@ describe('HttpsProxyAgent', () => {
 					rejectUnauthorized: false,
 				});
 				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
 				res2.resume();
 				const s2 = res2.socket;
 				assert(s1 === s2);

packages/pac-proxy-agent/src/index.ts

@@ -236,7 +236,10 @@ export class PacProxyAgent<Uri extends string> extends Agent {
 					const servername = opts.servername || opts.host;
 					socket = tls.connect({
 						...opts,
-						servername: (!servername || net.isIP(servername)) ? undefined : servername,
+						servername:
+							!servername || net.isIP(servername)
+								? undefined
+								: servername,
 					});
 				} else {
 					socket = net.connect(opts);

packages/pac-proxy-agent/test/test.ts

@@ -8,6 +8,7 @@ import { listen } from 'async-listen';
 import { ProxyServer, createProxy } from 'proxy';
 import { req, json } from 'agent-base';
 import { PacProxyAgent } from '../src';
+import { once } from 'events';
 
 const sslOptions = {
 	key: fs.readFileSync(`${__dirname}/ssl-cert-snakeoil.key`),
@@ -244,6 +245,44 @@ describe('PacProxyAgent', () => {
 			assert.equal(httpServerUrl.host, data.host);
 			assert(gotReq);
 		}, 10000); // This test is slow on Windows :/
+
+		it('should work with `keepAlive: true`', async () => {
+			httpServer.on('request', function (req, res) {
+				res.end(JSON.stringify(req.headers));
+			});
+
+			function FindProxyForURL() {
+				return 'PROXY localhost:PORT;';
+			}
+
+			const uri = `data:,${encodeURIComponent(
+				FindProxyForURL.toString().replace('PORT', proxyServerUrl.port)
+			)}`;
+			const agent = new PacProxyAgent(uri, { keepAlive: true });
+
+			try {
+				const res = await req(new URL('/test', httpServerUrl), {
+					agent,
+				});
+				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
+				res.resume();
+				const s1 = res.socket;
+				await once(s1, 'free');
+
+				const res2 = await req(new URL('/test', httpServerUrl), {
+					agent,
+				});
+				expect(res2.headers.connection).toEqual('keep-alive');
+				// expect(res2.statusCode).toEqual(200); // TODO: this assertion will cause this test to fail; uncomment in a follow-on PR to fix keepAlive for PacProxyAgent+HTTP
+				res2.resume();
+				const s2 = res2.socket;
+				assert(s1 === s2);
+				await once(s2, 'free');
+			} finally {
+				agent.destroy();
+			}
+		});
 	});
 
 	describe('"https" module', () => {
@@ -358,5 +397,46 @@ describe('PacProxyAgent', () => {
 			assert.equal(proxyCount, 4);
 			assert(gotReq);
 		}, 10000); // This test is slow on Windows :/
+
+		it('should work with `keepAlive: true`', async () => {
+			httpsServer.on('request', function (req, res) {
+				res.end(JSON.stringify(req.headers));
+			});
+
+			function FindProxyForURL() {
+				return 'PROXY localhost:PORT;';
+			}
+
+			const uri = `data:,${encodeURIComponent(
+				FindProxyForURL.toString().replace('PORT', proxyServerUrl.port)
+			)}`;
+			const agent = new PacProxyAgent(uri, {
+				keepAlive: true,
+				rejectUnauthorized: false,
+			});
+
+			try {
+				const res = await req(new URL('/test', httpsServerUrl), {
+					agent,
+				});
+				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
+				res.resume();
+				const s1 = res.socket;
+				await once(s1, 'free');
+
+				const res2 = await req(new URL('/test', httpsServerUrl), {
+					agent,
+				});
+				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
+				res2.resume();
+				const s2 = res2.socket;
+				assert(s1 === s2);
+				await once(s2, 'free');
+			} finally {
+				agent.destroy();
+			}
+		});
 	});
 });

packages/proxy-agent/src/index.ts

@@ -65,7 +65,7 @@ export type ProxyAgentOptions = HttpProxyAgentOptions<''> &
 		httpsAgent?: http.Agent;
 		/**
 		 * A callback for dynamic provision of proxy for url.
-		 * Defaults to standard proxy environment variables, 
+		 * Defaults to standard proxy environment variables,
 		 * see https://www.npmjs.com/package/proxy-from-env for details
 		 */
 		getProxyForUrl?: GetProxyForUrlCallback;

packages/proxy-agent/test/test.ts

@@ -158,6 +158,7 @@ describe('ProxyAgent', () => {
 				});
 				res.resume();
 				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
 				const s1 = res.socket;
 				await once(s1, 'free');
 
@@ -166,6 +167,7 @@ describe('ProxyAgent', () => {
 				});
 				res2.resume();
 				expect(res2.headers.connection).toEqual('keep-alive');
+				// expect(res2.statusCode).toEqual(200); // TODO: this assertion will cause this test to fail; uncomment in a follow-on PR to fix keepAlive for ProxyAgent+HTTP
 				const s2 = res2.socket;
 				assert(s1 === s2);
 
@@ -254,7 +256,7 @@ describe('ProxyAgent', () => {
 
 		it('should call provided function with getProxyForUrl option', async () => {
 			let gotCall = false;
-			let urlParameter = "";
+			let urlParameter = '';
 			httpsServer.once('request', function (req, res) {
 				res.end(JSON.stringify(req.headers));
 			});
@@ -265,7 +267,7 @@ describe('ProxyAgent', () => {
 					gotCall = true;
 					urlParameter = u;
 					return httpsProxyServerUrl.href;
-				}
+				},
 			});
 			const requestUrl = new URL('/test', httpsServerUrl);
 			const res = await req(requestUrl, {