fix(bootstrap): resume downloads without discarding a valid archive
Resuming a partial download leaned entirely on the remote size from a HEAD request. When that size could not be determined the code played it safe by deleting the partial file and starting over, so a flaky HEAD (or an offline mirror) threw away gigabytes that were fine to resume. It also could not tell a real size from an error page, and if the server answered a ranged request with the whole file (200, not 206) libcurl could append it to the partial and silently corrupt the archive. - The HEAD now returns transport status, HTTP code, and size together, and a size counts as authoritative only on a 2xx reply with a positive Content-Length. - A pure helper decides what to do from the existing bytes and that authoritative flag: start fresh, truncate a stale oversized file, reuse a complete one, or resume with a ranged GET. When the size is not authoritative the partial is kept and a ranged GET is attempted rather than discarded. - A ranged request that comes back as 200 instead of 206 now restarts cleanly from scratch instead of appending to the partial. - The free-space pre-check runs only when the size is authoritative, and a failed download reports how many bytes were kept for the retry. Tests cover the resume-decision helper across the fresh, stale, complete, resuming, and non-authoritative cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+62
-37
@@ -255,7 +255,9 @@ void Bootstrap::runDownloadAndInstall(const std::string& dataDir, const std::str
|
||||
formatSize((double)sz) + ") to resume"
|
||||
: std::string("Could not write the download to disk (disk full?)");
|
||||
else
|
||||
why = "Download failed — check your internet connection";
|
||||
why = (sz > 0 && sz != static_cast<std::uintmax_t>(-1))
|
||||
? "Download failed — " + formatSize((double)sz) + " kept, will resume on retry"
|
||||
: std::string("Download failed — check your internet connection");
|
||||
setProgress(State::Failed, why); // last: after the partial-file cleanup
|
||||
return;
|
||||
}
|
||||
@@ -443,9 +445,10 @@ void Bootstrap::setProgress(State state, const std::string& text, double downloa
|
||||
// HEAD request for remote file size
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
long long Bootstrap::getRemoteFileSize(const std::string& url) {
|
||||
Bootstrap::RemoteInfo Bootstrap::getRemoteFileSize(const std::string& url) {
|
||||
RemoteInfo info;
|
||||
CURL* curl = curl_easy_init();
|
||||
if (!curl) return -1;
|
||||
if (!curl) return info;
|
||||
|
||||
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
|
||||
curl_easy_setopt(curl, CURLOPT_NOBODY, 1L); // HEAD request
|
||||
@@ -455,6 +458,7 @@ long long Bootstrap::getRemoteFileSize(const std::string& url) {
|
||||
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
|
||||
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L);
|
||||
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L);
|
||||
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L); // timeouts are safe on a worker thread
|
||||
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
|
||||
curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, abortOnCancelCallback);
|
||||
curl_easy_setopt(curl, CURLOPT_XFERINFODATA, &cancel_requested_);
|
||||
@@ -463,15 +467,26 @@ long long Bootstrap::getRemoteFileSize(const std::string& url) {
|
||||
if (res != CURLE_OK) {
|
||||
DEBUG_LOGF("[Bootstrap] HEAD request failed: %s\n", curl_easy_strerror(res));
|
||||
curl_easy_cleanup(curl);
|
||||
return -1;
|
||||
return info; // transportOk=false, size=-1
|
||||
}
|
||||
|
||||
info.transportOk = true;
|
||||
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &info.httpCode);
|
||||
curl_off_t cl = -1;
|
||||
curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, &cl);
|
||||
info.size = (long long)cl;
|
||||
curl_easy_cleanup(curl);
|
||||
|
||||
DEBUG_LOGF("[Bootstrap] Remote file size: %lld bytes\n", (long long)cl);
|
||||
return (long long)cl;
|
||||
DEBUG_LOGF("[Bootstrap] HEAD %ld, size %lld\n", info.httpCode, info.size);
|
||||
return info;
|
||||
}
|
||||
|
||||
Bootstrap::ResumeAction Bootstrap::decideResume(long long existingBytes, bool authoritative,
|
||||
long long remoteSize) {
|
||||
if (existingBytes <= 0) return ResumeAction::Fresh;
|
||||
if (!authoritative) return ResumeAction::RangedGet; // keep the file; a non-206 reply is caught after perform
|
||||
if (existingBytes > remoteSize) return ResumeAction::TruncateAndFresh; // stale: older/larger zip
|
||||
if (existingBytes == remoteSize) return ResumeAction::ReuseComplete; // complete: skip the GET; verifyChecksums runs
|
||||
return ResumeAction::RangedGet; // existing < remote
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -518,45 +533,43 @@ bool Bootstrap::download(const std::string& url, const std::string& destZip) {
|
||||
|
||||
// The remote size (HEAD) detects server-side changes that would corrupt a resumed download, lets a
|
||||
// complete earlier download be reused, and feeds the early free-space check below.
|
||||
const long long remoteSize = getRemoteFileSize(url);
|
||||
if (cancel_requested_) { // the HEAD was cut short: keep a partial file for the next resume
|
||||
const RemoteInfo info = getRemoteFileSize(url);
|
||||
if (cancel_requested_) { // the HEAD was cut short: keep a partial file for the next resume
|
||||
curl_easy_cleanup(curl);
|
||||
return false;
|
||||
}
|
||||
if (existing_bytes > 0) {
|
||||
if (remoteSize > 0) {
|
||||
if (existing_bytes == remoteSize) {
|
||||
// Complete from an earlier attempt (e.g. "free some space, then Retry"): reuse it instead of
|
||||
// re-downloading several GB. verifyChecksums() still runs on it and deletes it on a mismatch.
|
||||
DEBUG_LOGF("[Bootstrap] Local file is complete (%lld bytes) - reusing it\n", existing_bytes);
|
||||
curl_easy_cleanup(curl);
|
||||
resume_offset_ = 0;
|
||||
setProgress(State::Downloading,
|
||||
"Reusing the completed download (" + formatSize((double)existing_bytes) + ")",
|
||||
(double)existing_bytes, (double)remoteSize);
|
||||
return true;
|
||||
}
|
||||
if (existing_bytes > remoteSize) {
|
||||
// Bigger than the remote file — stale from an older zip
|
||||
DEBUG_LOGF("[Bootstrap] Local file (%lld bytes) > remote (%lld bytes), re-downloading\n",
|
||||
existing_bytes, remoteSize);
|
||||
std::error_code ec;
|
||||
fs::remove(destZip, ec);
|
||||
existing_bytes = 0;
|
||||
}
|
||||
} else {
|
||||
// Could not determine remote size — play it safe and re-download
|
||||
DEBUG_LOGF("[Bootstrap] Could not determine remote file size, re-downloading to be safe\n");
|
||||
const bool authoritative = info.transportOk && (info.httpCode / 100 == 2) && info.size > 0;
|
||||
|
||||
switch (decideResume(existing_bytes, authoritative, info.size)) {
|
||||
case ResumeAction::ReuseComplete: {
|
||||
// Complete from an earlier attempt (e.g. "free some space, then Retry"): reuse it instead of
|
||||
// re-downloading several GB. verifyChecksums() still runs on it and deletes it on a mismatch.
|
||||
DEBUG_LOGF("[Bootstrap] Local file is complete (%lld bytes) - reusing it\n", existing_bytes);
|
||||
curl_easy_cleanup(curl);
|
||||
resume_offset_ = 0;
|
||||
setProgress(State::Downloading,
|
||||
"Reusing the completed download (" + formatSize((double)existing_bytes) + ")",
|
||||
(double)existing_bytes, (double)info.size);
|
||||
return true;
|
||||
}
|
||||
case ResumeAction::TruncateAndFresh: {
|
||||
DEBUG_LOGF("[Bootstrap] Local file (%lld) > remote (%lld), restarting\n", existing_bytes, info.size);
|
||||
std::error_code ec;
|
||||
fs::remove(destZip, ec);
|
||||
existing_bytes = 0;
|
||||
break;
|
||||
}
|
||||
case ResumeAction::Fresh:
|
||||
existing_bytes = 0; // nothing usable on disk; fresh download
|
||||
break;
|
||||
case ResumeAction::RangedGet:
|
||||
break; // resume from existing_bytes (may be an offline/unverified resume)
|
||||
}
|
||||
|
||||
// Early, non-destructive check: the bytes still to download plus 256 MiB must fit (fails open when the
|
||||
// size or the free space is unknown). Nothing is deleted; a partial file is kept for a later resume.
|
||||
if (remoteSize > existing_bytes) {
|
||||
const std::uint64_t need = satAdd(static_cast<std::uint64_t>(remoteSize - existing_bytes),
|
||||
// Early, non-destructive free-space check: only when the remote size is authoritative. Nothing is
|
||||
// deleted; a partial file is always kept for a later resume.
|
||||
if (authoritative && info.size > existing_bytes) {
|
||||
const std::uint64_t need = satAdd(static_cast<std::uint64_t>(info.size - existing_bytes),
|
||||
256ull * 1024ull * 1024ull);
|
||||
std::error_code ec;
|
||||
const fs::space_info sp = fs::space(fs::path(destZip).parent_path(), ec);
|
||||
@@ -610,6 +623,8 @@ bool Bootstrap::download(const std::string& url, const std::string& destZip) {
|
||||
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L);
|
||||
|
||||
CURLcode res = curl_easy_perform(curl);
|
||||
long httpCode = 0;
|
||||
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &httpCode);
|
||||
fclose(fp);
|
||||
curl_easy_cleanup(curl);
|
||||
|
||||
@@ -622,6 +637,16 @@ bool Bootstrap::download(const std::string& url, const std::string& destZip) {
|
||||
return download(url, destZip); // recursive retry without resume
|
||||
}
|
||||
|
||||
// We asked to resume but the server answered 200 (whole file), not 206: libcurl may append it to the
|
||||
// partial instead of erroring, which would corrupt the archive. Restart cleanly from scratch.
|
||||
if (res == CURLE_OK && existing_bytes > 0 && httpCode != 206) {
|
||||
DEBUG_LOGF("[Bootstrap] Ranged request answered %ld (not 206) - restarting without resume\n", httpCode);
|
||||
resume_offset_ = 0;
|
||||
std::error_code ec;
|
||||
fs::remove(destZip, ec);
|
||||
return download(url, destZip);
|
||||
}
|
||||
|
||||
if (res != CURLE_OK) {
|
||||
DEBUG_LOGF("[Bootstrap] curl error: %s\n", curl_easy_strerror(res));
|
||||
last_curl_ = static_cast<int>(res);
|
||||
|
||||
Reference in New Issue
Block a user