Repository navigation
arcup: download_file's error handling is silently defeated on curl 8.14.0/8.14.1 #309
Description
Summary
Split off from review discussion on #304.
download_file in arcup/arcup relies on curl's exit code to detect
failed downloads:
if ! curl_with_headers -#fL --max-time 900 -o "$output_path" "$url"; then
rm -f "$output_path"
download_error "$tag" "$filename" "$url"
ficurl_with_headers includes CURL_RETRY_ARGS=(--retry 3 --retry-delay 2 --connect-timeout 15). On curl 8.14.0 and 8.14.1, combining --retry with
--fail causes curl to exit 0 even on a 404 (upstream bug, see
curl/curl#17554 — fixed in later releases; --retry 0 or omitting
--retry exits 22 correctly on those versions).
Reproduced by running the arcup test suite's e2e test against main on a
box with curl 8.14.1: instead of download_error's intended message, the
script fails later with Checksum file is empty: … — a much less useful
error for whoever hits it.
Impact
The install still fails closed (checksum verification catches the empty/
missing file downstream), so this isn't a security issue — just a
misleading-error-message issue on a curl line version that ships in
current distros and nixpkgs.
Suggested fix
Add an explicit non-empty-file check after the curl call in download_file
(and download_file_with_github_api, which has the same pattern),
independent of curl's exit code:
if ! curl_with_headers -#fL --max-time 900 -o "$output_path" "$url" || [[ ! -s "$output_path" ]]; then
rm -f "$output_path"
download_error "$tag" "$filename" "$url"
fiThis immunizes the check against the curl version-specific behavior
rather than depending on the exit code alone.
Activity
osr21 commentedon Sep 2, 2026
Reproduced this on curl 8.14.1 and confirmed the diagnosis. Three things I found while verifying it that I think change the scope of the fix — the most important being that
arcup/installdoes not fail closed.Version boundaries, confirmed
The fix is commit
b42776b(upstream PR curl/curl#17559). Checking containment against release tags: it is not incurl-8_14_1(that tag is 18 commits behind the fix) and is incurl-8_15_0(released 2025-07-16). So the affected set is exactly 8.14.0 and 8.14.1, fixed in 8.15.0 — matching the report.Side-by-side on the same 404, same box:
-fL-fL --retry 3-fL --retry 0The trigger is narrower than "a 404" — and it's the worst possible subset
The upstream PR title is "fix return code when
--retryis used but not triggered", and that's exactly what the behavior shows. Only non-retryable statuses lose the exit code:$?So transient failures — the ones
--retryexists for — still report correctly, and it is precisely the permanent failures that go silent. Worth noting403is in that set:raw.githubusercontent.comserves 403 under rate limiting, so this is reachable without anyone publishing a bad URL.One trap for anyone implementing a fix:
-w '%{exitcode}'is clobbered too (reports0alongside the shell's0).%{http_code}stays truthful (404). Any exit-code-independent check has to key onhttp_code, notexitcode.arcup/installis the real bug — it silently installs a 0-byte arcupThe issue's impact assessment ("fails closed, checksum verification catches it downstream") holds for
arcupitself. I walked all sixCURL_RETRY_ARGScall sites, and every one inarcuphappens to be backstopped by a content check that accidentally covers for the exit code:download_file(327)Checksum file is empty— the reported symptomdownload_file_with_github_api(352)[[ -f "$output_path" ]]--fail, verified)resolve_github_asset_download_url(135)[[ -n "$location" ]]HTTP/2 404— but a 404 carries noLocation)fetch_release_json/_latest(216–228)update_arcup(441)remote_versiongrepFailed to determine remote version;cpis after the grep, so no clobberarcup/installline 75 is the exception, and it has no checksum step anywhere:mktempcreates the file, so unlikedownload_filethere is no "missing file" to trip over — the empty temp file is copied straight to$BIN_DIR/arcupandchmod +x'd. Running that fragment verbatim against a 404:main()then runs"$BIN_DIR/arcup", an empty script that exits 0, and the installer prints its full success epilogue. Net effect on 8.14.x: the one-liner install reports success, installs nothing, and returns 0 — not a misleading message, a silent success. That's also the path most likely to meet an affected curl, since it runs on whatever the user's distro ships before arcup exists.The test suite is structurally unable to catch this
test_download_error_lists_assetssimulates failure withcurl_with_headers() { return 22; }, and thefakebin/curlstubs follow the same convention. The tests assert against the exact exit-code contract that real 8.14.x violates, so they will stay green forever. A regression test has to stubreturn 0while leaving the output path empty or absent — that's the shape the bug actually produces.On the fix
The proposed
|| [[ ! -s "$output_path" ]]is right fordownload_file, and I'd extend it:download_file_with_github_api: upgrade the existing[[ -f ]]to[[ -s ]]— one character, closes the stale-file gap too.arcup/install: same[[ ! -s ]]guard. This is the one that changes behavior from "silently installs nothing" to an actual error.installcarries its own copy ofCURL_RETRY_ARGS, a per-call-site fix has to be applied in both files.Worth considering alongside it, as a single central mitigation:
That trades retries for correct exit codes on exactly the two broken versions and needs no per-site changes. Belt-and-braces would be both: the version gate restores the contract, and the
-sguards are version-independent and cost nothing.huklaa commentedon Sep 10, 2026
Hi, I would like to work on this issue. Could you assign it to me?
huklaa commentedon Sep 18, 2026
Maintainer coordination under the new contribution policy: I requested assignment here before the policy change, and PR #314 already contains the focused fix and regression test for this issue. I am not starting duplicate work; I would like to preserve that existing contribution. Could a maintainer please assign #309 to @huklaa and confirm whether #314 should continue as the implementation, or whether you want it resubmitted only after assignment?
huklaa commentedon Sep 18, 2026
Maintainer coordination under the new contribution policy: I requested assignment here before the policy change, and PR #314 already contains the focused fix and regression test for this issue. I am not starting duplicate work; I would like to preserve that existing contribution. Could a maintainer please assign #309 to @huklaa and confirm whether #314 should continue as the implementation, or whether you want it resubmitted only after assignment?