Explorar el Código

Fix archive endpoints crash with "Is a directory" for BambuStudio prints (#475)

When FTP download of a 3MF fails (e.g. BambuStudio-initiated prints),
the fallback archive has file_path="". Path.exists() on
settings.base_dir / "" resolves to the base directory itself and
returns True, so ZipFile() then fails with [Errno 21] Is a directory.

Replace .exists() with .is_file() across all 15 archive route checks
and 1 in main.py. Add file_path truthiness guard for finish photo
capture to prevent saving photos under the base directory.
maziggy hace 3 meses
padre
commit
055243683c

+ 1 - 0
CHANGELOG.md

@@ -16,6 +16,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Queue "Any Model" Jobs Stuck in "Waiting" After Plate Clear** ([#435](https://github.com/maziggy/bambuddy/issues/435)) — After the queue visibility fix above, "Any Model" jobs were correctly assigned to an idle printer but immediately crashed with `'>=' not supported between instances of 'str' and 'int'` when computing AMS filament mapping. MQTT raw data returns AMS unit and tray IDs as strings, but `_build_loaded_filaments()` compared them to integers without casting. The crash prevented the assignment from committing, so the scheduler retried every 30 seconds in an infinite loop. Cast `ams_id` and `tray_id` to `int()` to match the pattern already used for external spool IDs.
 - **Queue "Any Model" Jobs Stuck in "Waiting" After Plate Clear** ([#435](https://github.com/maziggy/bambuddy/issues/435)) — After the queue visibility fix above, "Any Model" jobs were correctly assigned to an idle printer but immediately crashed with `'>=' not supported between instances of 'str' and 'int'` when computing AMS filament mapping. MQTT raw data returns AMS unit and tray IDs as strings, but `_build_loaded_filaments()` compared them to integers without casting. The crash prevented the assignment from committing, so the scheduler retried every 30 seconds in an infinite loop. Cast `ams_id` and `tray_id` to `int()` to match the pattern already used for external spool IDs.
 - **SD Card Cleanup After Print Never Runs** ([#374](https://github.com/maziggy/bambuddy/issues/374)) — The post-print SD card cleanup (which deletes uploaded gcode from the printer root to prevent phantom prints on power cycle) used `printer_manager.get_printer()`, which returns a `PrinterInfo` with only `name` and `serial_number`. Accessing `.ip_address`, `.access_code`, and `.model` raised `AttributeError`, silently caught by the outer exception handler. Replaced with a DB query for the `Printer` model, matching the pattern used everywhere else in `on_print_complete()`.
 - **SD Card Cleanup After Print Never Runs** ([#374](https://github.com/maziggy/bambuddy/issues/374)) — The post-print SD card cleanup (which deletes uploaded gcode from the printer root to prevent phantom prints on power cycle) used `printer_manager.get_printer()`, which returns a `PrinterInfo` with only `name` and `serial_number`. Accessing `.ip_address`, `.access_code`, and `.model` raised `AttributeError`, silently caught by the outer exception handler. Replaced with a DB query for the `Printer` model, matching the pattern used everywhere else in `on_print_complete()`.
 - **Finish Photo Not Shown on Archives for BambuStudio Prints** ([#474](https://github.com/maziggy/bambuddy/issues/474)) — When a print was started from BambuStudio (not Bambuddy), the auto-archive had an empty `file_path`. The finish photo was saved correctly to `data/photos/`, but the photo serving endpoint resolved the path as `(base_dir / "").parent / "photos/"` which evaluates to `base_dir.parent/photos/` — one directory level too high. The photo existed on disk but the API returned 404. Fixed the path resolution in `get_photo`, `upload_photo`, and `delete_photo` to use `base_dir / Path(file_path).parent` (same pattern as the save code), which correctly resolves to `base_dir/photos/` when `file_path` is empty.
 - **Finish Photo Not Shown on Archives for BambuStudio Prints** ([#474](https://github.com/maziggy/bambuddy/issues/474)) — When a print was started from BambuStudio (not Bambuddy), the auto-archive had an empty `file_path`. The finish photo was saved correctly to `data/photos/`, but the photo serving endpoint resolved the path as `(base_dir / "").parent / "photos/"` which evaluates to `base_dir.parent/photos/` — one directory level too high. The photo existed on disk but the API returned 404. Fixed the path resolution in `get_photo`, `upload_photo`, and `delete_photo` to use `base_dir / Path(file_path).parent` (same pattern as the save code), which correctly resolves to `base_dir/photos/` when `file_path` is empty.
+- **Archive Endpoints Crash With "Is a directory" for BambuStudio Prints** ([#475](https://github.com/maziggy/bambuddy/issues/475)) — When a print was started from BambuStudio (not Bambuddy), the 3MF file is transient on the printer and FTP download fails, creating a fallback archive with `file_path=""`. The archive endpoints used `Path.exists()` to check if the 3MF file was available, but `settings.base_dir / ""` resolves to the base directory itself — which `exists()` reports as True. Subsequent `ZipFile()` calls then failed with `[Errno 21] Is a directory`. Replaced all `.exists()` checks on archive file paths with `.is_file()` across 15 locations in the archive routes and 1 in the main module. Also added a `file_path` truthiness guard for finish photo capture to prevent saving photos under the base directory when the archive has no file path.
 - **ntfy Notifications Fail With "Illegal header value"** ([#466](https://github.com/maziggy/bambuddy/issues/466)) — When sending ntfy notifications with image attachments (progress, error events), the message body was placed in an HTTP `Message` header. Multi-line messages (e.g., printer name + remaining time) contain newline characters, which are illegal in HTTP headers. Test notifications worked because they are single-line with no image. Now escapes newlines to literal `\n` in the header, which ntfy interprets and renders as actual line breaks. Additionally, ntfy servers with attachments disabled rejected thumbnail uploads with "attachments not allowed" (HTTP 400 / code 40014), causing the entire notification to fail. Now automatically retries without the image when the server doesn't support attachments.
 - **ntfy Notifications Fail With "Illegal header value"** ([#466](https://github.com/maziggy/bambuddy/issues/466)) — When sending ntfy notifications with image attachments (progress, error events), the message body was placed in an HTTP `Message` header. Multi-line messages (e.g., printer name + remaining time) contain newline characters, which are illegal in HTTP headers. Test notifications worked because they are single-line with no image. Now escapes newlines to literal `\n` in the header, which ntfy interprets and renders as actual line breaks. Additionally, ntfy servers with attachments disabled rejected thumbnail uploads with "attachments not allowed" (HTTP 400 / code 40014), causing the entire notification to fail. Now automatically retries without the image when the server doesn't support attachments.
 - **Inventory Date Format Ignores Settings** ([#463](https://github.com/maziggy/bambuddy/issues/463)) — The inventory page used a local `formatDate()` that hardcoded the `en-GB` locale, always displaying dates in a fixed format regardless of the date format setting. Now fetches the `date_format` setting and uses the shared `formatDateInput()` utility which formats as MM/DD/YYYY, DD/MM/YYYY, YYYY-MM-DD, or browser locale based on the user's choice.
 - **Inventory Date Format Ignores Settings** ([#463](https://github.com/maziggy/bambuddy/issues/463)) — The inventory page used a local `formatDate()` that hardcoded the `en-GB` locale, always displaying dates in a fixed format regardless of the date format setting. Now fetches the `date_format` setting and uses the shared `formatDateInput()` utility which formats as MM/DD/YYYY, DD/MM/YYYY, YYYY-MM-DD, or browser locale based on the user's choice.
 - **Inventory Location Shows Garbled Characters for AMS-HT Slots** ([#463](https://github.com/maziggy/bambuddy/issues/463)) — The inventory location column computed slot letters via `String.fromCharCode(65 + ams_id)`, which produced accented characters (e.g., `Á`) for AMS-HT units (ams_id ≥ 128). Now uses the shared `formatSlotLabel()` utility which correctly handles AMS-HT and external spool slots.
 - **Inventory Location Shows Garbled Characters for AMS-HT Slots** ([#463](https://github.com/maziggy/bambuddy/issues/463)) — The inventory location column computed slot letters via `String.fromCharCode(65 + ams_id)`, which produced accented characters (e.g., `Á`) for AMS-HT units (ams_id ≥ 128). Now uses the shared `formatSlotLabel()` utility which correctly handles AMS-HT and external spool slots.

+ 15 - 15
backend/app/api/routes/archives.py

@@ -829,7 +829,7 @@ async def rescan_archive(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     # Parse the 3MF file
     # Parse the 3MF file
@@ -966,7 +966,7 @@ async def rescan_all_archives(
     for archive in archives:
     for archive in archives:
         try:
         try:
             file_path = settings.base_dir / archive.file_path
             file_path = settings.base_dir / archive.file_path
-            if not file_path.exists():
+            if not file_path.is_file():
                 errors.append({"id": archive.id, "error": "File not found"})
                 errors.append({"id": archive.id, "error": "File not found"})
                 continue
                 continue
 
 
@@ -1036,7 +1036,7 @@ async def backfill_content_hashes(
     for archive in archives:
     for archive in archives:
         try:
         try:
             file_path = settings.base_dir / archive.file_path
             file_path = settings.base_dir / archive.file_path
-            if not file_path.exists():
+            if not file_path.is_file():
                 errors.append({"id": archive.id, "error": "File not found"})
                 errors.append({"id": archive.id, "error": "File not found"})
                 continue
                 continue
 
 
@@ -1095,7 +1095,7 @@ async def download_archive(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     # Use inline disposition to let browser/OS handle file association
     # Use inline disposition to let browser/OS handle file association
@@ -1123,7 +1123,7 @@ async def download_archive_with_filename(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     return FileResponse(
     return FileResponse(
@@ -1179,7 +1179,7 @@ async def download_archive_for_slicer(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     return FileResponse(
     return FileResponse(
@@ -1990,7 +1990,7 @@ async def get_archive_capabilities(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     has_model = False
     has_model = False
@@ -2208,7 +2208,7 @@ async def get_gcode(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     try:
     try:
@@ -2250,7 +2250,7 @@ async def get_plate_preview(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "File not found")
         raise HTTPException(404, "File not found")
 
 
     try:
     try:
@@ -2411,7 +2411,7 @@ async def get_archive_plates(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     plates = []
     plates = []
@@ -2679,7 +2679,7 @@ async def get_plate_thumbnail(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     try:
     try:
@@ -2718,7 +2718,7 @@ async def get_filament_requirements(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     filaments = []
     filaments = []
@@ -2938,7 +2938,7 @@ async def get_project_page(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     parser = ProjectPageParser(file_path)
     parser = ProjectPageParser(file_path)
@@ -2963,7 +2963,7 @@ async def update_project_page(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     parser = ProjectPageParser(file_path)
     parser = ProjectPageParser(file_path)
@@ -2995,7 +2995,7 @@ async def get_project_image(
         raise HTTPException(404, "Archive not found")
         raise HTTPException(404, "Archive not found")
 
 
     file_path = settings.base_dir / archive.file_path
     file_path = settings.base_dir / archive.file_path
-    if not file_path.exists():
+    if not file_path.is_file():
         raise HTTPException(404, "Archive file not found")
         raise HTTPException(404, "Archive file not found")
 
 
     parser = ProjectPageParser(file_path)
     parser = ProjectPageParser(file_path)

+ 2 - 2
backend/app/main.py

@@ -1011,7 +1011,7 @@ def _load_objects_from_archive(archive, printer_id: int, logger) -> None:
         from backend.app.services.archive import extract_printable_objects_from_3mf
         from backend.app.services.archive import extract_printable_objects_from_3mf
 
 
         file_path = app_settings.base_dir / archive.file_path
         file_path = app_settings.base_dir / archive.file_path
-        if file_path.exists() and str(file_path).endswith(".3mf"):
+        if file_path.is_file() and str(file_path).endswith(".3mf"):
             with open(file_path, "rb") as f:
             with open(file_path, "rb") as f:
                 threemf_data = f.read()
                 threemf_data = f.read()
             # Extract with positions for UI overlay
             # Extract with positions for UI overlay
@@ -2499,7 +2499,7 @@ async def on_print_complete(printer_id: int, data: dict):
                         result = await db.execute(select(PrintArchive).where(PrintArchive.id == archive_id))
                         result = await db.execute(select(PrintArchive).where(PrintArchive.id == archive_id))
                         archive = result.scalar_one_or_none()
                         archive = result.scalar_one_or_none()
 
 
-                        if archive:
+                        if archive and archive.file_path:
                             import uuid
                             import uuid
                             from datetime import datetime
                             from datetime import datetime
                             from pathlib import Path
                             from pathlib import Path

+ 92 - 0
backend/tests/unit/test_archive_file_path_guard.py

@@ -0,0 +1,92 @@
+"""Tests for archive file_path guard against empty paths and directories (#475).
+
+When a 3mf download fails (e.g. BambuStudio-initiated prints), the fallback
+archive is created with file_path="". Previously, `settings.base_dir / ""`
+resolved to the base directory itself, which passed `exists()` but caused
+`[Errno 21] Is a directory` when opened as a ZipFile.
+
+The fix replaces `.exists()` with `.is_file()` across all archive endpoints,
+and adds an `archive.file_path` truthiness check for photo capture.
+"""
+
+from pathlib import Path
+
+import pytest
+
+
+class TestIsFileGuard:
+    """Verify that is_file() correctly rejects directories and empty paths."""
+
+    def test_empty_path_resolves_to_parent(self, tmp_path: Path):
+        """Path('') / '' resolves to the parent directory (which exists but is not a file)."""
+        base_dir = tmp_path / "data"
+        base_dir.mkdir()
+
+        file_path = base_dir / ""
+        # exists() returns True (the directory exists) — this was the old broken check
+        assert file_path.exists()
+        # is_file() returns False (it's a directory, not a file)
+        assert not file_path.is_file()
+
+    def test_real_file_passes_is_file(self, tmp_path: Path):
+        """A real 3mf file passes is_file()."""
+        fake_3mf = tmp_path / "archive" / "test.3mf"
+        fake_3mf.parent.mkdir(parents=True)
+        fake_3mf.write_bytes(b"PK\x03\x04")  # ZIP magic bytes
+
+        assert fake_3mf.is_file()
+
+    def test_nonexistent_file_fails_is_file(self, tmp_path: Path):
+        """A nonexistent path fails is_file()."""
+        missing = tmp_path / "archive" / "missing.3mf"
+        assert not missing.is_file()
+
+    def test_directory_fails_is_file(self, tmp_path: Path):
+        """A directory path fails is_file()."""
+        dir_path = tmp_path / "archive"
+        dir_path.mkdir()
+        assert not dir_path.is_file()
+
+
+class TestFallbackArchiveFilePath:
+    """Verify that a fallback archive (file_path='') is handled safely."""
+
+    def test_base_dir_slash_empty_string_is_base_dir(self, tmp_path: Path):
+        """Joining base_dir with empty string produces base_dir (a directory)."""
+        base_dir = tmp_path / "data"
+        base_dir.mkdir()
+
+        # Simulate: file_path = settings.base_dir / archive.file_path
+        # where archive.file_path = ""
+        file_path = base_dir / ""
+
+        # The resolved path IS the directory itself
+        assert file_path.resolve() == base_dir.resolve()
+        # exists() says True (this caused the old bug)
+        assert file_path.exists()
+        # is_file() says False (this is the fix)
+        assert not file_path.is_file()
+
+    def test_archive_file_path_empty_string_is_falsy(self):
+        """Empty string file_path is falsy (used for photo capture guard)."""
+        file_path = ""
+        assert not file_path
+
+    def test_archive_file_path_real_is_truthy(self):
+        """Real file_path is truthy."""
+        file_path = "archive/2026/02/test.3mf"
+        assert file_path
+
+
+class TestPhotoPathDerivation:
+    """Verify that photo directory derivation is safe with empty file_path."""
+
+    def test_empty_file_path_parent_is_dot(self):
+        """Path('').parent is '.' — would resolve to base_dir instead of archive dir."""
+        parent = Path("").parent
+        assert str(parent) == "."
+
+    def test_real_file_path_parent_is_archive_dir(self):
+        """Real file_path parent gives the correct archive directory."""
+        parent = Path("archive/2026/02/test.3mf").parent
+        assert str(parent) == "archive/2026/02"