From b2b511e9c2021476994798d74e4de06b2e71e8e1 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:21:37 +0000 Subject: [PATCH] fix(net): cap curl output while reading it, not after Command::output() buffered the whole response before the size check, and curl before 8.20 does not apply --max-filesize to decoded bodies, so a small compressed response could exhaust memory. Read stdout up to the limit, then kill and reap curl (Pullfrog review on #55). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/curl_impersonate.rs | 85 ++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index 0b1d69b..c8cdaa2 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -1,4 +1,5 @@ -use std::process::{Command, Output}; +use std::io::Read; +use std::process::{Command, Output, Stdio}; use crate::error::IdxError; @@ -104,11 +105,26 @@ const MAX_TIME_SECS: &str = "600"; const MAX_RESPONSE_BYTES: usize = 256 * 1024 * 1024; fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result { - let max_filesize = MAX_RESPONSE_BYTES.to_string(); - let output = Command::new(binary) + run_binary_limited(stage, binary, args, MAX_RESPONSE_BYTES) +} + +/// `--max-filesize` cannot stop a response without a length, nor (before +/// curl 8.20) a small compressed one that decodes to more, so stdout is +/// also capped while it is read: past `limit`, curl is killed and reaped. +fn run_binary_limited( + stage: &str, + binary: &str, + args: &[String], + limit: usize, +) -> Result { + let max_filesize = limit.to_string(); + let mut child = Command::new(binary) .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) .args(args) - .output() + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { return IdxError::Http(format!( @@ -118,12 +134,41 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result MAX_RESPONSE_BYTES { - return Err(IdxError::Http(format!( - "{stage} response exceeds {MAX_RESPONSE_BYTES} bytes" - ))); + // stderr drains on its own thread so a chatty curl cannot block on it. + let mut stderr_pipe = child.stderr.take().expect("stderr is piped"); + let stderr_reader = std::thread::spawn(move || { + let mut stderr = Vec::new(); + let _ = stderr_pipe.read_to_end(&mut stderr); + stderr + }); + let mut stdout = Vec::new(); + let read = child + .stdout + .take() + .expect("stdout is piped") + .take(limit as u64 + 1) + .read_to_end(&mut stdout); + if read.is_err() || stdout.len() > limit { + let _ = child.kill(); + let _ = child.wait(); + let _ = stderr_reader.join(); + return Err(match read { + Err(e) => IdxError::Http(format!( + "failed to read {stage} response from {binary}: {e}" + )), + Ok(_) => IdxError::Http(format!("{stage} response exceeds {limit} bytes")), + }); } + let status = child + .wait() + .map_err(|e| IdxError::Http(format!("failed to wait for {binary} for {stage}: {e}")))?; + let stderr = stderr_reader.join().unwrap_or_default(); + let output = Output { + status, + stdout, + stderr, + }; + if output.status.success() { return Ok(output); } @@ -140,3 +185,25 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result