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): ignore chunked trailers #2357

Merged
merged 1 commit into from
Dec 15, 2020
Merged

fix(h1): ignore chunked trailers #2357

merged 1 commit into from
Dec 15, 2020

Conversation

alpire
Copy link
Contributor

@alpire alpire commented Dec 13, 2020

Previously, hyper returned an "Invalid chunk end CR" error on chunked
responses with trailers, as described in RFC 7230 Section 4.1.2. This
commit adds code to ignore the trailers.

Closes #2171

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for writing this up! I really appreciate you including a test too!

For completeness, would you mind adding a couple more tests (either in tests/, or just testing the decoder in proto::h1::decode::tests)? It'd be good to check that this will handle multiple trailer liners (it looks like it does correctly), and perhaps some error cases.

Previously, hyper returned an "Invalid chunk end CR" error on chunked
responses with trailers, as described in RFC 7230 Section 4.1.2. This
commit adds code to ignore the trailers.

Closes #2171
@alpire
Copy link
Contributor Author

alpire commented Dec 15, 2020

Thanks for the review and feedback! I just added the additional tests you mentioned:

  • A test with multiple trailers (in tests/)
  • A test checking the new error case I introduced, occurring when the trailer is missing a new line (in proto::h1::decode::tests)

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks!

@seanmonstar seanmonstar merged commit 1dd761c into hyperium:master Dec 15, 2020
MOZGIII pushed a commit to vectordotdev/hyper that referenced this pull request Dec 16, 2020
Previously, hyper returned an "Invalid chunk end CR" error on chunked
responses with trailers, as described in RFC 7230 Section 4.1.2. This
commit adds code to ignore the trailers.

Closes hyperium#2171
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
Previously, hyper returned an "Invalid chunk end CR" error on chunked
responses with trailers, as described in RFC 7230 Section 4.1.2. This
commit adds code to ignore the trailers.

Closes hyperium#2171
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.

Decoding response from gRPC REST proxy results in "Invalid chunk end CR" error
2 participants