mirror of
https://github.com/0xrsydn/idx-cli.git
synced 2026-11-07 01:35:36 +00:00
fix(net): kill curl itself, not only its profile script, on overflow
Run curl in its own process group and kill the group, so the curl behind a curl_* wrapper script ends at once instead of at its next write or --max-time (Pullfrog review on #55). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh
This commit is contained in:
parent
6b0ef687d0
commit
a360d0b429
3 changed files with 54 additions and 10 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -908,6 +908,7 @@ dependencies = [
|
||||||
"clap",
|
"clap",
|
||||||
"directories",
|
"directories",
|
||||||
"fastrand",
|
"fastrand",
|
||||||
|
"libc",
|
||||||
"quick-xml",
|
"quick-xml",
|
||||||
"rusqlite",
|
"rusqlite",
|
||||||
"serde",
|
"serde",
|
||||||
|
|
|
||||||
|
|
@ -34,3 +34,6 @@ rusqlite = { workspace = true, optional = true }
|
||||||
quick-xml = { version = "0.37", optional = true }
|
quick-xml = { version = "0.37", optional = true }
|
||||||
sha2 = { workspace = true, optional = true }
|
sha2 = { workspace = true, optional = true }
|
||||||
zip = { workspace = true, optional = true }
|
zip = { workspace = true, optional = true }
|
||||||
|
|
||||||
|
[target.'cfg(unix)'.dependencies]
|
||||||
|
libc = "0.2"
|
||||||
|
|
|
||||||
|
|
@ -118,12 +118,25 @@ fn run_binary_limited(
|
||||||
limit: usize,
|
limit: usize,
|
||||||
) -> Result<Output, IdxError> {
|
) -> Result<Output, IdxError> {
|
||||||
let max_filesize = limit.to_string();
|
let max_filesize = limit.to_string();
|
||||||
let mut child = Command::new(binary)
|
let mut command = Command::new(binary);
|
||||||
.args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()])
|
command
|
||||||
|
.args([
|
||||||
|
"--max-time",
|
||||||
|
MAX_TIME_SECS,
|
||||||
|
"--max-filesize",
|
||||||
|
max_filesize.as_str(),
|
||||||
|
])
|
||||||
.args(args)
|
.args(args)
|
||||||
.stdin(Stdio::null())
|
.stdin(Stdio::null())
|
||||||
.stdout(Stdio::piped())
|
.stdout(Stdio::piped())
|
||||||
.stderr(Stdio::piped())
|
.stderr(Stdio::piped());
|
||||||
|
// The curl_* profiles are scripts that run curl without exec; a process
|
||||||
|
// group of its own lets an overflow kill curl too, not just the script.
|
||||||
|
// Ctrl+C then reaches only idx: curl ends on its next write to the closed
|
||||||
|
// pipe, or at --max-time.
|
||||||
|
#[cfg(unix)]
|
||||||
|
std::os::unix::process::CommandExt::process_group(&mut command, 0);
|
||||||
|
let mut child = command
|
||||||
.spawn()
|
.spawn()
|
||||||
.map_err(|e| {
|
.map_err(|e| {
|
||||||
if e.kind() == std::io::ErrorKind::NotFound {
|
if e.kind() == std::io::ErrorKind::NotFound {
|
||||||
|
|
@ -147,14 +160,10 @@ fn run_binary_limited(
|
||||||
.take(limit as u64 + 1)
|
.take(limit as u64 + 1)
|
||||||
.read_to_end(&mut stdout);
|
.read_to_end(&mut stdout);
|
||||||
if read.is_err() || stdout.len() > limit {
|
if read.is_err() || stdout.len() > limit {
|
||||||
// The curl_* profiles are scripts that run curl without exec, so the
|
|
||||||
// kill reaches only the wrapper; curl itself still holds stderr. Do
|
|
||||||
// not wait for it: closing stdout ends it on its next write (or
|
|
||||||
// --max-time ends a stalled one), and the stderr thread exits with it.
|
|
||||||
drop(stdout_pipe);
|
drop(stdout_pipe);
|
||||||
let _ = child.kill();
|
kill_process_group(&mut child);
|
||||||
let _ = child.wait();
|
let _ = child.wait();
|
||||||
drop(stderr_reader);
|
let _ = stderr_reader.join();
|
||||||
return Err(match read {
|
return Err(match read {
|
||||||
Err(e) => IdxError::Http(format!(
|
Err(e) => IdxError::Http(format!(
|
||||||
"failed to read {stage} response from {binary}: {e}"
|
"failed to read {stage} response from {binary}: {e}"
|
||||||
|
|
@ -189,6 +198,18 @@ fn run_binary_limited(
|
||||||
)))
|
)))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Kills curl and the profile script around it (their group, on Unix).
|
||||||
|
fn kill_process_group(child: &mut std::process::Child) {
|
||||||
|
#[cfg(unix)]
|
||||||
|
if let Ok(pgid) = libc::pid_t::try_from(child.id()) {
|
||||||
|
// SAFETY: kill(2) takes plain integers; the group is the one this
|
||||||
|
// child leads (process_group(0) at spawn), not yet reaped.
|
||||||
|
unsafe { libc::kill(-pgid, libc::SIGKILL) };
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let _ = child.kill();
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(all(test, unix))]
|
#[cfg(all(test, unix))]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
@ -200,9 +221,13 @@ mod tests {
|
||||||
let dir = std::env::temp_dir().join(format!("idx-curl-{}", fastrand::u64(..)));
|
let dir = std::env::temp_dir().join(format!("idx-curl-{}", fastrand::u64(..)));
|
||||||
std::fs::create_dir_all(&dir).unwrap();
|
std::fs::create_dir_all(&dir).unwrap();
|
||||||
let fake = dir.join("curl");
|
let fake = dir.join("curl");
|
||||||
|
let pid_file = dir.join("curl.pid");
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
&fake,
|
&fake,
|
||||||
"#!/bin/sh\n( head -c 8192 /dev/zero; sleep 30 )\ntrue\n",
|
format!(
|
||||||
|
"#!/bin/sh\nsh -c 'echo $$ > {}; head -c 8192 /dev/zero; exec sleep 30'\ntrue\n",
|
||||||
|
pid_file.display()
|
||||||
|
),
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let mut perms = std::fs::metadata(&fake).unwrap().permissions();
|
let mut perms = std::fs::metadata(&fake).unwrap().permissions();
|
||||||
|
|
@ -214,6 +239,21 @@ mod tests {
|
||||||
|
|
||||||
assert!(err.to_string().contains("exceeds 4096 bytes"), "{err}");
|
assert!(err.to_string().contains("exceeds 4096 bytes"), "{err}");
|
||||||
assert!(started.elapsed() < std::time::Duration::from_secs(10));
|
assert!(started.elapsed() < std::time::Duration::from_secs(10));
|
||||||
|
// The stand-in curl behind the wrapper is gone too, not left to its
|
||||||
|
// timeout (allow a moment for init to reap it).
|
||||||
|
let pid: libc::pid_t = std::fs::read_to_string(&pid_file)
|
||||||
|
.unwrap()
|
||||||
|
.trim()
|
||||||
|
.parse()
|
||||||
|
.unwrap();
|
||||||
|
let alive = || unsafe { libc::kill(pid, 0) } == 0;
|
||||||
|
for _ in 0..50 {
|
||||||
|
if !alive() {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
std::thread::sleep(std::time::Duration::from_millis(100));
|
||||||
|
}
|
||||||
|
assert!(!alive(), "curl {pid} outlived the overflow");
|
||||||
std::fs::remove_dir_all(&dir).unwrap();
|
std::fs::remove_dir_all(&dir).unwrap();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue