From 667c89ba510682aca8a9d592250f997a306499b3 Mon Sep 17 00:00:00 2001 From: DanS Date: Tue, 13 Oct 2026 20:57:21 -0500 Subject: [PATCH] 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 --- src/util/bootstrap.cpp | 99 ++++++++++++++++++++++++++---------------- src/util/bootstrap.h | 10 +++-- tests/test_phase4.cpp | 14 ++++++ 3 files changed, 83 insertions(+), 40 deletions(-) diff --git a/src/util/bootstrap.cpp b/src/util/bootstrap.cpp index 160c356..a3f852e 100644 --- a/src/util/bootstrap.cpp +++ b/src/util/bootstrap.cpp @@ -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(-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(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(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(res); diff --git a/src/util/bootstrap.h b/src/util/bootstrap.h index 11defc2..ce697af 100644 --- a/src/util/bootstrap.h +++ b/src/util/bootstrap.h @@ -141,6 +141,10 @@ public: /// expectedHexLen hex chars; "" otherwise (an HTTP error page is never a checksum). static std::string parseChecksumFile(const std::string& content, std::size_t expectedHexLen); + enum class ResumeAction { Fresh, TruncateAndFresh, ReuseComplete, RangedGet }; + /// Pure, no-I/O. authoritative = HEAD was 2xx with a positive Content-Length. + static ResumeAction decideResume(long long existingBytes, bool authoritative, long long remoteSize); + private: mutable std::mutex mutex_; Progress progress_; @@ -173,9 +177,9 @@ private: /// Verify downloaded zip against remote checksums. Returns true if valid. bool verifyChecksums(const std::string& zipPath, const std::string& baseUrl); - /// Perform a HEAD request to get remote file size. - /// Returns -1 on failure. - long long getRemoteFileSize(const std::string& url); + struct RemoteInfo { bool transportOk = false; long httpCode = 0; long long size = -1; }; + /// HEAD the remote file. transportOk=false on any curl error; size=-1 when absent. + RemoteInfo getRemoteFileSize(const std::string& url); /// Byte offset of the partial file when resuming a download. /// Added to dlnow in progress reports so the bar reflects true total. diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 7233fa5..4a6325e 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -3961,6 +3961,19 @@ void testBootstrapParseChecksum() EXPECT_EQ(Bootstrap::parseChecksumFile("", 64), std::string()); } +void testBootstrapDecideResume() +{ + using R = dragonx::util::Bootstrap::ResumeAction; + using dragonx::util::Bootstrap; + EXPECT_TRUE(Bootstrap::decideResume(0, false, -1) == R::Fresh); + EXPECT_TRUE(Bootstrap::decideResume(0, true, 100) == R::Fresh); + EXPECT_TRUE(Bootstrap::decideResume(5, true, 3) == R::TruncateAndFresh); + EXPECT_TRUE(Bootstrap::decideResume(5, true, 5) == R::ReuseComplete); + EXPECT_TRUE(Bootstrap::decideResume(3, true, 5) == R::RangedGet); + EXPECT_TRUE(Bootstrap::decideResume(3, false, -1) == R::RangedGet); // offline: keep + try range + EXPECT_TRUE(Bootstrap::decideResume(3, false, 999) == R::RangedGet); // bogus error-page length ignored +} + void testBootstrapSpaceSufficient() { using dragonx::util::Bootstrap; @@ -18448,6 +18461,7 @@ int main() testXmrigProvisionPolicy(); testBootstrapSpaceSufficient(); testBootstrapParseChecksum(); + testBootstrapDecideResume(); testBootstrapPlanArchive(); #ifndef _WIN32 testBootstrapReclaimable();