Skip to content

fix: verify the size of a finished download - #291

Merged
mkb79 merged 1 commit into
masterfrom
fix/verify-download-size
Aug 15, 2026
Merged

fix: verify the size of a finished download#291
mkb79 merged 1 commit into
masterfrom
fix/verify-download-size

Conversation

@mkb79

@mkb79 mkb79 commented Aug 14, 2026

Copy link
Copy Markdown
Owner

The download size was never checked. check_download_size existed but nothing called it — the postprocessing list held check_status_code twice, once in the slot the size check belonged in:

status_checks = [
    check_status_for_message,
    check_status_code,
    check_status_code,      # ← where check_download_size belonged
    check_content_type
]

So an audiobook that arrived at a different length than announced was renamed to its final name and counted as a success.

Three defects, each hiding the next

Defect Why it stayed hidden
1 check_download_size missing from _postprocessing The duplicate check_status_code made the list look full
2 Its return Status.DownloadSizeMismatch sat outside the comparison, firing even when the sizes matched Never called, so never observed
3 _check_and_return_download_result never passed head_response — the one argument the check needs Wiring it up would have raised TypeError, not checked anything

Fixing any one alone does nothing useful. Fixing 1 without 2 would have failed every download.

The old utils.Downloader does check the size (utils.py:279), so covers and PDFs were covered while the audiobooks — what the tool exists for — were not.

What it catches, and what it does not

Catches: a download whose final length differs from the one the preceding request announced.

Not a mid-transfer cut. httpx already rejects a response that ends early against its own Content-Length, raising RemoteProtocolError before postprocessing runs. Verified.

Encoded transfers are skipped, whichever scheme. Content-Length may then describe the encoded bytes while what reaches the file may have been decoded on the way. The guard asks whether the transfer was plain rather than listing schemes, so gzip, deflate, br, zstd, chained values like gzip, br and anything added later all skip the comparison; only an absent header and identity reach it.

Deliberately out of scope

Parsing Content-Range, insisting on 206 for a resumed request, and tying the two requests together with If-Range. Those harden resume itself and belong with the downloader rework. Size alone does not prove the content is right; it catches the length going wrong.

Verification

24 new tests, 101 → 125. Each defect and the encoding guard was reintroduced individually to confirm the tests fail. The wiring is proven through behaviour rather than by reading source: _postprocessing runs over a prepared temp file and must return DownloadSizeMismatch and leave the target absent.

Encoding matrix: 9 schemes skip the comparison, 3 spellings of identity still reach it, and an encoding on either the probe or the real response is enough to skip.

Smoke tested against the real API in a temp directory:

Case Result
Cover, chapters (old downloader path) unchanged
aaxc, 1.7 GB streamed Success, no false positive, no leftovers
aaxc, 32 MB in one go Success
250 MB, aborted at 40 MB, restarted resumed from 38.9M/250M, Success, MP4 box chain ftyp → moov → mdat ends exactly flush with the file size
gzip / deflate transfer Success, comparison correctly skipped

@mkb79
mkb79 force-pushed the fix/verify-download-size branch from 529602e to 9d015bf Compare August 14, 2026 12:55
The download size was never checked. `check_download_size` existed but
nothing called it: the postprocessing list held `check_status_code`
twice, once in the slot the size check belonged in. So an audiobook that
arrived at a different length than announced was renamed to its final
name and counted as a success.

Three defects kept each other hidden:

- The check was missing from `Downloader._postprocessing`, replaced by a
  duplicate of `check_status_code`.
- `check_download_size` returned `DownloadSizeMismatch` whenever both
  sizes were known, equal or not — its `return` sat outside the
  comparison. Wiring the check up without fixing this would have failed
  every download.
- `_check_and_return_download_result` never passed `head_response`,
  which is the only argument the size check actually needs. Wiring it up
  would have raised TypeError instead of checking anything.

The old `utils.Downloader` does check the size, so covers and PDFs were
covered while the audiobooks were not.

What it catches: a download whose final length differs from the one the
preceding request announced. Not a mid-transfer cut — httpx already
rejects a response that ends early against its own Content-Length.

An encoded transfer is left alone, whichever scheme it uses.
Content-Length may then describe the encoded bytes while httpx may hand
over decoded ones, so the two are not reliably comparable and the
comparison could reject a sound download. The guard asks whether the
transfer was plain rather than listing schemes, so gzip, deflate, br,
zstd, chained values such as "gzip, br" and anything added later all
skip the comparison; only an absent header and `identity` reach it.

Deliberately out of scope, for the downloader work rather than a bug
fix: parsing Content-Range, insisting on 206 for a resumed request, and
tying the two requests together through If-Range. Size alone does not
prove the content is right; it catches the length going wrong.

The check runs last. A text response is reported through its own message
first, so `DownloadIndividualParts` still reaches the caller instead of
being reported as a size mismatch.
@mkb79
mkb79 force-pushed the fix/verify-download-size branch from 9d015bf to 5bdb2f0 Compare August 14, 2026 20:57
@mkb79
mkb79 merged commit 72ef602 into master Aug 15, 2026
6 checks passed
@mkb79
mkb79 deleted the fix/verify-download-size branch August 15, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant