fix: validate grpc-accept-encoding header when client sends gzip request - #12978
Open
jlaportebot wants to merge 8 commits into
Open
fix: validate grpc-accept-encoding header when client sends gzip request#12978jlaportebot wants to merge 8 commits into
jlaportebot wants to merge 8 commits into
Conversation
- Add validation in Http2ClientStreamTransportState to check if server response includes grpc-accept-encoding: gzip when client sent gzip-encoded request - Log warning at FINE level when server misbehavior detected (missing or invalid header) - Add setMessageCompression(boolean, String) method to ClientStream interface and implementations to track when client sends gzip-compressed requests - Add unit tests for grpc-accept-encoding validation Fixes grpc#1804
…nProcessClientStream NettyClientStream extends AbstractClientStream but compilation failed because AbstractClientStream did not override the new ClientStream.setMessageCompression(boolean, String) method added to the interface. The method was only on the inner TransportState class. Add the override to AbstractClientStream that delegates to transportState(), matching the pattern of setDecompressorRegistry. Also add the override to InProcessClientStream (noop, matching existing setMessageCompression(boolean)). Signed-off-by: jlaportebot <jlaportebot@gmail.com>
NettyClientStream extends AbstractClientStream but compilation failed because AbstractClientStream did not override the new ClientStream.setMessageCompression(boolean, String) method added to the interface. The method was only on the inner TransportState class. Add the override to InProcessClientStream (noop, matching existing setMessageCompression(boolean)). Also add the override to AbstractClientStream that delegates to transportState(), matching the pattern of setDecompressorRegistry. Signed-off-by: jlaportebot <jlaportebot@gmail.com>
…ream and InProcessClientStream Both classes implement Stream interface which requires setMessageCompression(boolean). Previously only setMessageCompression(boolean, String) was implemented. Fixes compilation errors in grpc-inprocess module.
…n(boolean, String) The ServerStream interface only extends Stream and doesn't declare the two-argument setMessageCompression method. Only ClientStream has it. The @OverRide annotation was causing a compilation error. Signed-off-by: jlaportebot <jlaportebot@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements validation for the
grpc-accept-encodingresponse header when the client sends a gzip-encoded request.According to the gRPC spec, when a client sends a gzip-encoded request, the server must respond with
grpc-accept-encoding: gzipin the response headers to indicate it can accept gzip-encoded responses. If this header is missing or doesn't include gzip, it's a server misbehavior.Changes
Added validation logic in
Http2ClientStreamTransportState.transportHeadersReceived()that:setMessageCompression(boolean, String)grpc-accept-encodingheader when client sent gzipExtended
ClientStreaminterface withsetMessageCompression(boolean enabled, String compressorName)method to pass compressor information to the transport layerUpdated all ClientStream implementations to support the new method signature:
AbstractClientStream.TransportState(base implementation)Http2ClientStreamTransportState(actual validation logic)ForwardingClientStream,NoopClientStream,DelayedStream,RetriableStreamInProcessTransport(both client and server streams)MultiMessageClientStream,SingleMessageClientStream(binder transport)Added comprehensive unit tests in
Http2ClientStreamTransportStateGrpcAcceptEncodingTest.javacovering:grpc-accept-encoding: gzipheaderidentitywhen gzip was sent (logs warning)gzip,deflatewhen gzip was sent (OK)GZIP,gzip)Testing
All existing tests pass. New tests added for the validation logic.
Fixes #1804