Skip to content

fix: Don't send content length header for unary gRPC responses#2075

Merged
johanandren merged 2 commits intomainfrom
wip-dont-send-unary-response-content-length
Aug 21, 2025
Merged

fix: Don't send content length header for unary gRPC responses#2075
johanandren merged 2 commits intomainfrom
wip-dont-send-unary-response-content-length

Conversation

@johanandren
Copy link
Copy Markdown
Contributor

Motivation in #2071 depends on upstream Akka HTTP change akka/akka-http#4504

@johanandren johanandren marked this pull request as ready for review August 20, 2025 15:23
@johanandren
Copy link
Copy Markdown
Contributor Author

In addition to the built in Netty interop tests I also manually tested this out with grpcurl and grpcui (possible they have the same underpinnings though) and both worked fine without the content length header.

@johanandren johanandren added this to the 2.5.6 milestone Aug 20, 2025
@johanandren johanandren requested a review from patriknw August 20, 2025 15:29
Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM if my question is not relevant

headers = headers,
entity = HttpEntity(contentType, encodeDataToFrameBytes(codec, data, trailer)),
entity =
HttpEntity.Strict(contentType, encodeDataToFrameBytes(codec, data, trailer), reportContentLength = false),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there is no risk that this changes the behavior (for others) in undesired way and we should enable it via a config instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was also worried about that, but James has assured me. The Content-Length header is redundant with HTTP/2 but can be included for backwards compatibility, the binary gRPC protocol however was always only HTTP/2, so unlikely that some client would assume HTTP/1.1 stuff.

@johanandren johanandren merged commit 2a0a5e9 into main Aug 21, 2025
12 checks passed
@johanandren johanandren deleted the wip-dont-send-unary-response-content-length branch August 21, 2025 07:54
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