Skip to content

fix(sandbox): deliver a queued HTTP rejection when the upload fails - #5339

Closed
rafamolina1 wants to merge 1 commit into
NVIDIA:mainfrom
rafamolina1:fix/queued-http-rejection-on-upload
Closed

rafamolina1 wants to merge 1 commit into
NVIDIA:mainfrom
rafamolina1:fix/queued-http-rejection-on-upload

Conversation

@rafamolina1

Copy link
Copy Markdown

Summary

An oversized upload that the middleware rejects still left curl with an empty reply. The proxy had already written the HTTP rejection, but copy_bidirectional returned the upload's BrokenPipe and dropped that queued response. The request stayed blocked; only the response was lost.

Related Issue

Closes #5132

Changes

  • Replace the RelayReady relay in complete_network_open with a same-task copy of both directions.
  • Keep half-close on a clean EOF, with no deadline on a live connection.
  • After an upload failure, stop reading the body and allow one second for a response already in the reverse buffer to flush.
  • Stop immediately on a response-direction failure and do not drain the unread upload.
  • Cover the real complete_network_open path: queued rejection with an unread body, traffic in both directions, half-close, an idle pair, and a response error.

Release Notes

A sandbox upload rejected before it is forwarded now receives the HTTP rejection the supervisor already prepared, instead of an empty reply.

Testing

  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

cargo fmt --all -- --check, the SPDX header check on the touched files, and cargo clippy --locked -p openshell-sandbox-backend --all-targets -- -D warnings pass. cargo test -p openshell-sandbox-backend network_relay passes all five relay tests. The Nomos curl lab from the issue was not run.

Checklist

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

copy_bidirectional returns the upload's BrokenPipe and abandons the
reverse copy, so a middleware rejection the proxy has already written
never reaches the sandbox. Curl then reports an empty reply even though
the request stayed blocked.

Copy the two directions in the same task. A clean EOF still half-closes
that direction. An upload failure stops reading the body and allows one
second for the queued response to flush. A response failure returns
immediately and does not drain the unread upload.

Signed-off-by: Rafael Molina <128931481+rafamolina1@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @rafamolina1.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions github-actions Bot closed this Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@rafamolina1

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

1 similar comment
@rafamolina1

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

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.

Preserve the HTTP rejection response during an oversized upload

1 participant