fix: accept pre-response data on CONNECT (#284)

Author: szmarczak

package.json

@@ -1,6 +1,6 @@
 {
   "name": "proxy-chain",
-  "version": "2.0.6",
+  "version": "2.0.7",
   "description": "Node.js implementation of a proxy server (think Squid) with support for SSL, authentication, upstream proxy chaining, and protocol tunneling.",
   "main": "dist/index.js",
   "keywords": [

src/chain.ts

@@ -59,7 +59,14 @@ export const chain = (
     }: ChainOpts,
 ): void => {
     if (head && head.length > 0) {
-        throw new Error(`Unexpected data on CONNECT: ${head.length} bytes`);
+        // HTTP/1.1 has no defined semantics when sending payload along with CONNECT and servers can reject the request.
+        // HTTP/2 only says that subsequent DATA frames must be transferred after HEADERS has been sent.
+        // HTTP/3 says that all DATA frames should be transferred (implies pre-HEADERS data).
+        //
+        // Let's go with the HTTP/3 behavior.
+        // There are also clients that send payload along with CONNECT to save milliseconds apparently.
+        // Beware of upstream proxy servers that send out valid CONNECT responses with diagnostic data such as IPs!
+        sourceSocket.unshift(head);
     }
 
     const { proxyChainId } = sourceSocket;
@@ -118,8 +125,8 @@ export const chain = (
         }
 
         if (clientHead.length > 0) {
-            targetSocket.destroy(new Error(`Unexpected data on CONNECT: ${clientHead.length} bytes`));
-            return;
+            // See comment above
+            targetSocket.unshift(clientHead);
         }
 
         server.emit('tunnelConnectResponded', {

src/direct.ts

@@ -45,7 +45,8 @@ export const direct = (
     }
 
     if (head.length > 0) {
-        throw new Error(`Unexpected data on CONNECT: ${head.length} bytes`);
+        // See comment in chain.ts
+        sourceSocket.unshift(head);
     }
 
     const options = {

test/server.js

@@ -1331,6 +1331,63 @@ it('supports localAddress', async () => {
     }
 });
 
+it('supports pre-response CONNECT payload', (done) => {
+    const plain = net.createServer((socket) => {
+        socket.pipe(socket);
+    });
+
+    plain.once('error', done);
+
+    plain.listen(0, async () => {
+        const server = new Server({
+            port: 0,
+        });
+
+        try {
+            await server.listen();
+        } catch (error) {
+            done(error);
+            return;
+        }
+
+        const socket = net.connect({
+            host: '127.0.0.1',
+            port: server.port,
+        });
+
+        socket.write([
+            `CONNECT 127.0.0.1:${plain.address().port} HTTP/1.1`,
+            `Host: 127.0.0.1:${plain.address().port}`,
+            ``,
+            `foobar`,
+        ].join('\r\n'));
+
+        let success = false;
+
+        socket.once('error', done);
+        socket.on('data', (data) => {
+            success = data.includes('foobar');
+            socket.end();
+        });
+
+        socket.setTimeout(1000, () => {
+            socket.destroy(new Error('Socket timed out'));
+        });
+
+        socket.once('close', () => {
+            plain.close(async () => {
+                await server.close();
+
+                if (success) {
+                    done();
+                } else {
+                    done(new Error('failure'));
+                }
+            });
+        });
+    });
+});
+
 // Run all combinations of test parameters
 const useSslVariants = [
     false,