Skip to content

client: validate streaming gRPC response content types - #266

Merged
iainmcgin merged 2 commits into
connectrpc:mainfrom
EffortlessSteven:claude/grpc-stream-response-content-type-upstream
Aug 21, 2026
Merged

client: validate streaming gRPC response content types#266
iainmcgin merged 2 commits into
connectrpc:mainfrom
EffortlessSteven:claude/grpc-stream-response-content-type-upstream

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Contributor

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.

Fixes #203.

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
#1123 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.

EffortlessSteven and others added 2 commits August 21, 2026 09:44
Signed-off-by: EffortlessSteven <git@effortlesssteven.com>
…status

Share the unary path's HTTP-status / content-type gate with
make_server_stream as check_grpc_response_head, and run it before the
trailers-only check. A trailers-only reply in the wrong content-type
family is now rejected on both paths instead of being believed for its
grpc-status on the streaming one, and a non-2xx reply that is not
speaking gRPC carries the same content-type detail after its HTTP status.

Adds a parity test driving the same response head through both parsers
across the status / content-type / grpc-status matrix, two accept-side
rows (grpc+json exact match; Connect streaming unaffected), and corrects
a test comment about connect-go's bare-type rule.

Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
@iainmcgin
iainmcgin force-pushed the claude/grpc-stream-response-content-type-upstream branch from c8aec5c to 9b9fbcb Compare August 21, 2026 16:51
@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Thanks for this — the fix is well-placed and the tests are thorough. To get it into 0.9.0 I've rebased the branch onto current main (which now has #257, #262, #236 and #238, all of which touch make_server_stream or the validator's call sites) and pushed one maintainer commit on top rather than round-tripping review comments. Your commit is unchanged apart from the rebase.

The fixup addresses one behavioural question the review turned up. With the validation placed after the trailers-only check, a trailers-only reply carrying a wrong-family content-type and a non-zero grpc-status was believed for its status on the streaming path but rejected as unknown on the unary path (which, since #257, settles HTTP status and content-type before reading grpc-status, as connect-go's grpcValidateResponse does). Same bytes, two answers. Rather than document the divergence, the unary gate is now extracted as check_grpc_response_head and make_server_stream calls it before parse_grpc_error_from_trailers, so the two paths agree by construction. A side effect is that a non-2xx reply that isn't speaking gRPC now carries the content-type detail after HTTP error N on streaming calls too, matching unary.

Also in the fixup: a parity test that drives the same response head through both parsers across the status / content-type / grpc-status matrix; two extra accept-side rows (application/grpc+json exact match, and Connect streaming being unaffected); and a correction to the pre-existing test comment that said the bare type is accepted for every codec "as in connect-go" (connect-go only accepts it for proto — your rewritten function doc already had this right). Changelog fragment extended by a sentence.

Verified locally on the rebased branch: 635 unit tests, client conformance gRPC 1454/0 and gRPC-Web 2838/0.

Connect-protocol streaming responses remain unvalidated here (connect-go does check them); the failure mode is a clean internal at end-of-stream rather than a silent Ok(None), so I'll file that as a follow-up rather than widen this PR.

@iainmcgin
iainmcgin added this pull request to the merge queue Aug 21, 2026
Merged via the queue into connectrpc:main with commit 062ab66 Aug 21, 2026
14 checks passed
iainmcgin added a commit to aknott-ant/connect-rust that referenced this pull request Aug 24, 2026
…alse codegen collision

procedure_uri's no-prefix fast path used PathAndQuery::from_static, which
panics on a byte that is illegal in a path (a space, #, non-ASCII) while
the prefixed path returned internal for the same Spec; both now validate
through from_maybe_shared over Bytes::from_static, still without
allocating. check_method_collisions rejected Get + GetSpec as a clash on
get_spec, but the constants are X_GET_SPEC and X_GET_SPEC_SPEC and
get_spec is only a client method name, so the pair compiles again and
the test pins that.

Spec's type doc said it could be "compared freely"; it now says PartialEq
and Hash include origin, which is why a client-side value is not == the
generated constant, and points at same_method, which takes its argument
by value since Spec is Copy. Spec::client and Spec::server gain # Panics
sections. The per-constant doc codegen emits is cut to one sentence so a
forty-method service does not carry forty copies of a paragraph about
Spec. ClientConfig::new documents how request URIs are formed from the
base, and the changelog snippet gains its imports.

The conformance client used the generated constant regardless of the
service and method the suite named; client_spec() keeps the constant when
they match and builds an ad-hoc client Spec otherwise. One connectrpc#266 test
moves to the Spec-based call signature.

Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
(cherry picked from commit 948178da98a2af553d55b7c9055dcb5b3a484ea1)
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.

client: validate response content-type on streaming gRPC / gRPC-Web calls

3 participants