-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is 'connection: close' still needed now? #1496
Comments
The commit only sets 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,
)
}) |
The code is flawed if you assume the proxy reaches HTTP and HTTPS origins.
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 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. |
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?
The text was updated successfully, but these errors were encountered: