Browse Source

fix(uploads): pre-flight validation for 3MF/gcode + visible upload errors (#1401)

  Reporter @iitazz uploaded slicer output to Bambuddy, clicked Print,
  and the printer rejected every job with "Printing stopped because
  the printer was unable to parse the 3mf file". Support bundle showed
  the stored library file ended in .gcode (not .gcode.3mf), and
  background_dispatch.py appends ".3mf" to filenames that don't
  already end in .gcode.3mf/.3mf — so raw gcode shipped to the printer
  named .gcode.3mf and the firmware's 3MF parser choked. Same shape
  also surfaced as "File is not a zip file" on Bambuddy's own plate
  parser.

  New validate_print_file_upload() helper in library.py runs at upload
  time:
    - Reject filenames ending in .gcode (but not .gcode.3mf) with a
      clear message — Bambu printers need .gcode.3mf zip containers,
      not raw gcode.
    - For .3mf / .gcode.3mf uploads, verify body starts with PK\x03\x04
      (ZIP magic); reject otherwise pointing at the slicer's "Export
      Plate Sliced File" action.

  Applied to every relevant upload route: POST /library/files (covers
  File Manager + printer-card drag-drop), POST /archives/upload,
  POST /archives/upload-bulk (rejects per-row so one bad file doesn't
  abort the batch), POST /archives/{id}/source, POST /archives/upload-source.
  Runs after _resolve_upload_destination so folder-permission errors
  (403 readonly, 400 missing-path, 409 collision) still take precedence.
  STL / image / other non-print uploads bypass the validator.

  FileUploadModal frontend fix: the modal auto-closed after every
  batch regardless of per-file results, so a 400 rejection was captured
  but invisible. Now:
    - Errors render inline as red text under the file row instead of
      as a hover-only title tooltip.
    - Modal stays open if any file ended with status='error', so the
      user can read the backend's remediation message before closing.
    - Successful-only batches still auto-close as before.

  UploadModal (bulk archive) was already showing inline errors and
  not auto-closing — no change needed there.
maziggy 1 week ago
parent
commit
bff240e90a

File diff suppressed because it is too large
+ 1 - 0
CHANGELOG.md


+ 29 - 0
backend/app/api/routes/archives.py

@@ -2911,6 +2911,12 @@ async def upload_archive(
 
 
     try:
     try:
         content = await file.read()
         content = await file.read()
+        # #1401: same content validation as library upload — catches
+        # raw-gcode-renamed-to-.3mf and other unprintable shapes before
+        # archiving them and offering them up for print.
+        from backend.app.api.routes.library import validate_print_file_upload
+
+        validate_print_file_upload(file.filename, content)
         temp_path.write_bytes(content)
         temp_path.write_bytes(content)
 
 
         service = ArchiveService(db)
         service = ArchiveService(db)
@@ -2937,6 +2943,8 @@ async def upload_archives_bulk(
     current_user: User | None = RequirePermissionIfAuthEnabled(Permission.ARCHIVES_CREATE),
     current_user: User | None = RequirePermissionIfAuthEnabled(Permission.ARCHIVES_CREATE),
 ):
 ):
     """Bulk upload multiple 3MF files to archive."""
     """Bulk upload multiple 3MF files to archive."""
+    from backend.app.api.routes.library import validate_print_file_upload
+
     results = []
     results = []
     errors = []
     errors = []
 
 
@@ -2951,6 +2959,15 @@ async def upload_archives_bulk(
 
 
         try:
         try:
             content = await file.read()
             content = await file.read()
+            # #1401: bulk-upload variant of the library validation. Collect
+            # the rejection per-file rather than aborting the whole batch
+            # so one bad file in a 10-file drag-drop doesn't lose the
+            # other nine.
+            try:
+                validate_print_file_upload(file.filename, content)
+            except HTTPException as exc:
+                errors.append({"filename": file.filename, "error": exc.detail})
+                continue
             temp_path.write_bytes(content)
             temp_path.write_bytes(content)
 
 
             service = ArchiveService(db)
             service = ArchiveService(db)
@@ -3864,6 +3881,12 @@ async def upload_source_3mf(
     source_path = source_dir / source_filename
     source_path = source_dir / source_filename
 
 
     content = await file.read()
     content = await file.read()
+    # #1401: validate zip header on source 3MF uploads too — source files
+    # are uploaded for reprint and slicing, so an invalid one breaks the
+    # same downstream paths as a bad sliced file.
+    from backend.app.api.routes.library import validate_print_file_upload
+
+    validate_print_file_upload(file.filename, content)
     source_path.write_bytes(content)
     source_path.write_bytes(content)
 
 
     # Update archive with source path (relative to base_dir)
     # Update archive with source path (relative to base_dir)
@@ -4065,6 +4088,12 @@ async def upload_source_3mf_by_name(
     source_path = source_dir / source_filename
     source_path = source_dir / source_filename
 
 
     content = await file.read()
     content = await file.read()
+    # #1401: same zip-header check as the other upload routes — the
+    # match-by-name endpoint is used by slicer post-processing scripts,
+    # so a misconfigured script is exactly how a bad 3MF would slip in.
+    from backend.app.api.routes.library import validate_print_file_upload
+
+    validate_print_file_upload(file.filename, content)
     source_path.write_bytes(content)
     source_path.write_bytes(content)
 
 
     # Update archive with source path
     # Update archive with source path

+ 59 - 2
backend/app/api/routes/library.py

@@ -139,6 +139,55 @@ def calculate_file_hash(file_path: Path) -> str:
     return sha256_hash.hexdigest()
     return sha256_hash.hexdigest()
 
 
 
 
+def validate_print_file_upload(filename: str, content: bytes) -> None:
+    """Reject obviously-unprintable uploads early so the printer doesn't see them (#1401).
+
+    Bambu printers in network mode only parse ``.gcode.3mf`` zip containers
+    — raw ``.gcode`` and corrupt/non-zip ``.3mf`` uploads cascade into a
+    confusing "Printing stopped because the printer was unable to parse the
+    3mf file" rejection 30 seconds after the user clicks Print. The
+    background dispatcher (``background_dispatch.py``) appends ``.3mf`` to
+    a raw-gcode filename when constructing the FTP destination, which is
+    how the printer ends up with a file named ``.gcode.3mf`` whose body is
+    raw gcode — exactly the shape that triggers the firmware parse
+    failure. Catching both classes here gives an actionable error at the
+    upload itself.
+
+    Compares the filename suffix rather than ``os.path.splitext`` because
+    compound extensions like ``.gcode.3mf`` show up as just ``.3mf`` after
+    ``splitext`` — same content validation needs to fire for both
+    single-``.3mf`` and ``.gcode.3mf`` uploads.
+
+    Raises ``HTTPException(400, ...)`` with a human-readable message on
+    rejection; returns ``None`` for valid (or irrelevant — e.g. STL,
+    image) uploads.
+    """
+    lower_filename = filename.lower()
+    is_3mf_upload = lower_filename.endswith(".3mf")
+    is_raw_gcode_upload = lower_filename.endswith(".gcode") and not lower_filename.endswith(".gcode.3mf")
+
+    if is_raw_gcode_upload:
+        raise HTTPException(
+            status_code=400,
+            detail=(
+                "Raw .gcode files can't be printed on Bambu printers in network mode — "
+                "they need a .gcode.3mf zip container (gcode plus metadata). Re-export from "
+                "your slicer and make sure the file ends in '.gcode.3mf', not just '.gcode'. "
+                "If your OS hides extensions, double-check the file with the extension visible."
+            ),
+        )
+
+    if is_3mf_upload and not content.startswith(b"PK\x03\x04"):
+        raise HTTPException(
+            status_code=400,
+            detail=(
+                "This .3mf file isn't a valid ZIP container. 3MF files are ZIP archives — "
+                "either the file is corrupted or it's raw gcode renamed to .3mf. Re-export "
+                "from your slicer using its 'Export Plate Sliced File' action."
+            ),
+        )
+
+
 def _resolve_upload_destination(target_folder: LibraryFolder | None, filename: str) -> tuple[Path, bool]:
 def _resolve_upload_destination(target_folder: LibraryFolder | None, filename: str) -> tuple[Path, bool]:
     """Resolve the on-disk destination for an uploaded file.
     """Resolve the on-disk destination for an uploaded file.
 
 
@@ -1508,11 +1557,19 @@ async def upload_file(
 
 
         # Writable external folders write through to the mount so the file is
         # Writable external folders write through to the mount so the file is
         # visible outside Bambuddy (#1112); everything else lands under the
         # visible outside Bambuddy (#1112); everything else lands under the
-        # internal library dir with a UUID-scoped filename.
+        # internal library dir with a UUID-scoped filename. Resolved BEFORE
+        # the content validation below so folder-permission rejections
+        # (403 read-only, 400 missing path, 409 collision) still surface
+        # before any "bad file format" 400 — preserves existing error
+        # ordering / tests.
         file_path, is_external_upload = _resolve_upload_destination(target_folder, filename)
         file_path, is_external_upload = _resolve_upload_destination(target_folder, filename)
 
 
-        # Save file
+        # Read upload now so the validation can sniff magic bytes; the file
+        # is written to disk only after the checks. #1401.
         content = await file.read()
         content = await file.read()
+        validate_print_file_upload(filename, content)
+
+        # Save file
         with open(file_path, "wb") as f:
         with open(file_path, "wb") as f:
             f.write(content)
             f.write(content)
 
 

+ 11 - 1
backend/tests/integration/test_external_folders_api.py

@@ -593,12 +593,22 @@ class TestExternalFolderWritableUpload:
         """DB row must have ``is_external=True`` and ``file_path`` = absolute external path,
         """DB row must have ``is_external=True`` and ``file_path`` = absolute external path,
         so scan-dedupe and deletion behaviour match scanned files."""
         so scan-dedupe and deletion behaviour match scanned files."""
         import io
         import io
+        import zipfile
 
 
         from backend.app.models.library import LibraryFile
         from backend.app.models.library import LibraryFile
 
 
+        # #1401 hardened the library upload route to reject .3mf files that
+        # aren't valid ZIP containers. This test asserts external-folder
+        # DB shape, not the upload validator, so feed it a minimal real zip
+        # rather than placeholder bytes.
+        zip_buf = io.BytesIO()
+        with zipfile.ZipFile(zip_buf, "w", zipfile.ZIP_DEFLATED) as zf:
+            zf.writestr("placeholder.txt", "")
+        zip_buf.seek(0)
+
         response = await async_client.post(
         response = await async_client.post(
             f"/api/v1/library/files?folder_id={writable_folder['id']}",
             f"/api/v1/library/files?folder_id={writable_folder['id']}",
-            files={"file": ("model.3mf", io.BytesIO(b"x"), "application/octet-stream")},
+            files={"file": ("model.3mf", zip_buf, "application/octet-stream")},
         )
         )
         assert response.status_code == 200
         assert response.status_code == 200
         file_id = response.json()["id"]
         file_id = response.json()["id"]

+ 110 - 0
backend/tests/integration/test_library_api.py

@@ -1112,3 +1112,113 @@ class TestLibraryPermissions:
         )
         )
         # Viewers don't have delete_own or delete_all permissions
         # Viewers don't have delete_own or delete_all permissions
         assert response.status_code == 403
         assert response.status_code == 403
+
+
+class TestPrintFileUploadValidation:
+    """#1401: pre-flight rejection of unprintable uploads at the library +
+    archive routes. Smoke tests the shared ``validate_print_file_upload``
+    helper through both surfaces a user can reach with a drag-drop."""
+
+    def _valid_3mf_bytes(self, name: str = "Metadata/plate_1.gcode") -> bytes:
+        """Build a minimal-but-real zip with the gcode-3mf magic in it so
+        the validator's ``startswith(b"PK\\x03\\x04")`` check passes."""
+        buf = io.BytesIO()
+        with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf:
+            zf.writestr(name, "; G-code\nG28\n")
+        return buf.getvalue()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_rejects_raw_gcode_upload(self, async_client: AsyncClient, db_session):
+        """``Foo.gcode`` direct uploads are blocked at the library route —
+        the dispatcher would otherwise append ``.3mf`` and ship raw gcode
+        to the printer as a fake 3MF."""
+        files = {"file": ("plate_1.gcode", b"; raw gcode\nG28\n", "application/octet-stream")}
+        response = await async_client.post("/api/v1/library/files", files=files)
+        assert response.status_code == 400
+        # Error message must name the actual remedy, not just say "invalid".
+        assert "gcode.3mf" in response.json()["detail"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_rejects_non_zip_3mf_upload(self, async_client: AsyncClient, db_session):
+        """A ``.3mf`` upload whose body isn't a zip is rejected — covers
+        raw gcode renamed to .3mf, corrupted downloads, etc."""
+        files = {"file": ("model.3mf", b"; raw gcode\nG28\n", "application/octet-stream")}
+        response = await async_client.post("/api/v1/library/files", files=files)
+        assert response.status_code == 400
+        assert "ZIP container" in response.json()["detail"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_rejects_non_zip_gcode_3mf_upload(self, async_client: AsyncClient, db_session):
+        """The compound-extension ``.gcode.3mf`` case is gated by the same
+        zip-magic check — splitext returns just ``.3mf``, but the suffix
+        match covers both."""
+        files = {"file": ("plate_1.gcode.3mf", b"; raw gcode\nG28\n", "application/octet-stream")}
+        response = await async_client.post("/api/v1/library/files", files=files)
+        assert response.status_code == 400
+        assert "ZIP container" in response.json()["detail"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_accepts_valid_gcode_3mf_upload(self, async_client: AsyncClient, db_session):
+        """A real ``.gcode.3mf`` zip uploads successfully — the existing
+        happy path is not regressed by the new validation."""
+        files = {
+            "file": (
+                "plate_1.gcode.3mf",
+                self._valid_3mf_bytes(),
+                "application/zip",
+            )
+        }
+        response = await async_client.post("/api/v1/library/files", files=files)
+        assert response.status_code == 200
+        result = response.json()
+        assert result["filename"] == "plate_1.gcode.3mf"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_still_accepts_non_print_extensions(self, async_client: AsyncClient, db_session):
+        """STL / image / other non-print uploads bypass the validator
+        entirely — Bambuddy is also a library, not just a print dispatcher."""
+        files = {"file": ("model.stl", b"solid test\nendsolid test", "application/octet-stream")}
+        response = await async_client.post(
+            "/api/v1/library/files", files=files, params={"generate_stl_thumbnails": "false"}
+        )
+        assert response.status_code == 200
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_archive_upload_rejects_non_zip(self, async_client: AsyncClient, db_session):
+        """``POST /archives/upload`` shares the same validator — covers the
+        manual archive-upload entry point too."""
+        files = {"file": ("model.3mf", b"; raw gcode\nG28\n", "application/octet-stream")}
+        response = await async_client.post("/api/v1/archives/upload", files=files)
+        assert response.status_code == 400
+        assert "ZIP container" in response.json()["detail"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_archive_bulk_upload_collects_per_file_errors(self, async_client: AsyncClient, db_session):
+        """The bulk-archive route reports validation failures per file and
+        continues processing the remaining items — one bad upload in a
+        10-file drag-drop must not abort the whole batch."""
+        good = self._valid_3mf_bytes()
+        bad = b"; raw gcode\nG28\n"
+        # httpx multipart with a list-of-tuples preserves order + same field name.
+        files = [
+            ("files", ("good.3mf", good, "application/zip")),
+            ("files", ("bad.3mf", bad, "application/octet-stream")),
+        ]
+        response = await async_client.post("/api/v1/archives/upload-bulk", files=files)
+        assert response.status_code == 200
+        body = response.json()
+        # The bulk route's archive_print may still reject the "good" file
+        # downstream (no printer match, etc.) — we don't care about that
+        # here; what matters is the bad file lands in `errors` with the
+        # validator's message and the route didn't 500.
+        assert body["failed"] >= 1
+        bad_errors = [e for e in body["errors"] if e["filename"] == "bad.3mf"]
+        assert bad_errors, body
+        assert "ZIP container" in bad_errors[0]["error"]

+ 18 - 1
frontend/src/components/FileUploadModal.tsx

@@ -112,7 +112,17 @@ export function FileUploadModal({ folderId, onClose, onUploadComplete, onFileUpl
 
 
     setIsUploading(false);
     setIsUploading(false);
     onUploadComplete();
     onUploadComplete();
-    onClose();
+    // #1401: don't auto-close if any file ended with an error — the user
+    // needs to see the rejection message (e.g. "raw .gcode upload"), not
+    // have the modal vanish before they can read it. Closing happens via
+    // the X / Close button instead, after the user has seen what failed.
+    setFiles((prev) => {
+      const anyFailed = prev.some((f) => f.status === 'error');
+      if (!anyFailed) {
+        onClose();
+      }
+      return prev;
+    });
   };
   };
 
 
   const addFiles = (newFiles: File[]) => {
   const addFiles = (newFiles: File[]) => {
@@ -287,6 +297,13 @@ export function FileUploadModal({ folderId, onClose, onUploadComplete, onFileUpl
                         <span className="text-green-400 ml-2">• {t('fileManager.filesExtracted', { count: uploadFile.extractedCount })}</span>
                         <span className="text-green-400 ml-2">• {t('fileManager.filesExtracted', { count: uploadFile.extractedCount })}</span>
                       )}
                       )}
                     </p>
                     </p>
+                    {/* #1401: errors render inline rather than as a hover-only
+                        title. The backend's rejection messages explain the
+                        actual fix (re-export as .gcode.3mf) — useless if the
+                        user can't read them. */}
+                    {uploadFile.status === 'error' && uploadFile.error && (
+                      <p className="text-xs text-red-400 mt-1 break-words">{uploadFile.error}</p>
+                    )}
                   </div>
                   </div>
                   {uploadFile.status === 'pending' && (
                   {uploadFile.status === 'pending' && (
                     <button
                     <button

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-CfUZn1UK.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
 
     <!-- Splash screens for iOS -->
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DuEb_u5w.js"></script>
+    <script type="module" crossorigin src="/assets/index-CfUZn1UK.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-Baw5c3Hn.css">
     <link rel="stylesheet" crossorigin href="/assets/index-Baw5c3Hn.css">
   </head>
   </head>
   <body>
   <body>

Some files were not shown because too many files changed in this diff