Skip to content

Fix keepAlive for http-proxy-agent#169

Closed
jportner wants to merge 8 commits intoTooTallNate:mainfrom
jportner:fix-http-proxy-agent-keepalive
Closed

Fix keepAlive for http-proxy-agent#169
jportner wants to merge 8 commits intoTooTallNate:mainfrom
jportner:fix-http-proxy-agent-keepalive

Conversation

@jportner
Copy link
Copy Markdown
Contributor

@jportner jportner commented May 15, 2023

I was testing the new keepAlive option, and it turns out that while it works for https-proxy-agent, it doesn't work with http-proxy-agent.

I use Smokescreen, and when testing keepAlive with http-proxy-agent, the first request succeeded, but all subsequent requests failed with an error: "This is a proxy server. Does not respond to non-proxy requests"

Turns out, the first request used an absolute URL (http://localhost:3000/foo) while all subsequent requests used relative URLs (/foo). HTTP proxy servers require that HTTP requests use absolute URLs, which is why this happens. (HTTPS requests are not impacted because they are encrypted, and regular non-MITM proxies cannot read the URL).

The http-proxy-agent package already had logic to rewrite the relative URL to an absolute URL:

// Change the `http.ClientRequest` instance's "path" field
// to the absolute path of the URL that will be requested.
req.path = String(url);

However, that only happens on connect(), so it only worked on the first request. This PR makes the following changes:

  1. Updates existing tests and adds additional tests to demonstrate the problem: 1467942 (three tests now fail)
  2. Fixes the problem, and now the tests all pass: 6f3bc0d

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants