fix(tools): resume partial downloads with curl -C - (#2029)

Fixes #2028. A download that died mid-stream kept its partial file but started over from zero on retry. Downloads now resume with curl -C - when the partial belongs to the same URL (a .url sidecar records it; an untracked file or a different URL starts from zero, so a stale partial can't be spliced in); a server without byte ranges (curl exit 33) or HTTP 416 drops the partial and restarts. Covered by tests against a local HTTP server with and without Range support.

Co-authored-by: a-rookie-of-C-language <[email protected]>
Цей коміт міститься в:
arookieofcтаa-rookie-of-C-language authored and GitHub committed 2026-11-08 01:20:53 +08:00
1 джерело 053a2c37d8
коміт 7479b7ad9d
2 files changed
+286 -13

No files matched your search

+109 -13
Переглянути файл
@@ -430,6 +430,9 @@ impl Http {
}
/// Downloads `url` to `dest`, reporting progress against `size` when known.
/// A `.url` sidecar ties partial bytes to their source. Matching partials
/// resume with `-C -`; untracked files or a changed URL start from zero.
/// Rejected ranges (curl exit 33 or HTTP 416) also restart from zero.
pub fn download(
&self,
url: &str,
@@ -438,8 +441,67 @@ impl Http {
cancel: &CancellationToken,
emit: Emit,
) -> IdeResult<()> {
if cancel.is_cancelled() {
return Err(IdeError::cancelled());
}
let mut url_path = dest.as_os_str().to_owned();
url_path.push(".url");
let url_path = std::path::PathBuf::from(url_path);
if !std::fs::read_to_string(&url_path).is_ok_and(|previous| previous == url) {
match std::fs::remove_file(dest) {
Ok(()) => {}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => return Err(e.into()),
}
}
std::fs::write(&url_path, url)?;
let had_partial = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0) > 0;
let mut resume = true;
loop {
let attempt = self.fetch(url, dest, size, cancel, emit, resume);
if cancel.is_cancelled() {
let _ = std::fs::remove_file(&url_path);
}
let (status, http_code, stderr, token) = attempt?;
// Some curl versions return success for 416 even for oversized partials.
if resume && had_partial && (status.code() == Some(33) || http_code == 416) {
resume = false;
std::fs::remove_file(dest)?;
continue;
}
if status.success() && http_code != 416 {
break;
}
return Err(IdeError::new(
ErrorKind::Io,
format!("download of {url} failed: {}", redact_auth(stderr.trim(), token.as_deref())),
));
}
std::fs::remove_file(url_path)?;
let bytes = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0);
emit(progress_event(bytes, size.or(Some(bytes))));
Ok(())
}
/// One curl download attempt: with `resume` it continues `dest`'s bytes,
/// without it starts over. Returns curl's exit status, HTTP code, stderr,
/// and auth token for redacting failures; spawn and cancellation failures
/// are errors.
fn fetch(
&self,
url: &str,
dest: &Path,
size: Option<u64>,
cancel: &CancellationToken,
emit: Emit,
resume: bool,
) -> IdeResult<(std::process::ExitStatus, u16, String, Option<String>)> {
let (mut cmd, token) = self.base(url, cancel)?;
cmd.arg("-o").arg(dest).arg(url).stdout(Stdio::null()).stderr(Stdio::piped());
if resume {
cmd.args(["-C", "-"]);
}
cmd.args(["--write-out", "%{http_code}"]).arg("-o").arg(dest).arg(url);
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
let mut child = cmd.spawn().map_err(|e| IdeError::unavailable(format!("cannot run curl: {e}")))?;
send_auth(&mut child, token.as_deref())?;
let mut stderr = child.stderr.take().expect("piped");
@@ -469,18 +531,9 @@ impl Http {
}
}
};
if !status.success() {
return Err(IdeError::new(
ErrorKind::Io,
format!(
"download of {url} failed: {}",
redact_auth(err_t.join().unwrap_or_default().trim(), token.as_deref())
),
));
}
let bytes = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0);
emit(progress_event(bytes, size.or(Some(bytes))));
Ok(())
let mut http_code = String::new();
child.stdout.take().expect("piped").read_to_string(&mut http_code)?;
Ok((status, http_code.trim().parse().unwrap_or(0), err_t.join().unwrap_or_default(), token))
}
}
@@ -578,6 +631,9 @@ pub(crate) fn run_input(
String::from_utf8(output).map_err(|e| IdeError::invalid(e.to_string()))
}
#[cfg(test)]
mod download_tests;
#[cfg(test)]
mod tests {
use super::*;
@@ -900,6 +956,46 @@ mod github_auth_tests {
assert!(!error.contains("gho_fake_first"));
}
#[test]
fn download_resumes_the_partial_file() {
let f = Fixture::new();
let dest = f.path("download");
fs::write(&dest, "partial").unwrap();
fs::write(f.path("download.url"), "https://example.test/file").unwrap();
f.curl(&format!("printf done > {}", quote(&dest)));
f.http.download("https://example.test/file", &dest, None, &CancellationToken::new(), &|_| {}).unwrap();
assert!(
fs::read_to_string(f.path("curl-args")).unwrap().contains("-C\n-\n"),
"curl resumes the partial file with `-C -`"
);
}
#[test]
fn download_restarts_when_the_server_has_no_byte_ranges() {
let f = Fixture::new();
let dest = f.path("download");
let count = f.path("attempts");
fs::write(&dest, "partial").unwrap();
fs::write(f.path("download.url"), "https://example.test/file").unwrap();
// First attempt: curl reports it cannot resume (exit 33). Second:
// a plain download from zero, which succeeds.
f.curl(&format!(
"n=$(cat {} 2>/dev/null || echo 0); echo $((n + 1)) > {}; \
if [ \"$n\" = 0 ]; then echo 'curl: (33) no byte ranges' >&2; exit 33; fi; \
printf done > {}",
quote(&count),
quote(&count),
quote(&dest)
));
f.http.download("https://example.test/file", &dest, None, &CancellationToken::new(), &|_| {}).unwrap();
assert_eq!(fs::read_to_string(&dest).unwrap(), "done");
assert_eq!(fs::read_to_string(&count).unwrap().trim(), "2");
assert!(
!fs::read_to_string(f.path("curl-args")).unwrap().contains("-C\n-\n"),
"the restart downloads from zero, without `-C -`"
);
}
#[test]
fn credential_lookup_is_bounded_and_cancellable_even_with_inherited_stdout() {
let f = Fixture::new();
+177
Переглянути файл
@@ -0,0 +1,177 @@
use super::*;
use std::fs;
use std::net::TcpListener;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex};
const CONTENT: &[u8] = b"a complete archive with enough bytes to interrupt and resume";
const PARTIAL: usize = 13;
struct Server {
url: String,
ranges: Arc<Mutex<Vec<Option<usize>>>>,
stopped: Arc<AtomicBool>,
thread: Option<std::thread::JoinHandle<()>>,
}
impl Server {
fn new(supports_range: bool, interrupt_first: bool) -> Self {
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
listener.set_nonblocking(true).unwrap();
let url = format!("http://{}/archive", listener.local_addr().unwrap());
let ranges = Arc::new(Mutex::new(Vec::new()));
let stopped = Arc::new(AtomicBool::new(false));
let requests = ranges.clone();
let stop = stopped.clone();
let thread = std::thread::spawn(move || {
while !stop.load(Ordering::Relaxed) {
let (mut stream, _) = match listener.accept() {
Ok(connection) => connection,
Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => {
std::thread::sleep(Duration::from_millis(5));
continue;
}
Err(e) => panic!("accept: {e}"),
};
stream.set_nonblocking(false).unwrap();
stream.set_read_timeout(Some(Duration::from_secs(2))).unwrap();
stream.set_write_timeout(Some(Duration::from_secs(2))).unwrap();
let mut request = String::new();
let mut reader = BufReader::new(&stream);
loop {
let mut line = String::new();
if reader.read_line(&mut line).unwrap() == 0 || line == "\r\n" {
break;
}
request.push_str(&line);
}
let range = request.lines().find_map(|line| {
line.to_ascii_lowercase().strip_prefix("range: bytes=")?.strip_suffix('-')?.parse::<usize>().ok()
});
let mut requests = requests.lock().unwrap();
let interrupted = interrupt_first && requests.is_empty();
requests.push(range);
drop(requests);
let (status, content_range, body) = match range.filter(|_| supports_range) {
Some(start) if start >= CONTENT.len() => {
("416 Range Not Satisfiable", format!("Content-Range: bytes */{}\r\n", CONTENT.len()), &b""[..])
}
Some(start) => (
"206 Partial Content",
format!("Content-Range: bytes {start}-{}/{}\r\n", CONTENT.len() - 1, CONTENT.len()),
&CONTENT[start..],
),
None => ("200 OK", String::new(), CONTENT),
};
let header = format!(
"HTTP/1.1 {status}\r\n{content_range}Content-Length: {}\r\nConnection: close\r\n\r\n",
body.len()
);
let _ = stream.write_all(header.as_bytes());
let _ = stream.write_all(if interrupted { &body[..PARTIAL] } else { body });
}
});
Self { url, ranges, stopped, thread: Some(thread) }
}
fn ranges(&self) -> Vec<Option<usize>> {
self.ranges.lock().unwrap().clone()
}
}
impl Drop for Server {
fn drop(&mut self) {
self.stopped.store(true, Ordering::Relaxed);
let result = self.thread.take().unwrap().join();
if !std::thread::panicking() {
result.unwrap();
}
}
}
fn http() -> Option<Http> {
let Ok(curl) = which::which("curl") else {
eprintln!("skipping download test: curl is not installed");
return None;
};
Some(Http { curl, gh: None, explicit_github_token: None, https_only: false, revoke_best_effort: false })
}
#[test]
fn interrupted_download_resumes_with_range() {
let Some(http) = http() else { return };
let server = Server::new(true, true);
let dir = tempfile::tempdir().unwrap();
let dest = dir.path().join("archive.tar.gz");
let cancel = CancellationToken::new();
assert!(http.download(&server.url, &dest, None, &cancel, &|_| {}).is_err());
assert_eq!(fs::read(&dest).unwrap(), &CONTENT[..PARTIAL]);
assert_eq!(fs::read_to_string(dir.path().join("archive.tar.gz.url")).unwrap(), server.url);
http.download(&server.url, &dest, None, &cancel, &|_| {}).unwrap();
assert_eq!(fs::read(&dest).unwrap(), CONTENT);
assert_eq!(server.ranges(), vec![None, Some(PARTIAL)]);
assert!(!dir.path().join("archive.tar.gz.url").exists());
}
#[test]
fn interrupted_download_restarts_without_range_support() {
let Some(http) = http() else { return };
let server = Server::new(false, true);
let dir = tempfile::tempdir().unwrap();
let dest = dir.path().join("archive");
let cancel = CancellationToken::new();
assert!(http.download(&server.url, &dest, None, &cancel, &|_| {}).is_err());
http.download(&server.url, &dest, None, &cancel, &|_| {}).unwrap();
assert_eq!(fs::read(&dest).unwrap(), CONTENT);
assert_eq!(server.ranges(), vec![None, Some(PARTIAL), None]);
assert!(!dir.path().join("archive.url").exists());
}
#[test]
fn different_or_unknown_url_discards_stale_bytes_including_same_size_files() {
let Some(http) = http() else { return };
for previous_url in [Some("https://example.test/older-version"), None] {
for length in [PARTIAL, CONTENT.len()] {
let server = Server::new(true, false);
let dir = tempfile::tempdir().unwrap();
let dest = dir.path().join("archive");
fs::write(&dest, vec![b'x'; length]).unwrap();
if let Some(url) = previous_url {
fs::write(dir.path().join("archive.url"), url).unwrap();
}
http.download(&server.url, &dest, None, &CancellationToken::new(), &|_| {}).unwrap();
assert_eq!(fs::read(&dest).unwrap(), CONTENT);
assert_eq!(server.ranges(), vec![None]);
assert!(!dir.path().join("archive.url").exists());
}
}
}
#[test]
fn oversized_partial_restarts_after_http_416() {
let Some(http) = http() else { return };
let server = Server::new(true, false);
let dir = tempfile::tempdir().unwrap();
let dest = dir.path().join("archive");
let length = CONTENT.len() + 10;
fs::write(&dest, vec![b'x'; length]).unwrap();
fs::write(dir.path().join("archive.url"), &server.url).unwrap();
http.download(&server.url, &dest, None, &CancellationToken::new(), &|_| {}).unwrap();
assert_eq!(fs::read(&dest).unwrap(), CONTENT);
assert_eq!(server.ranges(), vec![Some(length), None]);
assert!(!dir.path().join("archive.url").exists());
}
#[test]
fn complete_file_for_same_url_succeeds() {
let Some(http) = http() else { return };
let server = Server::new(true, false);
let dir = tempfile::tempdir().unwrap();
let dest = dir.path().join("archive");
fs::write(&dest, CONTENT).unwrap();
fs::write(dir.path().join("archive.url"), &server.url).unwrap();
http.download(&server.url, &dest, None, &CancellationToken::new(), &|_| {}).unwrap();
assert_eq!(fs::read(&dest).unwrap(), CONTENT);
assert_eq!(server.ranges()[0], Some(CONTENT.len()));
assert!(!dir.path().join("archive.url").exists());
}