facebook
HTTP CONNECT
For HTTP CONNECT request, the Connection: keep-alive header is discarded and Connection: close header is added, even though the Connection: keep-alive header is set in the upstream for 407 status code.
Can you fix this issue.
In my case, I want to do a proxy chain, so the proxygen has to chain the response is received from upstream. In this case, it should set the headers as it is instead of removing Connection: keep-alive header.
Since it seems like a protocol issue. Can you fix this.
@SteveSelva Where is the Connection: keep-alive header being stripped? Is this happening within the codec when parsing a response?
@afrind This is happening while generating headers for downstream from the headers received from upstream. The upstream headers object contains the Connection: keep-alive header. But when the downstream headers object is generated from the upstream, the Connection: keep-alive is removed and instead Connection: close is added to it for a HTTP CONNECT request where the response is not between 200 - 300.
The Connection: keep-alive header is stripped at this line: https://github.com/facebook/proxygen/blob/5c693600d782017867243292b25350a0586ed0d0/proxygen/lib/http/codec/HTTP1xCodec.cpp#L565-L571
Here they have decided to generate Connection: keep-alive header using the keepAlive_ member.
But the keepAlive_ is already set to false for response of HTTP CONNECT request where the response status code is not between 200-300.
https://github.com/facebook/proxygen/blob/5c693600d782017867243292b25350a0586ed0d0/proxygen/lib/http/codec/HTTP1xCodec.cpp#L439-L454
Atlast when the keepAlive_ is false, then Connection: close is added at this line.
https://github.com/facebook/proxygen/blob/5c693600d782017867243292b25350a0586ed0d0/proxygen/lib/http/codec/HTTP1xCodec.cpp#L676-L680
I don't know how to fix this issue properly without impacting the HTTP Protocol. Can you fix it please.
As long as we haven't received any bytes beyond the headers, it should be ok to allow keep-alive on 4xx. Maybe adding a setParserPaused(true); when detecting the end of an upgrade request is the right move? If the app unpauses the parser before generating a 200-300, set keepAlive_ = false;?