Skip to content

refactor(network): unify buffered HTTP request handling #2284

Description

@pimlock

Problem Statement

OpenShell currently buffers and normalizes HTTP/1.1 request bodies in several places:

  • REST middleware and body credential rewriting in crates/openshell-supervisor-network/src/l7/rest.rs
  • JSON-RPC and MCP inspection through crates/openshell-supervisor-network/src/l7/http.rs
  • GraphQL inspection in crates/openshell-supervisor-network/src/l7/graphql.rs
  • inference request parsing in crates/openshell-supervisor-network/src/l7/inference.rs

These paths independently parse chunked framing, enforce limits, consume trailers, and rebuild headers. Their behavior has drifted. PR #2027 exposed one example: REST buffering discarded actual trailers but left the Trailer declaration on the normalized request, while JSON-RPC, GraphQL, and inference removed it.

Independent implementations also make it harder to verify consistent handling of malformed framing, chunk extensions, trailers, byte limits, and pipelined request boundaries.

Transparent request relay is a different case: it intentionally preserves chunked wire framing and trailers without buffering or transforming the body. That path should remain separate.

Proposed Design

Introduce a shared buffered-request framing and normalization core with caller-specific adapters:

  1. Define a common chunked decoder/state machine that produces a structured result containing the decoded body, consumed wire bytes, trailers as separate fields, and original framing metadata.
  2. Centralize framing validation and configurable limits, including decoded bytes, wire bytes, chunk count, and trailer line/byte limits.
  3. Centralize header normalization: remove Transfer-Encoding, Trailer, and the old Content-Length; set the decoded Content-Length; and make the trailer policy explicit (drop, reject, or a future field-aware retention mode).
  4. Provide an incremental async adapter for REST, middleware, credential rewriting, JSON-RPC, and GraphQL. Keep caller-specific behavior such as Expect: 100-continue, policy-generation guards, and fail-open recovery outside the shared decoder.
  5. Provide a slice-based adapter for inference, which parses already-buffered bytes and must return the exact number of bytes consumed for keep-alive requests.
  6. Move GraphQL onto the shared HTTP inspection helper where its requirements already match JSON-RPC/MCP.
  7. Add a shared conformance matrix covering content-length bodies, chunk extensions, trailers, malformed or ambiguous framing, limits, incomplete input, pipelined boundaries, and exact consumed-byte accounting.
  8. Document that buffered transformation paths currently discard trailers. Preserve the raw relay_chunked path for requests that do not require inspection or body rewriting.

The goal is one framing/parser contract and one normalization contract, not necessarily one monolithic I/O function.

Alternatives Considered

Use one async buffering function everywhere

Inference parses an existing byte slice and needs exact consumed-byte accounting, while the other paths read incrementally from a stream. REST also has policy-generation checks and fail-open recovery concerns. Forcing all callers through one async API would add coupling without removing the important differences.

Keep the implementations separate and add tests

Tests help, but duplicated parsing and header normalization can still drift as limits and protocol behavior evolve. A shared conformance suite alone would not eliminate that maintenance and security risk.

Preserve all trailers after buffering

Buffered paths may transform the body or regenerate the upstream request. HTTP field semantics determine whether a trailer can safely become a header, so universal preservation or merging is unsafe. Keeping trailers separate in the shared result allows an explicit future policy without silently changing semantics.

Agent Investigation

Reviewed the request flows associated with PR #2027 and traced chunked bodies through REST credential injection, supervisor middleware, JSON-RPC/MCP, GraphQL, and inference routing. REST and JSON-RPC use incremental readers with different surrounding control flow; GraphQL duplicates much of the JSON-RPC buffering behavior; inference uses a synchronous slice parser for keep-alive framing. Searched existing OpenShell issues and found no issue directly tracking consolidation of buffered HTTP request handling.

  • I reviewed existing issues and architecture documentation before proposing this design.
  • This issue includes a concrete design proposal and alternatives.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions