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

Added check for POST method in http2_server #4241

Merged
merged 2 commits into from
Mar 8, 2021

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented Mar 3, 2021

Fixes issue #2881.

Added check to after HTTP/2 stream logic checks in order for the precedence of error checking to account for HTTP/2 first, as gRPC rests on top of HTTP/2.

@zasweq zasweq added this to the 1.37 Release milestone Mar 3, 2021
easwars
easwars previously requested changes Mar 3, 2021
internal/transport/http2_server.go Show resolved Hide resolved
internal/transport/http_util.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
internal/transport/transport_test.go Outdated Show resolved Hide resolved
@zasweq
Copy link
Contributor Author

zasweq commented Mar 4, 2021

@dfawley Tested on what's currently in master and fails :).

@zasweq zasweq assigned easwars and unassigned zasweq Mar 4, 2021
@dfawley dfawley assigned zasweq and unassigned easwars Mar 5, 2021
@ejona86
Copy link
Member

ejona86 commented May 10, 2021

It is inappropriate to fail the stream with a PROTOCOL_ERROR; no HTTP/2 protocol error occurred. This should be an HTTP-level failure.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants