Snapshot resume should reject mismatched Content-Range starts before appending to .part files

[Kewe63]

Summary

The snapshot downloader supports resumable downloads by keeping a .part file and retrying with a Range request starting at the existing partial file size.

However, when the server returns HTTP 206 Partial Content, the downloader does not verify that the response Content-Range start offset matches the local .part file size before appending the response body.

This means a server, CDN, cache, or proxy that returns a mismatched partial response can cause the downloader to append bytes from the wrong offset to the existing .part file. The resulting snapshot file can be corrupted while still being treated as a successful download.

---

Affected File

crates/snapshots/src/download.rs

---

Observed Behavior

When a partial file already exists, the downloader sends a request like:

Range: bytes=7-

If the server responds with:

HTTP/1.1 206 Partial Content
Content-Range: bytes 0-3/11

the response starts at byte 0, not byte 7.

The current implementation still appends the returned body to the existing .part file instead of rejecting the response or restarting the download.

---

Expected Behavior

For resumable downloads, when a local .part file exists and the downloader requests:

Range: bytes=<existing_size>-

then a 206 response should only be accepted if the Content-Range start offset is exactly <existing_size>.

If the Content-Range start does not match the requested/local offset, the downloader should either:

1. return an error, or
2. discard the stale .part file and restart the download from byte 0.

It should not append mismatched bytes to the existing partial file.

---

Reproduction

I verified this with a focused regression test against the current code.

The test creates a partial file containing:

prefix-

The partial file size is 7 bytes, so the downloader sends:

Range: bytes=7-

The mock server intentionally responds with a mismatched Content-Range:

Content-Range: bytes 0-3/11

and body:

pref

The expected behavior is for the downloader to reject this response. Instead, the current code accepts it and appends the bytes, so the regression test fails.

Test command used:

cargo +1.94.0 test -p arc-snapshots resumable_download_rejects_mismatched_content_range_start -- --nocapture

Result on current code:

test download::tests::resumable_download_rejects_mismatched_content_range_start ... FAILED

Failure message:

mismatched Content-Range should not append to the existing .part file

---

Why This Matters

A resumed snapshot download relies on the server returning exactly the missing byte range. If the response starts at a different offset, appending it to the local .part file produces a corrupted snapshot.

This can happen due to:

• a buggy origin server,
• a misbehaving proxy/cache/CDN,
• stale cached partial responses,
• or any server-side issue that returns an incorrect 206 response.

The downloader already tracks the local partial size and sends a precise Range request, so validating the returned Content-Range start would prevent accepting corrupted resumed downloads.

---

Suggested Fix

When resuming from an existing .part file:

1. Parse the Content-Range header on HTTP 206 responses.
2. Extract the start offset.
3. Compare it with the local partial file size / requested Range start.
4. If they differ, reject the response or restart from scratch.

For example:

• existing .part size: 7
• request: Range: bytes=7-
• valid response: Content-Range: bytes 7-10/11
• invalid response: Content-Range: bytes 0-3/11

The invalid response should not be appended to the existing .part file.

---

Potential Regression Test

A regression test can be added near the existing resumable download tests in crates/snapshots/src/download.rs.

Test idea:

• seed a .part file with b"prefix-"
• configure the mock server to expect Range: bytes=7-
• return HTTP 206 with Content-Range: bytes 0-3/11
• assert that run_resumable_download(...) returns an error
• assert that mismatched data is not accepted as a completed snapshot

[osr21]

Confirmed accurate. I don't have a Rust toolchain in this environment so I reviewed the source rather than running your test, but the mechanism is unambiguous in the code:

• parse_total_size (download.rs:441) does .split('/').next_back() — it extracts the total only. The start offset is not parsed anywhere in the file.
• attempt_download (download.rs:498) decides append-vs-truncate purely on is_partial && existing_size > 0 (line 512). Nothing compares the advertised range start against existing_size.

So a 206 carrying Content-Range: bytes 0-3/11 in response to Range: bytes=7- is appended verbatim. Your report is correct.

Credit where it's due: the adjacent cases are handled

Worth saying so this reads as one gap in an otherwise careful function rather than a careless downloader:

• A server that ignores Range and returns 200 — the more common misbehaviour by far — is handled correctly. is_partial is false, so open_part_file(.., false) calls File::create and truncates instead of appending.
• A 206 arriving when existing_size == 0 also truncates.
• A stale .part belonging to a different URL is handled by prepare_partial_download and the url_identity marker, with a deliberate comment about deletion/marker ordering surviving process termination.

That last one shows the author thought hard about exactly this class of problem — appending to bytes that don't belong to this transfer — and the Content-Range start is the one case that slipped through.

Severity needs correcting in both directions

Higher than the issue states in one respect. I grepped all of crates/snapshots/src/ for sha256/checksum/digest/blake3: the only hit is a comment inside a test. There is no integrity verification of the archive anywhere in the crate, and the manifest carries no hash or size field. So nothing authenticates the bytes.

Lower in another. "Corrupted while still being treated as a successful download" is true of resumable_download, but not end-to-end. extract_archive (download.rs:586) runs lz4::Decoder::new(file)? and then tar. A byte-spliced lz4 frame will fail frame/block decoding with near-certainty, and tar's per-header checksums sit behind that. The realistic outcome is a loud failure at extraction, not a silently corrupt datadir. I think that's worth stating explicitly, because it changes what this bug costs.

And what it costs is still meaningful — it's just economic rather than a correctness hole. download_archive's own doc comment says the whole point of .part is to avoid "transferring tens of gigabytes again." When extraction fails, the callers (download.rs:798–808 and 826–832) remove_dir_all the staging dir, so the corrupt archive and all resumable progress are destroyed and the multi-GB transfer restarts from zero. Because the corruption is only detectable at extraction — outside resumable_download's retry loop — the in-function retries cannot help.

One mitigating detail: it does self-heal. After cleanup the next run starts at 0, sends no Range header, and gets a plain 200, so a one-off bad 206 won't loop forever.

A second gap in the same path: total is never checked against the bytes on disk

Ok(total) => {
    std::fs::rename(&part_path, &final_path)?;

attempt_download returns the server-advertised total and resumable_download (download.rs:555) promotes the .part immediately. Nothing asserts std::fs::metadata(&part_path)?.len() == total. That one line would catch a broader class of short and over-long writes than the Content-Range fix alone.

But it would not catch your repro — and that's the interesting part. Your .part is 7 bytes, the mismatched body is 4 bytes, and the advertised total is 11. 7 + 4 == 11, so the corrupt result is exactly the right length. That coincidence is a strong argument for your specific fix rather than against it: the start-offset check catches corruption that is invisible to any length-based validation. The two checks are complementary, not redundant, and I'd suggest landing both.

Same class of gap, worth folding in: total is also never compared across resume attempts. If a stable URL's content changes between attempts, the .part becomes a splice of two different files. url_identity guards URL changes but not content changes behind the same URL.

A live bug in the exact function your fix will touch

RFC 9110 §14.4 permits the complete-length to be * when the server doesn't know it, so Content-Range: bytes 0-3/* is a legal 206. Today parse_total_size takes next_back() → "*" → parse::<u64>() fails → None, and the caller reports:

Server did not provide Content-Length or Content-Range header

…which is false; the server provided exactly that header. Since your fix has to rewrite this parse anyway to extract the start offset, it's a good moment to handle * (accept the range, treat the total as unknown) rather than leave a valid 206 rejected with a misleading message.

While you're in there, a real parse also lets you reject an inverted or contradictory range like bytes 7-3/11, which currently parses "fine" because only the segment after the last / is ever read.

Fix shape

Rather than bolting a second parse onto parse_total_size, parsing once into a small struct keeps both call sites honest:

struct ContentRange { start: u64, end: u64, total: Option<u64> }

fn parse_content_range(value: &str) -> Option<ContentRange> { /* "bytes <start>-<end>/<total|*>" */ }

Then in attempt_download, when is_partial && existing_size > 0, require range.start == existing_size and fail (or discard the .part and restart from 0) otherwise. Your option 2 — discard and restart — is the friendlier default for an operator sitting behind a broken CDN, since option 1 leaves them stuck retrying against the same bad cache until MAX_DOWNLOAD_RETRIES runs out.

Test placement

Your proposed test drops straight into the existing block: resumable_download_resumes_the_same_url (download.rs:1118) already builds exactly this scenario with wiremock and ResponseTemplate::new(206).append_header("Content-Range", "bytes 7-10/11"). The mismatched case is that same test with a different header value and an error assertion, so it should be a genuinely small addition — and it will sit naturally beside the eight resumable_download_discards_* tests, which is precisely the behaviour class this belongs to.

One small thing: rust-toolchain.toml pins channel = "1.93.0", but the reported command was cargo +1.94.0 test, which overrides the pin. Worth re-running under the pinned toolchain so the result matches what CI would produce.