node-http-proxy icon indicating copy to clipboard operation
node-http-proxy copied to clipboard

Is 'connection: close' still needed now?

Open lispc opened this issue 3 years ago • 2 comments

In this commit 7 years ago, 'connection: close' is added to header due to as node core doesn't handle this COMPLETELY properly yet.

I wonder is this still needed today?

lispc avatar Dec 30 '20 07:12 lispc

The commit only sets Connection: close if no agent is used. Setting an agent worked fine for me:

const agent = new http.Agent({keepAlive: true})
const proxyServer = httpProxy.createProxyServer()
app.use("/api/db", (request, response, next) => {
    proxyServer.web(
        request,
        response,
        {agent, target: {host: hostname, port, path}},
        next,
    )
})

michael42 avatar Aug 28 '21 09:08 michael42

The commit only sets Connection: close if no agent is used. Setting an agent worked fine for me:

The code is flawed if you assume the proxy reaches HTTP and HTTPS origins.

Not found because of proxy error: TypeError [ERR_INVALID_PROTOCOL]: Protocol "http:" not supported. Expected "https:"

Or the other way around. Its more complicated to support both protocols, and nobody "has it right", in any downstream code I've seen using node-http-proxy.

https://github.com/http-party/node-http-proxy/blob/master/lib/http-proxy.js#L28 Implies use a global, but whoops, now your missing Clear or SSL protocols

https://github.com/http-party/node-http-proxy/blob/master/lib/http-proxy/passes/web-incoming.js#L111 but dispatches clear/SSL per request, but agent: is a global typically.....

I think I see how to fix this in my code, but the docs for node-http-proxy and downstream code are all a trap using a global agent, instead of 2 agent objects and per req dispatch.

bulk88 avatar Jun 21 '23 19:06 bulk88