Repository navigation
fix(curl): forward stdin to curl so -d @- bodies are not dropped - #4085
Conversation
Command::output() gives the child an immediate-EOF stdin, so stdin-based bodies (-d @-, --data-binary @-, -T -, -K -) arrived empty: the server received Content-Length: 0. Inherit rtk's own stdin before capturing stdout/stderr, mirroring RunOptions::inherit_stdin (npm, wc). Fixes rtk-ai#4084
📊 Automated PR Analysis
SummaryFixes rtk curl dropping stdin-based request bodies (e.g. -d @-, --data-binary @-) because Command::output() gave the child process an immediate-EOF stdin. The fix forwards rtk's own stdin via Stdio::inherit() before capturing stdout/stderr, and adds an integration test using an in-process TcpListener to verify the exact body is received. Review Checklist
Analyzed automatically by wshm · This is an automated analysis, not a human review. |
develop now fails the suite on a test file that names the rtk binary instead of building it with common::rtk_command, which keeps rtk's data out of the developer's own directories. With a proxy in the environment, curl sent the request to the proxy and exited 0 on its reply, so the local server never got a connection and the test waited for it forever. --noproxy keeps the request local. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KuSh
left a comment
There was a problem hiding this comment.
Round 1 of 3 — APPROVED
Claim: rtk curl forwards its own stdin to curl, so stdin-sourced request data (-d @-, --data-binary @-, -T -, -K -) reaches the server instead of an empty body.
Scope: accept (round 1, frozen). #4370 was closed as a duplicate of this PR. #4391 rewrites the same file and conflicts only in the use block, so this one lands first.
Ran: echo server × 14 rows, merge-base 5e0f92c vs head vs plain curl (LC_ALL=C). Stdin forms: -d @- piped and heredoc, --data-binary @- with 4 KiB random bytes and with 1 MiB, -T - (chunked), -F f=@-, -K -. Controls: -d @- </dev/null, literal -d, GET with an open, unused stdin (sleep 20 |), GET with stdin closed. Exit codes: connection refused, with and without @-. Invariants: under the hook (rtk rewrite on 10 forms), every decision, exact conditions (no controlling TTY + never-EOF stdin, Claude Code's /dev/null stdin). Savings: 0% on both sides, unchanged: a non-TTY or JSON body passes through by design.
Pins: 1 decision, 3 mutated (Stdio::null(), Stdio::piped(), line removed), 0 survived; curl_forwards_piped_stdin_body kills both that compile (removing the line leaves the import unused, which the crate's warnings = "deny" rejects at compile time).
Matrix: executed this round; pin check: executed this round
CI @92a25c0f8: all green (11/11). The author's 3fd7fc8 was also green, but on a merge commit from 2026-09-15; merged onto today's develop it fails core::test_isolation::tests_that_spawn_rtk_isolate_their_data, which the fixup addresses.
Blocks merge (0)
Filed as follow-ups
- #2431: other commands give the tool a closed stdin.
rtk go run .delivers 0 bytes wherego run .delivers 5; absorbed there, comment added with the transcript (#2431 (comment)).
Checked and correct — no need to re-verify
- On the merge-base, every stdin form arrives empty,
-F f=@-loses its 8-byte part, and-K -fails withcurl: (2) no URL specified. On the head, every row matches plain curl on exit code, body length and sha256, including 1 MiB and binary bodies. </dev/null, literal-d, and a GET whose stdin is an open pipe curl never reads are unchanged and return immediately.rtk proxy curl -d @-already forwarded stdin before this PR (same body on both sides), sortk curlis the only route that dropped it.- With no controlling terminal and a stdin that never closes,
-u userwith no password now waits for the password, as plain curl does. Before, it sent an empty one. Under Claude Code the shell's stdin is/dev/null, so nothing changes there. - Hook reach:
rtk rewriteroutes the bare forms (curl -d @- URL,-T -,-K -,-F f=@-) tortk curland leaves piped, heredoc and< fileforms to plain curl.
Your questions
(none)
Next
Fixup 92a25c0 pushed and code-reviewed. It merges develop (clean) and makes the test spawn rtk through common::rtk_command(), which develop now requires so that a test never writes to the developer's own rtk history (#3758). It also passes --noproxy '*': with a proxy in the environment, curl sent the request there and the test waited forever on its local server. CI is green on 92a25c0, approving. Thanks for the fix and for the test that pins it.
Rounds: 1/3. Threads: 0 open, 0 resolved this round.
Summary
rtk curl -d @-sent an empty request body:Command::output()gives the child an immediate-EOF stdin, so stdin-based bodies (-d @-,--data-binary @-,-T -,-K -) arrived at the server asContent-Length: 0. Fix: forward rtk's own stdin (Stdio::inherit()) before capturing stdout/stderr, mirroringRunOptions::inherit_stdin(npm, wc).TcpListenerand asserts the exact stdin body is received (Red before the fix, Green after).Test plan
cargo fmt --all && cargo clippy --all-targets && cargo testLEN=17 BODY={"via":"heredoc"}),-d @fileand direct-dunaffectedNote: one pre-existing failure on a clean
developcheckout here (git_log_malformed_digit_run_propagates_real_git_errorintests/guard_integration_test.rs, git-version-dependent error message) unrelated to this change.