Skip to content

client: validate streaming gRPC response content types - #6

Open
EffortlessSteven wants to merge 1 commit into
mainfrom
claude/grpc-stream-response-content-type
Open

EffortlessSteven wants to merge 1 commit into
mainfrom
claude/grpc-stream-response-content-type

Conversation

@EffortlessSteven

@EffortlessSteven EffortlessSteven commented Aug 3, 2026 •

Copy link
Copy Markdown
Owner

What this does

Server-streaming and bidi receive initialization did not validate that a successful response's content-type matched the configured gRPC family or codec. In the trailers-only regression, a gRPC client accepted application/grpc-web+proto: call_server_stream returned Ok(ServerStream), then message() returned Ok(None).

make_server_stream now reuses the unary validator after response-status classification and before encoding or stream construction. Missing and bare-family content types remain accepted; Connect and non-200 HTTP precedence are unchanged.

Verification

Check Result
public trailers-only witness unpatched Ok(ServerStream) then Ok(None); patched Unknown before stream exposure
ordinary response heads family and codec mismatches rejected without grpc-status; headers and compatibility cases preserved
mutation proof six mutants killed: deleted or conditional validation, hard-coded selectors, wrong ordering, and wrong call site
compatibility runner 1,462 gRPC and 2,854 gRPC-Web cases passed with zero failures

Review map

  • validate_grpc_response_content_type: takes protocol and codec directly.
  • make_server_stream: shared server-streaming and bidi validation seam.
  • Tests and changelog: rejection and compatibility tests, HTTP 502 precedence, and the public false-success regression.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved content-type validation for server-streaming and bidirectional gRPC responses.
    • Incompatible protocol or codec declarations are now rejected earlier with appropriate errors.
    • Preserved compatibility for missing or bare content-type headers.
    • Maintained existing HTTP status handling and response-header behavior.

Walkthrough

The client now validates successful gRPC and gRPC-Web streaming response content types before exposing streams. The shared validator uses explicit protocol and codec values. Tests cover compatibility, rejection, error classification, status precedence, headers, and trailers-only responses.

Changes

gRPC content-type validation

Layer / File(s) Summary
Validator contract and unary integration
connectrpc/src/client/mod.rs
The validator now accepts explicit Protocol and CodecFormat values. Unary call sites and compatibility tests use the updated interface.
Streaming response validation
connectrpc/src/client/mod.rs
Server-stream and bidirectional-stream construction validates successful gRPC and gRPC-Web responses. Connect responses remain unchanged.
Streaming validation coverage
connectrpc/src/client/mod.rs, .changes/unreleased/*
Fixtures and tests cover compatible, incompatible, missing, and bare content types, error classification, status precedence, response headers, trailers-only responses, and the changelog records the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GrpcClient
  participant make_server_stream
  participant validate_grpc_response_content_type
  GrpcClient->>make_server_stream: construct successful streaming response
  make_server_stream->>validate_grpc_response_content_type: validate protocol and codec content type
  validate_grpc_response_content_type-->>make_server_stream: accept response or return validation error
  make_server_stream-->>GrpcClient: expose stream or return error
Loading

Suggested reviewers: iainmcgin, fallintoplace, rpb-ant

Poem

A rabbit checks the stream with care,
For family and codec declared there.
Wrong headers stop before the run,
Bare or missing ones still pass on.
Tests hop softly, green and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: validation of streaming gRPC response content types.
Description check ✅ Passed The description directly explains the streaming validation change, compatibility behavior, tests, and verification results.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/grpc-stream-response-content-type

Comment @coderabbitai help to get the list of available commands.

@EffortlessSteven
EffortlessSteven force-pushed the claude/grpc-stream-response-content-type branch from e78dafd to 316b0c4 Compare August 3, 2026 03:17
@EffortlessSteven

EffortlessSteven commented Aug 3, 2026 •

Copy link
Copy Markdown
Owner Author

Mutation receipts retained for upstream review:

Local mutation Discriminating failure
remove validation from make_server_stream direct matrix and public trailers-only witness returned Ok(ServerStream)
require initial-header grpc-status before validation ordinary response-head matrix returned Ok(ServerStream) while the trailers-only witness still passed
hard-code Protocol::Grpc gRPC-Web cross-family row returned Ok(ServerStream)
hard-code CodecFormat::Proto JSON-configured mismatch row returned Ok(ServerStream)
validate before non-200 handling 502 text/html became Unknown instead of Unavailable
validate only in call_server_stream direct shared-construction matrix returned Ok(ServerStream)

All mutations were restored before the final test runs. The custom missing-terminal-status runner also completed with gRPC 1,462 passed and gRPC-Web 2,854 passed, zero failures.

@EffortlessSteven
EffortlessSteven force-pushed the claude/grpc-stream-response-content-type branch from 316b0c4 to ec693c3 Compare August 3, 2026 05:17
Signed-off-by: EffortlessSteven <git@effortlesssteven.com>
@EffortlessSteven
EffortlessSteven force-pushed the claude/grpc-stream-response-content-type branch from ec693c3 to c8aec5c Compare August 3, 2026 05:48
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.

1 participant