[https-proxy-agent] Properly reject errors during proxy CONNECT response (#184)

Fixes #162.

Author: TooTallNate

.changeset/eleven-seas-rule.md

@@ -0,0 +1,5 @@
+---
+"https-proxy-agent": patch
+---
+
+Properly reject errors during proxy `CONNECT` response

packages/https-proxy-agent/package.json

@@ -36,7 +36,7 @@
     "@types/debug": "4",
     "@types/jest": "^29.5.1",
     "@types/node": "^14.18.45",
-    "async-listen": "^2.1.0",
+    "async-listen": "^3.0.0",
     "async-retry": "^1.3.3",
     "jest": "^29.5.0",
     "proxy": "workspace:*",

packages/https-proxy-agent/src/parse-proxy-response.ts

@@ -30,16 +30,17 @@ export function parseProxyResponse(
 		function cleanup() {
 			socket.removeListener('end', onend);
 			socket.removeListener('error', onerror);
-			socket.removeListener('close', onclose);
 			socket.removeListener('readable', read);
 		}
 
-		function onclose(err?: Error) {
-			debug('onclose had error %o', err);
-		}
-
 		function onend() {
+			cleanup();
 			debug('onend');
+			reject(
+				new Error(
+					'Proxy connection ended before receiving CONNECT response'
+				)
+			);
 		}
 
 		function onerror(err: Error) {
@@ -65,7 +66,10 @@ export function parseProxyResponse(
 			const headerParts = buffered.toString('ascii').split('\r\n');
 			const firstLine = headerParts.shift();
 			if (!firstLine) {
-				throw new Error('No header received');
+				socket.destroy();
+				return reject(
+					new Error('No header received from proxy CONNECT response')
+				);
 			}
 			const firstLineParts = firstLine.split(' ');
 			const statusCode = +firstLineParts[1];
@@ -75,7 +79,12 @@ export function parseProxyResponse(
 				if (!header) continue;
 				const firstColon = header.indexOf(':');
 				if (firstColon === -1) {
-					throw new Error(`Invalid header: "${header}"`);
+					socket.destroy();
+					return reject(
+						new Error(
+							`Invalid header from proxy CONNECT response: "${header}"`
+						)
+					);
 				}
 				const key = header.slice(0, firstColon).toLowerCase();
 				const value = header.slice(firstColon + 1).trimStart();
@@ -88,7 +97,7 @@ export function parseProxyResponse(
 					headers[key] = value;
 				}
 			}
-			debug('got proxy server response: %o', firstLine);
+			debug('got proxy server response: %o %o', firstLine, headers);
 			cleanup();
 			resolve({
 				connect: {
@@ -101,7 +110,6 @@ export function parseProxyResponse(
 		}
 
 		socket.on('error', onerror);
-		socket.on('close', onclose);
 		socket.on('end', onend);
 
 		read();

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

@@ -1,4 +1,5 @@
 import fs from 'fs';
+import net from 'net';
 import http from 'http';
 import https from 'https';
 import assert from 'assert';
@@ -32,25 +33,25 @@ describe('HttpsProxyAgent', () => {
 	beforeAll(async () => {
 		// setup target HTTP server
 		server = http.createServer();
-		serverUrl = (await listen(server)) as URL;
+		serverUrl = await listen(server);
 	});
 
 	beforeAll(async () => {
 		// setup HTTP proxy server
 		proxy = createProxy();
-		proxyUrl = (await listen(proxy)) as URL;
+		proxyUrl = await listen(proxy);
 	});
 
 	beforeAll(async () => {
 		// setup target HTTPS server
 		sslServer = https.createServer(sslOptions);
-		sslServerUrl = (await listen(sslServer)) as URL;
+		sslServerUrl = await listen(sslServer);
 	});
 
 	beforeAll(async () => {
 		// setup SSL HTTP proxy server
 		sslProxy = createProxy(https.createServer(sslOptions));
-		sslProxyUrl = (await listen(sslProxy)) as URL;
+		sslProxyUrl = await listen(sslProxy);
 	});
 
 	beforeEach(() => {
@@ -197,7 +198,11 @@ describe('HttpsProxyAgent', () => {
 
 			const connectPromise = once(server, 'connect');
 
-			http.get({ agent });
+			http.get({ agent }).on('error', () => {
+				// "error" happens because agent didn't receive proper
+				// CONNECT response before the socket was closed.
+				// We can safely ignore that.
+			});
 
 			const [req, socket] = await connectPromise;
 			assert.equal('CONNECT', req.method);
@@ -212,15 +217,23 @@ describe('HttpsProxyAgent', () => {
 			});
 
 			const connectPromise = once(server, 'connect');
-			http.get({ agent });
+			http.get({ agent }).on('error', () => {
+				// "error" happens because agent didn't receive proper
+				// CONNECT response before the socket was closed.
+				// We can safely ignore that.
+			});
 
 			const [req, socket] = await connectPromise;
 			assert.equal('CONNECT', req.method);
 			assert.equal('1', req.headers.number);
 			socket.destroy();
 
 			const connectPromise2 = once(server, 'connect');
-			http.get({ agent });
+			http.get({ agent }).on('error', () => {
+				// "error" happens because agent didn't receive proper
+				// CONNECT response before the socket was closed.
+				// We can safely ignore that.
+			});
 
 			const [req2, socket2] = await connectPromise2;
 			assert.equal('CONNECT', req2.method);
@@ -252,6 +265,30 @@ describe('HttpsProxyAgent', () => {
 				agent.destroy();
 			}
 		});
+
+		it('should emit "error" on request if proxy has invalid header', async () => {
+			const badProxy = net.createServer((socket) => {
+				socket.write(
+					'HTTP/1.1 200 Connection established\r\nbadheader\r\n\r\n'
+				);
+			});
+			const addr = await listen(badProxy);
+			let err: Error | undefined;
+			try {
+				const agent = new HttpsProxyAgent(
+					addr.href.replace('tcp', 'http')
+				);
+				await req('http://example.com', { agent });
+			} catch (_err) {
+				err = _err as Error;
+			} finally {
+				badProxy.close();
+			}
+			assert(err);
+			expect(err.message).toEqual(
+				'Invalid header from proxy CONNECT response: "badheader"'
+			);
+		});
 	});
 
 	describe('"https" module', () => {