fix(upload): keep queued ingest names within filesystem and processor limits - #1511
Open
AlfonsoDehesa wants to merge 1 commit into
Open
fix(upload): keep queued ingest names within filesystem and processor limits#1511AlfonsoDehesa wants to merge 1 commit into
AlfonsoDehesa wants to merge 1 commit into
Conversation
… limits Browser uploads of long file names failed with `OSError: [Errno 36] File name too long` because `_get_ingest_path` only knew about the final basename, while three sibling names share the same stem in the ingest directory: the streaming `.uploading` temp file, the `.cwa.json` sidecar manifest written for add-format uploads, and the `.cwa.failed.json` copy that the ingest processor preserves on add-format failure. The destination filesystem rejects any of those once the stem + suffix exceeds its per-component name limit, and `scripts/ingest_processor.py` silently truncates queued basenames over 150 characters before looking up their manifest. The 150-character truncation would also orphan the sidecar manifest for long add-format uploads even when the filesystem limit was generous. This change allocates one budget that respects both limits at the same time and keeps the upload pipeline stable on every supported deployment. - Query the ingest directory's `PC_NAME_MAX` via `os.pathconf`, falling back to a documented 255-byte Linux/POSIX limit when the call is unavailable or returns an invalid value. - Reserve the longest suffix actually created around an upload: `.uploading`, `.cwa.json`, `.cwa.failed.json`. - Cap the queued basename at CWA's ingest processor 150-character threshold so the processor never renames it after we hand it off. - Truncate only the stem, on a UTF-8 boundary, preserving the sanitized extension and the uniqueness prefix. - Keep ordinary short filenames byte-for-byte unchanged. - Raise a clear `ValueError` when the fixed components alone exceed the budget. The `tests/unit/test_editbooks_ingest_path.py` suite covers the new behaviour end-to-end: new-book and add-format prefixes, ordinary short names, multibyte stems and extensions, every sidecar path fitting the filesystem limit, the 150-character processor cap, `pathconf` returning `OSError` / `0` / `-1` / `None` / non-integer values, and a real `tmp_path` write of the longest allowed name. The suite passes against the production `crocodilestick/calibre-web-automated:v4.0.6` image (8/8) and on the current `main` test container. Notes for reviewers: - The 150-character limit in `scripts/ingest_processor.py` is left intact as a defensive guard for files that arrive through other paths (manual copy, Syncthing, NFS move). Relaxing or removing it is a separate audit, not part of this PR. - The 255-byte fallback matches the POSIX `NAME_MAX` used by every supported Linux deployment. It is documented and tested for filesystems that reject `pathconf`. - Broader Ruff cleanup of `cps/editbooks.py` is intentionally out of scope; the touched lines do not introduce new violations.
Author
|
Note on the For evidence on the patch itself, the new test suite passes locally:
The broader |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Browser uploads of long file names fail with:
and the CWA log records
OSError: [Errno 36] File name too long. Thishits real-world names from sources like Anna's Archive dumps that carry
title, series, publisher, ISBN, hash, and source information all in the
filename.
Root cause
_get_ingest_pathincps/editbooks.pyonly knows about the finalbasename. Three sibling names share that stem in the ingest directory:
.uploadingtemp file.cwa.jsonsidecar manifest written for add-format uploads.cwa.failed.jsoncopy thatscripts/ingest_processor.pypreserves on add-format failure
The destination filesystem rejects any of those once the stem + suffix
exceeds its per-component name limit.
scripts/ingest_processor.pyalso silently truncates queued basenames over 150 characters before
looking up their manifest, which would orphan the sidecar for long
add-format uploads even on filesystems with a generous name limit.
Fix
Allocate one budget that respects both limits at the same time:
PC_NAME_MAXviaos.pathconf, with adocumented 255-byte Linux/POSIX fallback when the call is unavailable
or returns an invalid value.
.uploading,.cwa.json,.cwa.failed.json.threshold so the processor never renames it after we hand it off.
extension and the uniqueness prefix.
ValueErrorwhen the fixed components alone exceed thebudget.
Tests
tests/unit/test_editbooks_ingest_path.pycovers the new behaviourend-to-end:
pathconfreturningOSError/0/-1/None/ non-integervalues
tmp_pathwrite of the longest allowed nameThe suite passes against the production
crocodilestick/calibre-web-automated:v4.0.6image (8/8) and on thecurrent
maintest container.Notes for reviewers
scripts/ingest_processor.pyis leftintact as a defensive guard for files that arrive through other paths
(manual copy, Syncthing, NFS move). Relaxing or removing it is a
separate audit, not part of this PR.
NAME_MAXused by everysupported Linux deployment. It is documented and tested for
filesystems that reject
pathconf.cps/editbooks.pyis intentionally out ofscope; the touched lines do not introduce new violations.