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(server): update proto::h1::end_body to return Result<()> #2264

Merged
merged 4 commits into from
Aug 12, 2020

Conversation

jxs
Copy link
Contributor

@jxs jxs commented Aug 10, 2020

No description provided.

@jxs jxs force-pushed the fix-nonchunk-stream branch from 8662dfa to 7185a5a Compare August 10, 2020 23:10
src/proto/h1/conn.rs Outdated Show resolved Hide resolved
@seanmonstar
Copy link
Member

Ok, so I understand what is happening with the failing test now. The problem is that in some states, the response needs to close the connection when the body is finished (close-delimited). This is when there is no content-length and no transfer-encoding. In that case, it's actually correct to just "close" the writing side, instead of dying in an error.

So, it seems we'd want to identify between "closing the body at this point is an actual error" vs "closing the body just means the socket should close cleanly".

@seanmonstar
Copy link
Member

To differentiate, we could return an enum that could be matched in the _not_eof case there.

@jxs
Copy link
Contributor Author

jxs commented Aug 12, 2020

updated Encoder::end to not return Err on CloseDelimited, only return Err when there's content-length left.
Updated Encoder eof tests, to match the behavior you described above: when there is no content-length and no transfer-encoding it's correct to close the writing side, so Encoder::end() should not return an Err

@seanmonstar seanmonstar merged commit 1ecbcbb into hyperium:master Aug 12, 2020
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.

2 participants