Skip to content

fix(network): preserve pipelined requests after chunked inspection - #3861

Merged
johntmyers merged 2 commits into
mainfrom
fix/http-chunked-request-boundary
Sep 29, 2026
Merged

johntmyers merged 2 commits into
mainfrom
fix/http-chunked-request-boundary

Conversation

@shiju-nv

Copy link
Copy Markdown
Collaborator

Summary

Prevent inspection of one chunked HTTP request from discarding the next pipelined request. Reads now stop at the current message boundary, leaving the next request for its own policy decision. GraphQL uses the corrected shared reader alongside MCP and generic JSON-RPC, and relay regressions also cover REST's existing streaming behavior.

Related Issue

No issue required: this is a localized correction to existing HTTP body readers.

Changes

  • Read chunk-size and trailer lines through their terminating CRLF, leaving subsequent request bytes in the connection reader. Scan only newly appended line bytes after checking any buffered prefix.
  • Bound payload reads by the remaining chunk length, retaining block reads and existing body/framing checks.
  • Remove GraphQL's duplicate body reader and normalizer in favor of the shared HTTP helper. Preserve GraphQL header validation, configured body limits, query classification, and field authorization; framing errors now use the shared HTTP diagnostics.
  • Cover fragmented framing, extensions, trailers, buffered prefixes, malformed input, and limits. Queue two requests through each persistent relay entry path for REST, GraphQL, MCP, and generic JSON-RPC; verify an allowed second request reaches upstream and a denied second request receives HTTP 403 without being forwarded.
  • Document the message boundary. REST retains chunked streaming; WebSocket frames already use explicit lengths. The forward proxy retains its one-request/connection-close contract.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

Stop chunked MCP and JSON-RPC body reads at each framing boundary so the
connection reader retains the next request for independent inspection.
Keep payload reads bounded by the remaining chunk length and scan framing
lines incrementally.

Cover buffered prefixes, fragmented framing, trailers, and malformed input.
Verify allowed and denied pipelined requests through both relay entry paths.

Signed-off-by: Shiju <shiju@nvidia.com>
Remove GraphQL's duplicate chunk decoder so all buffered HTTP inspectors
preserve the next request on a persistent connection. Keep GraphQL's
header checks, configured body limit and query classification.

Cover GraphQL trailers and fragmented framing, and exercise subsequent
request authorization for REST, GraphQL, MCP and JSON-RPC through both
persistent relay entry paths.

Signed-off-by: Shiju <shiju@nvidia.com>

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

This focused network-proxy correctness fix is project-valid, and the full initial review found no blocking defects. The architecture note documents the corrected HTTP message boundary; required E2E dispatch is the remaining step before pipeline monitoring begins.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None

Non-blocking suggestions:

  • None
Gator metadata
  • Validation: Localized correction to existing HTTP body readers with a clear failure mode and regression coverage
  • Docs: Direct Fern docs are not needed; the internal message-boundary contract is documented in architecture/sandbox.md
  • Checks: Baseline branch, Helm, Trivy, and DCO checks are green; required E2E dispatch is pending
  • E2E: test:e2e required for network proxy behavior and is being dispatched for the current head
  • Head SHA: 560f647ef6f20a4e17fda7d76e2ce4b7ac270b92
  • Base SHA: cfcc3733bd9177f29b3c2aceec9c052b8814422b
  • Merge base SHA: cfcc3733bd9177f29b3c2aceec9c052b8814422b
  • Patch ID: 937b0d24590396bd00bdcdcdb714324f85d11850
  • Gator payload: 9
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Sep 29, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 560f647. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Sep 29, 2026
@johntmyers
johntmyers added this pull request to the merge queue Sep 29, 2026
@johntmyers johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Sep 29, 2026
Merged via the queue into main with commit a6eefcf Sep 29, 2026
167 of 171 checks passed
@johntmyers
johntmyers deleted the fix/http-chunked-request-boundary branch September 29, 2026 17:16
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: Gator review found no blocking defects, the required E2E suite passed, and maintainer approval was present before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants