Do not propagate origin Connection:close to client http/2 session. #3061
Conversation
|
If Connection: close is propagated, that's the problem. However, the block is just using the header sent from ATS, if I implemented correctly. If response header between ATS and a client has Connection: close, that means that ATS (core or some plugins) wants to close the connection. Since Connection: close header is not available on H2, that block initiates graceful shutdown instead. |
|
yea, I feel like you removed the wrong block. |
zwoop
left a comment
zwoop
left a comment
There was a problem hiding this comment.
Yeh, this feels wrong, we should talk to try to understand what exactly the problem is.
|
Hmm. You are right. This is just handling the response from ATS to client. I encountered this problem running autest on my H2 to origin branch, so my directions got a bit messed up. I did have this check excluded for the other direction. I still think there is a problem with the http2 autest, but it may be a test set up issue instead of a core issue. Will close this for now and dig into this further. |
|
@shinrich Maybe because of |
4 participants
This is a concern about the commit created from PR #2106. I like the commit in general, but this bit that closes the HTTP2 session if the origin response comes back with Connection: close seems wrong.
In the HTTP/1.1 case we do not propagate connection close from origin to client. According the HTTP standard the connection header is a hop-by-hop header and should not be propagated in any case. The HttpTransactHeaders::copy_header_fields() will drop the hop-by-hop headers when moving between client and origin (or origin and client).
I noticed this while testing a build. The http2 test would fail due to errors about not being able to start new streams due to a half open session. Track it down to the fact that the shutdown timer was being initiated after the first response (which had the connection: close header).