From 183cfd2da4fd00c1cd5280f078db7f30d8a646f3 Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:11:34 -0700 Subject: [PATCH] Fix: keep counter and extension when shortening long public filenames (#14472) get_public_filename relied on sanitize_filename to truncate, which cuts from the end and so dropped the counter and extension of long titles. Multi-byte titles could also exceed the 255 byte filesystem limit. Shorten the title by UTF-8 bytes before appending the counter and extension. The exporter reserves room for its longer derived thumbnail and archive suffixes. --- .../management/commands/document_exporter.py | 19 ++++- src/documents/models.py | 35 +++++---- src/documents/tests/test_document_model.py | 72 +++++++++++++++++++ .../tests/test_management_exporter.py | 67 +++++++++++++++++ 4 files changed, 178 insertions(+), 15 deletions(-) diff --git a/src/documents/management/commands/document_exporter.py b/src/documents/management/commands/document_exporter.py index ea92f4e59..10b929679 100644 --- a/src/documents/management/commands/document_exporter.py +++ b/src/documents/management/commands/document_exporter.py @@ -3,6 +3,7 @@ from itertools import islice from pathlib import Path from typing import TYPE_CHECKING from typing import Any +from typing import Final from allauth.mfa.models import Authenticator from allauth.socialaccount.models import SocialAccount @@ -70,6 +71,15 @@ from paperless.models import ApplicationConfiguration from paperless_mail.models import MailAccount from paperless_mail.models import MailRule +# Derived names replace the original's extension with these suffixes +THUMBNAIL_SUFFIX: Final[str] = "-thumbnail.webp" +ARCHIVE_SUFFIX: Final[str] = "-archive.pdf" +# Leave room for the longest derived suffix, which replaces the original's extension +MAX_BASE_NAME_BYTES: Final[int] = Document.MAX_PUBLIC_FILENAME_BYTES - max( + len(THUMBNAIL_SUFFIX), + len(ARCHIVE_SUFFIX), +) + def serialize_queryset_batched( queryset: "QuerySet[Any]", @@ -512,7 +522,10 @@ class Command(CryptMixin, PaperlessCommand): counter=filename_counter, ) else: - base_name = document.get_public_filename(counter=filename_counter) + base_name = document.get_public_filename( + counter=filename_counter, + max_bytes=MAX_BASE_NAME_BYTES, + ) # The thumbnail and archive names are derived from the stem, so # uniqueness must hold without the original's extension @@ -541,7 +554,7 @@ class Command(CryptMixin, PaperlessCommand): document_dict[EXPORTER_FILE_NAME] = original_arc if not self.no_thumbnail: - thumbnail_name = base_name.parent / (base_name.stem + "-thumbnail.webp") + thumbnail_name = base_name.parent / (base_name.stem + THUMBNAIL_SUFFIX) if self.use_folder_prefix: thumbnail_name = Path("thumbnails") / thumbnail_name thumbnail_arc = thumbnail_name.as_posix() @@ -550,7 +563,7 @@ class Command(CryptMixin, PaperlessCommand): thumbnail_arc = None if not self.no_archive and document.has_archive_version: - archive_name = base_name.parent / (base_name.stem + "-archive.pdf") + archive_name = base_name.parent / (base_name.stem + ARCHIVE_SUFFIX) if self.use_folder_prefix: archive_name = Path("archive") / archive_name archive_arc = archive_name.as_posix() diff --git a/src/documents/models.py b/src/documents/models.py index ff8632b12..f5a9f6ddd 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -157,6 +157,8 @@ class StoragePath(MatchingModel): class Document(SoftDeleteModel, ModelWithOwner): # type: ignore[django-manager-missing] MAX_STORED_FILENAME_LENGTH: Final[int] = 1024 + # Per-component limit of common filesystems (ext4, XFS, btrfs, ZFS, APFS) + MAX_PUBLIC_FILENAME_BYTES: Final[int] = 255 correspondent = models.ForeignKey( Correspondent, @@ -484,28 +486,37 @@ class Document(SoftDeleteModel, ModelWithOwner): # type: ignore[django-manager- def archive_file(self): return Path(self.archive_path).open("rb") - def get_public_filename(self, *, archive=False, counter=0, suffix=None) -> str: + def get_public_filename( + self, + *, + archive: bool = False, + counter: int = 0, + max_bytes: int = MAX_PUBLIC_FILENAME_BYTES, + ) -> str: """ Returns a sanitized filename for the document, not including any paths. + + The name fits in max_bytes (UTF-8). A long title is shortened before + the counter and extension are appended, because sanitize_filename + truncates from the end. The tail is cut only when max_bytes is + smaller than the tail itself. """ # Root owns metadata for all versions context_document = ( self.root_document if self.root_document_id is not None else self ) - result = str(context_document) - if counter: - result += f"_{counter:02}" + extension = ".pdf" if archive else self.file_type + tail = f"_{counter:02}{extension}" if counter else extension - if suffix: - result += suffix + head_bytes = max(0, max_bytes - len(tail.encode())) + head = str(context_document).encode()[:head_bytes].decode(errors="ignore") - if archive: - result += ".pdf" - else: - result += self.file_type - - return pathvalidate.sanitize_filename(result, replacement_text="-") + return pathvalidate.sanitize_filename( + head + tail, + replacement_text="-", + max_len=max_bytes, + ) @property def file_type(self): diff --git a/src/documents/tests/test_document_model.py b/src/documents/tests/test_document_model.py index a6fb0b3f6..3ff51711e 100644 --- a/src/documents/tests/test_document_model.py +++ b/src/documents/tests/test_document_model.py @@ -4,13 +4,16 @@ from datetime import date from pathlib import Path from unittest import mock +import pytest from django.test import TestCase from django.test import override_settings from faker import Faker +from pytest_mock import MockerFixture from documents.models import Correspondent from documents.models import Document from documents.tasks import empty_trash +from paperless_testing.factories import DocumentFactory class TestDocument(TestCase): @@ -257,3 +260,72 @@ def test_suggestion_content() -> None: ) doc.content = content_over_limit assert doc.suggestion_content == expected_cropped_content + + +class TestPublicFilenameLength: + @pytest.fixture(autouse=True) + def _no_parser_registry(self, mocker: MockerFixture) -> None: + # The parser registry reads ApplicationConfiguration from the database + extensions = {"application/pdf": ".pdf", "image/jpeg": ".jpg"} + mocker.patch( + "documents.models.get_default_file_extension", + side_effect=extensions.__getitem__, + ) + + LIMIT = Document.MAX_PUBLIC_FILENAME_BYTES + + @pytest.mark.parametrize( + ("title", "archive", "counter", "max_bytes", "tail"), + [ + pytest.param("a" * 300, False, 0, LIMIT, ".jpg", id="ascii"), + pytest.param("a" * 300, True, 0, LIMIT, ".pdf", id="archive"), + pytest.param("ä" * 200, False, 0, LIMIT, ".jpg", id="umlaut"), + pytest.param("ä" * 200, False, 1, LIMIT, "_01.jpg", id="count"), + pytest.param("a" * 300, False, 1, 100, "_01.jpg", id="max"), + ], + ) + def test_long_title_keeps_tail_within_limit( + self, + title: str, + *, + archive: bool, + counter: int, + max_bytes: int, + tail: str, + ) -> None: + """ + GIVEN: + - A document whose title alone is longer than the byte limit + WHEN: + - The public file name is generated + THEN: + - The name ends with the counter and extension + - The name fits in the limit + """ + name = DocumentFactory.build( + title=title, + mime_type="image/jpeg", + ).get_public_filename( + archive=archive, + counter=counter, + max_bytes=max_bytes, + ) + + assert name.endswith(tail) + assert len(name.encode()) <= max_bytes + + def test_long_title_counters_give_distinct_names(self) -> None: + """ + GIVEN: + - A document whose title alone is longer than the byte limit + WHEN: + - Public file names are generated with increasing counters + THEN: + - Each counter produces a distinct name + """ + doc = DocumentFactory.build(title="ä" * 200, mime_type="application/pdf") + + names = [doc.get_public_filename(counter=counter) for counter in range(3)] + + assert len(set(names)) == 3 + assert all(len(n.encode()) <= self.LIMIT for n in names) diff --git a/src/documents/tests/test_management_exporter.py b/src/documents/tests/test_management_exporter.py index 9a2c95e9e..3bfc909c8 100644 --- a/src/documents/tests/test_management_exporter.py +++ b/src/documents/tests/test_management_exporter.py @@ -1537,3 +1537,70 @@ class TestExportDerivedFileNameCollisions: assert ( target / record[EXPORTER_ARCHIVE_NAME] ).read_text() == f"archive {pk}" + + def test_long_titles_export_within_file_name_limit( + self, + paperless_dirs: PaperlessDirs, + tmp_path: Path, + ) -> None: + """ + GIVEN: + - Two documents with the same created date and the same title + - The title alone is longer than 255 bytes and contains umlauts + WHEN: + - The exporter runs + THEN: + - The export finishes with distinct original, thumbnail and archive names + - Every exported file name fits in the byte limit + - Each exported file holds its own document's content + """ + for pk in (1, 2): + (paperless_dirs.originals_dir / f"{pk:07}.pdf").write_bytes( + f"original {pk}".encode(), + ) + (paperless_dirs.archive_dir / f"{pk:07}.pdf").write_bytes( + f"archive {pk}".encode(), + ) + (paperless_dirs.thumbnail_dir / f"{pk:07}.webp").write_bytes( + f"thumbnail {pk}".encode(), + ) + DocumentFactory( + pk=pk, + title="Bestellbestätigung " * 15, + created=date(2026, 10, 15), + filename=f"{pk:07}.pdf", + archive_filename=f"{pk:07}.pdf", + mime_type="application/pdf", + ) + + target = tmp_path / "export" + target.mkdir() + call_command( + "document_exporter", + str(target), + "--no-progress-bar", + skip_checks=True, + ) + + manifest = json.loads((target / "manifest.json").read_text()) + records = {r["pk"]: r for r in manifest if r["model"] == "documents.document"} + + for key in ( + EXPORTER_FILE_NAME, + EXPORTER_THUMBNAIL_NAME, + EXPORTER_ARCHIVE_NAME, + ): + assert records[1][key] != records[2][key] + + for pk in (1, 2): + record = records[pk] + for key, content in ( + (EXPORTER_FILE_NAME, f"original {pk}"), + (EXPORTER_THUMBNAIL_NAME, f"thumbnail {pk}"), + (EXPORTER_ARCHIVE_NAME, f"archive {pk}"), + ): + assert ( + len(Path(record[key]).name.encode()) + <= Document.MAX_PUBLIC_FILENAME_BYTES + ) + assert (target / record[key]).read_text() == content