Skip to content
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

fix(h1): ending close-delimited body should close #2322

Merged
merged 1 commit into from Nov 6, 2020

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Nov 6, 2020

PR #2264 introduced a bug where calling end_body on close-delimited
encoders would not actually close the connection. This is because the
call to encoder.end() now returns Ok(None) for close-delimited
encoders, to prevent the error from being propagated as though the user
body ended too early. However, when Ok is returned, the write state is
only advanced to Closed if the encoder returns true from is_last.
This means that when the body is close-delimited, ending the body does
nothing, leaving the connection in keepalive.

This causes a hang in Linkerd, where we proxy HTTP requests connections
by returning a client body in a response. If the upstream server closes
the connection, we need to rely on the close always being propagated to
the connection with our client. This bug resulted in the body closing
abruptly leaving the connection in a stuck state, instead.

This branch fixes this by special-casing close-delimited bodies so that
they will also set the write state to Closed when calling end_body,
even if the is_last bit has not been set.

Signed-off-by: Eliza Weisman eliza@buoyant.io

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit to linkerd/linkerd2-proxy that referenced this pull request Nov 6, 2020
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@seanmonstar seanmonstar merged commit 71f3402 into hyperium:master Nov 6, 2020
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants