connectrpc / connectrpc/connect-rust

Health and reflection error messages grow with the peer-supplied name

Open
#288 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
509
Forks
66
Avg merge
2d 8h
Merged PRs (30d)
10

Description

Since #272, the bundled health and reflection routes admit request messages up to MAX_REQUEST_BYTES (16 KiB). Two things in the services themselves still scale with that input rather than with a constant.

Error messages echo the whole peer-supplied name. StaticChecker answers an unregistered service with not_found("unknown service {service}"), so a 16 KiB name comes back as a 16 KiB grpc-message header (percent-encoded, up to 3× that for non-ASCII) or Connect error body. Reflection's lookup misses do the same in the in-band ErrorResponse, Debug-escaped (symbol {symbol:?} not found, up to ~5× the name for control characters), on top of the protocol's own original_request echo which already carries the request once. A miss is the common case for a probe with a typo and the only case a caller with no knowledge of the schema can force, so the response should not grow with the name.

request_limits() leaves the decode budget at the 32 MiB default. Neither HealthCheckRequest nor ServerReflectionRequest has a repeated or map field, so nothing legitimate charges buffa's element-memory budget on these routes; the profile should say so with a small bound rather than inherit the general-purpose one.

Proposed: echo at most a fixed prefix of the name (with the original length noted) in both crates, and set the element-memory limit in both request_limits() profiles to a small multiple of MAX_REQUEST_BYTES. Custom Checker implementations remain responsible for their own error text; the Checker docs should say so.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing StaticChecker's unknown-service response and the reflection lookup miss, then inspect request_limits() for HealthCheckRequest and ServerReflectionRequest in both crates. Done means both error formats cap the echoed name while noting its original length, both profiles use a small element-memory bound, and the Checker documentation states that custom implementations own their error text.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.