[BUG] PAC file proxy chaining over http not working

[smckee-r7]

Hey,
So while I was testing some behaviour with proxy chaining with PAC files. I noticed that http requests weren't being handled correctly. The specific flow that I was trying was: request -> mockttp proxy -> PAC file -> 2nd mockttp proxy -> web server.

Here's a script that shows the issue:

const mockttp = require("mockttp");

// start local server to host a pac file
const server = require("http").createServer((req, res) => {
  console.log("Request made to PAC file server");
  if (req.url === '/proxy.pac') {
    console.log("PAC file server response");
    res.writeHead(200, { 'Content-Type': 'text/javascript' });
    res.write(`
    function FindProxyForURL(url, host) {
      return "PROXY localhost:8096";
    }`);
  }
  res.end();
});
server.listen(5010, 'localhost', () => console.log("Started PAC file server on localhost:5010"));

// Launch proxies
(async () => {
  const https = await mockttp.generateCACertificate();

  // Proxy that uses PAC file
  const proxy = mockttp.getLocal({ https });
  await proxy.start(8095);
  console.log('Started 1st Proxy on 8095');
  await proxy.forAnyRequest().thenPassThrough({
    beforeRequest: req => {
      console.log('Reached the 1st Proxy:', req.url)
      return req;
    },
    ignoreHostHttpsErrors: true,
    proxyConfig: {
      proxyUrl: 'pac+http://localhost:5010/proxy.pac'
    }
  });

  // Proxy that handles requests from resolved PAC file
  const proxy2 = mockttp.getLocal({ https });
  await proxy2.start(8096);
  console.log('Started 2nd Proxy on 8096');
  await proxy2.forAnyRequest().thenPassThrough({
    ignoreHostHttpsErrors: true,
    beforeRequest: req => {
      console.log('Reached the 2nd Proxy:', req.url)
      return req;
    }
  });
})();


  /**
   * Failing Request: curl -k -x "http://127.0.0.1:8095/" "http://example.com/" -A "Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/81.0"
   * Working Request: curl -k -x "http://127.0.0.1:8095/" "https://example.com/" -A "Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/81.0"
   */

Basically https requests work as expected but http request fail to connect. From the bit of testing I did, it seems to have something to do with how mockttp sets the request headers as a raw array when forwarding requests and perhaps the setDefaultHeaders request option but I'm not entirely sure.

[pimterry]

I've done some digging, I think this is a http-proxy-agent bug. What happens is:

• In most cases, we send upstream requests with an array of raw headers (http.request({ ..., headers: ['k', 'v', 'k2', 'v2'] })). This isn't a commonly used API, but it's actively supported in Node and has been for many many years, and using this instead of flattening into a { k: v1, k2: v2 } object is lets us opt out of some header processing, to more precisely preserve headers when forwarding them.
• When using this API, Node doesn't do any HTTP header processing (by design) and so req.getHeader() and similar don't work - they just return undefined.
• Unlike all the other proxy agents, http-proxy-agent uses req.getHeader('host') as its only way to work out the request destination (here).
• Since that returns undefined, the upstream request goes to localhost instead, and fails.

Note that the other agents don't do this - e.g. https-proxy-agent and socks-proxy-agent both use opts.host (here and here respectively). I'm not sure why or what the consequences of this are - that needs some investigating.

This same issue would also apply to proxyUrl: "http://..." proxy configurations for Mockttp (without PAC) but we actually default to sending those using the CONNECT (HTTPS) agent in all cases and https-proxy-agent doesn't have this problem, so it just doesn't come up unless you configure a plain HTTP proxy via PAC file.

I'm fairly confident that all this means that Mockttp is doing the right thing: it passes a raw header array, but it also provides all the other correct options to define the request destination, which do work with the other proxy agents just fine. If you spot any specific issues with that, I'm happy to discuss.

If you want to fix this, I think you'll need to make a PR to http-proxy-agent. If you do so and it gets released, let me know! I'd be happy to update the dep here to get this sorted.

[smckee-r7]

Thanks, I'll raise it with them and look into perhaps fixing it there.

I did consider that it could be with pac-proxy-agent but I simply wanted to rule out mockttp first, since the array header format mockttp used was interesting. It obviously does exist/work I just couldn't find any NodeJS documentation mentioning it allowing an array format. Your explanation to use it to avoid header processing makes sense and is good to know for future though.

Anyway, I appreciate you taking your time to break it down and I'll look into it. Thanks again!