On bad response, the socket is destroyed only when the proxy server closes the connection

[kadler15]

Summary

hpagent receives a socket on proxy server response to the CONNECT request. On non-200 response, hpagent passes an Error to the createConnection callback without first destroying the socket.

While it's highly unlikely the proxy server will fail to close the connection immediately after responding with a bad status code, I'm nervous about the complete lack of client control over socket cleanup.

Environment

• Node v16.15.1
• hpagent v1.0.0
• Linux kernel 5.13

Steps to reproduce

Run a simple web server on port 80 that returns 403 in response to any request:

#!/bin/bash
while true; do
  echo -e "HTTP/1.1 403 FORBIDDEN\r\n$(date)\r\n\r\n<h1>hello world from $(hostname) on $(date)</h1>" | nc -vl -p 80
done

Run the following test script:

const https = require('https');
const {HttpsProxyAgent} = require('hpagent');

const agent = new HttpsProxyAgent({
  proxy: 'http://127.0.0.1:80',
  timeout: 2000
});

const req = https.get('https://www.google.com', {agent})
  .on('error', console.log)
  .on('response', console.log)
  .on('socket', (sock) => {
    console.log(`Got TLSSocket`);
    sock.on('close', () => console.log(`TLSSocket closed`));
    sock.on('connect', () => console.log(`TLSSocket connected`));
    sock.on('timeout', () => {
      console.log(`TLSSocket timeout`);
      sock.destroy();
    });
  })
  .on('timeout', () => console.log(`timeout`))
  .end();

// Uncomment the following line to print active handles after 5 seconds
//setTimeout(() => console.log(process._getActiveHandles()), 5000);

Note that Node doesn't exit until the server is killed, because the proxy socket remains connected, with no ability to destroy it on the client side.

A potential fix

Call socket.destroy() before passing a bad response error to the createConnection callback. E.g.:

      if (response.statusCode === 200) {
        const secureSocket = super.createConnection({ ...options, socket })
        callback(null, secureSocket)
      } else {
        socket.destroy();
        callback(new Error(`Bad response: ${response.statusCode}`), null)
      }