update to hyper 0.12.7 to fix a keep-alive bug #26
Conversation
Specifically proxied bodies would make use of an optimization in hyper that resulted in the connection not knowing (but it did know! just didn't tell itself...) that the body was finished, and so the connection was closed. 0.12.7 includes the fix in hyper. As part of this upgrade, the keep-alive tests have been adjusted to send a small body, since the empty body was not triggering this case. Signed-off-by: Sean McArthur <[email protected]>
Should we have tests with an empty body as well, in case of future regressions that only occur when the body is empty, or does this seem too unlikely to be worth worrying about? |
🤷 We could. In the hyper PR, the test suite was updated to run those tests with the matrix of (no body, with body, no proxy, proxy). |
I agree that it seems very useful to test this with an empty body too. Pretty much any HTTP-level thing we do should be tested with an empty body. It's great that there's a hyper-level test but, especially since we have previously discussed refactoring things significantly to unify the hyper and tower-h2 logic, having linkerd2-level tests seems prudent, esp. since the "extra" test was already written. |
Signed-off-by: Sean McArthur <[email protected]>
|
I've updated that test to run each request against two routes, one returning a small body, and one returning no body. |
|
thanks for hunting this down, @seanmonstar ! I'm 👍 when @briansmith is. |
Previously `Process` did its own environment variable parsing and did not benefit from the improved error handling that `config` now has. Additionally, future changes will need access to these same environment variables in other parts of the proxy. Move `Process`'s environment variable parsing to `config` to address both of these issues. Now there are no uses of `env::var` outside of `config` except for logging, which is the final desired state. I validated this manually.
Sync linkerd/linkerd2-proxy:main
Specifically proxied bodies would make use of an optimization in hyper
that resulted in the connection not knowing (but it did know! just
didn't tell itself...) that the body was finished, and so the connection
was closed. 0.12.7 includes the fix in hyper.
As part of this upgrade, the keep-alive tests have been adjusted to send
a small body, since the empty body was not triggering this case.
I believe this fully fixes linkerd/linkerd2#1353