Skip to content

fix(upload): keep queued ingest names within filesystem and processor limits - #1511

Open
AlfonsoDehesa wants to merge 1 commit into
crocodilestick:mainfrom
AlfonsoDehesa:upload-filename-fix-main
Open

fix(upload): keep queued ingest names within filesystem and processor limits#1511
AlfonsoDehesa wants to merge 1 commit into
crocodilestick:mainfrom
AlfonsoDehesa:upload-filename-fix-main

Conversation

@AlfonsoDehesa

Copy link
Copy Markdown

Problem

Browser uploads of long file names fail with:

Failed to queue upload for processing

and the CWA log records OSError: [Errno 36] File name too long. This
hits 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_path in cps/editbooks.py only knows about the final
basename. Three sibling names share that stem in the ingest directory:

  • the streaming .uploading temp file
  • the .cwa.json sidecar manifest written for add-format uploads
  • the .cwa.failed.json copy that scripts/ingest_processor.py
    preserves on add-format failure

The destination filesystem rejects any of those once the stem + suffix
exceeds its per-component name limit. scripts/ingest_processor.py
also 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:

  • Query the ingest directory's PC_NAME_MAX via os.pathconf, with a
    documented 255-byte Linux/POSIX fallback 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.

Tests

tests/unit/test_editbooks_ingest_path.py covers the new behaviour
end-to-end:

  • new-book and add-format prefixes
  • ordinary short names preserved byte-for-byte
  • 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
  • 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.

… 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.
@AlfonsoDehesa

Copy link
Copy Markdown
Author

Note on the action_required status: the upstream Test Suite workflow run completed immediately with no jobs created and no logs available on my end (GitHub purged them before I could fetch). I can't re-trigger it from my fork.

For evidence on the patch itself, the new test suite passes locally:

  • Python 3.13.12 (matches actions/setup-python@v5 in .github/workflows/tests.yml)
  • Run inside the official crocodilestick/calibre-web-automated:v4.0.6 image so cps imports resolve against the production runtime
  • Repository's own pytest.ini and pyproject.toml
============================= test session starts ==============================
platform linux -- Python 3.13.12, pytest-9.1.1, pluggy-1.6.0
rootdir: /src
configfile: pytest.ini
plugins: anyio-4.12.0, Flask-Dance-7.1.0
collected 8 items

tests/unit/test_editbooks_ingest_path.py ........                        [100%]

============================= 8 passed in 2.30s ===============================

The broader tests/unit/ directory was not re-run because it requires Calibre CLI tools I don't have in this environment, and that broader run is outside the scope of this PR. Happy to tighten anything in the new tests or in cps/editbooks.py based on review feedback.

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