Sfoglia il codice sorgente

fix(vp): stop uvloop from silently truncating VP FTP uploads (#1896)

    Native (non-Docker) installs launched uvicorn without --loop asyncio, so
    uvicorn[standard] auto-selected uvloop. uvloop's SSL layer drops
    already-received but still-buffered data when the client closes the data
    connection without a TLS close_notify while the reader is flow-control
    paused on slow storage. cmd_STOR writes each chunk to disk inside the read
    loop, so a slow consumer falls behind, the tail is lost, read() returns a
    clean EOF, and the loop exits with no exception -- the server acked 226 for
    a file it truncated itself, then archived, queued, and forwarded the corrupt
    3MF to the real printer.

    Fix in two independent layers:

    1. Remove the trigger: add --loop asyncio to every native launch path,
       matching the Dockerfile -- deploy/bambuddy.service, install/install.sh
       (systemd + launchd), spoolbuddy/install/install.sh, the Windows NSSM
       service, README, CONTRIBUTING dev command.

    2. Defense in depth (loop-independent): cmd_STOR now validates that a
       received .3mf opens as a ZIP (reads the central directory, no
       decompression) before replying 226. A truncated/corrupt file is dropped
       and answered with 426, and on_file_received never runs -- so a broken
       upload surfaces as an immediate slicer-side send error instead of being
       archived and pushed to the printer. Scoped to .3mf; other filetypes pass
       through unchanged.
maziggy 2 mesi fa
parent
commit
6e03ecdb8d

+ 3 - 2
CONTRIBUTING.md

@@ -117,8 +117,9 @@ pip install -r requirements-dev.txt  # Dev/test dependencies (pytest, ruff, band
 pip install pre-commit
 pip install pre-commit
 pre-commit install
 pre-commit install
 
 
-# Run backend
-DEBUG=true uvicorn backend.app.main:app --reload --host 0.0.0.0 --port 8000
+# Run backend (--loop asyncio matches production; avoids a uvloop TLS bug
+# that can truncate Virtual Printer FTP uploads on slow storage — see #1896)
+DEBUG=true uvicorn backend.app.main:app --reload --host 0.0.0.0 --port 8000 --loop asyncio
 ```
 ```
 
 
 ### Frontend Setup
 ### Frontend Setup

+ 2 - 2
README.md

@@ -665,8 +665,8 @@ python3 -m venv venv
 source venv/bin/activate
 source venv/bin/activate
 pip install -r requirements.txt
 pip install -r requirements.txt
 
 
-# Run
-uvicorn backend.app.main:app --host 0.0.0.0 --port 8000
+# Run (--loop asyncio avoids a uvloop TLS bug that can truncate VP FTP uploads)
+uvicorn backend.app.main:app --host 0.0.0.0 --port 8000 --loop asyncio
 ```
 ```
 
 
 Open **http://localhost:8000** and add your printer!
 Open **http://localhost:8000** and add your printer!

+ 38 - 0
backend/app/services/virtual_printer/ftp_server.py

@@ -13,6 +13,7 @@ import logging
 import os
 import os
 import random
 import random
 import ssl
 import ssl
+import zipfile
 from collections.abc import Callable
 from collections.abc import Callable
 from pathlib import Path
 from pathlib import Path
 
 
@@ -476,6 +477,43 @@ class FTPSession:
             await self.send(426, f"Transfer failed: {write_failed}")
             await self.send(426, f"Transfer failed: {write_failed}")
             return
             return
 
 
+        # Defense in depth (#1896): a clean read-loop EOF does NOT prove the
+        # upload arrived intact. Under uvloop, the SSL layer can silently drop
+        # already-received but still-buffered data when the client closes the
+        # data connection without a TLS close_notify (a "ragged EOF") while the
+        # transport is flow-control-paused on slow storage — read() then returns
+        # b"" and we would otherwise reply 226 for a tail-truncated file, archive
+        # it, queue it, and forward the corrupt job to the real printer.
+        #
+        # Bambu 3MF uploads are ZIP containers whose End-Of-Central-Directory
+        # record sits at the very end of the file, so any lost tail makes the
+        # archive impossible to open. Verify that before acknowledging success:
+        # a truncated file is treated exactly like a failed transfer (426 +
+        # drop) so the slicer surfaces an actionable send error instead of the
+        # printer choking on a half-written job later. Only ZIP-based (.3mf)
+        # uploads are validated — other filetypes keep the prior pass-through
+        # behaviour. Reading the central directory is O(dir), not O(file): no
+        # decompression, negligible next to the write loop above.
+        if filename.lower().endswith(".3mf"):
+            try:
+                with zipfile.ZipFile(file_path) as zf:
+                    zf.namelist()
+            except Exception as e:
+                logger.error(
+                    "FTP upload of %s is a corrupt/truncated 3MF (%s bytes): %s(%s) — "
+                    "rejecting with 426 instead of archiving a broken file",
+                    filename,
+                    total_received,
+                    type(e).__name__,
+                    e,
+                )
+                try:
+                    file_path.unlink(missing_ok=True)
+                except OSError:
+                    pass
+                await self.send(426, "Transfer failed: uploaded 3MF is incomplete or corrupt")
+                return
+
         # Confirm + notify
         # Confirm + notify
         logger.info("FTP saved file: %s (%s bytes)", file_path, total_received)
         logger.info("FTP saved file: %s (%s bytes)", file_path, total_received)
         await self.send(226, "Transfer complete")
         await self.send(226, "Transfer complete")

+ 68 - 1
backend/tests/unit/test_vp_ftp_stor.py

@@ -10,7 +10,9 @@ up a real TLS/FTP server.
 """
 """
 
 
 import asyncio
 import asyncio
+import io
 import ssl
 import ssl
+import zipfile
 from unittest.mock import AsyncMock, MagicMock
 from unittest.mock import AsyncMock, MagicMock
 
 
 import pytest
 import pytest
@@ -18,6 +20,22 @@ import pytest
 from backend.app.services.virtual_printer.ftp_server import MAX_UPLOAD_BYTES, FTPSession
 from backend.app.services.virtual_printer.ftp_server import MAX_UPLOAD_BYTES, FTPSession
 
 
 
 
+def _valid_3mf_bytes() -> bytes:
+    """A minimal but structurally valid ZIP (stands in for a .gcode.3mf).
+
+    Bambu 3MF files are ZIP containers; the streaming STOR path validates the
+    received file opens as a ZIP before acking 226 (#1896), so happy-path
+    tests must feed real ZIP bytes rather than arbitrary filler.
+    """
+    buf = io.BytesIO()
+    with zipfile.ZipFile(buf, "w") as zf:
+        zf.writestr("Metadata/slice_info.config", "<config/>")
+        zf.writestr("3D/3dmodel.model", "<model/>")
+        # Pad an entry so the archive spans several 64 KiB read chunks.
+        zf.writestr("plate_1.gcode", b"G1 X0 Y0\n" * 40000)
+    return buf.getvalue()
+
+
 def _make_session(tmp_path, *, data_chunks: list[bytes]) -> FTPSession:
 def _make_session(tmp_path, *, data_chunks: list[bytes]) -> FTPSession:
     """Build an FTPSession primed with a pre-fed StreamReader so cmd_STOR
     """Build an FTPSession primed with a pre-fed StreamReader so cmd_STOR
     can iterate through the chunks without a real TCP connection.
     can iterate through the chunks without a real TCP connection.
@@ -63,8 +81,9 @@ def _make_session(tmp_path, *, data_chunks: list[bytes]) -> FTPSession:
 async def test_stor_writes_payload_to_disk(tmp_path):
 async def test_stor_writes_payload_to_disk(tmp_path):
     """Happy path: chunks fed to the data reader land in the upload_dir
     """Happy path: chunks fed to the data reader land in the upload_dir
     with the right content + the slicer gets 226."""
     with the right content + the slicer gets 226."""
-    payload = b"X" * (3 * 64 * 1024 + 123)  # 3 chunks + a partial one
+    payload = _valid_3mf_bytes()  # spans several 64 KiB chunks, opens as ZIP
     chunks = [payload[i : i + 65536] for i in range(0, len(payload), 65536)]
     chunks = [payload[i : i + 65536] for i in range(0, len(payload), 65536)]
+    assert len(chunks) > 3  # exercise the multi-chunk read loop
     session = _make_session(tmp_path, data_chunks=chunks)
     session = _make_session(tmp_path, data_chunks=chunks)
     session.send = AsyncMock()
     session.send = AsyncMock()
 
 
@@ -78,6 +97,54 @@ async def test_stor_writes_payload_to_disk(tmp_path):
     sent_codes = [args[0][0] for args in session.send.call_args_list]
     sent_codes = [args[0][0] for args in session.send.call_args_list]
     assert 150 in sent_codes  # "Opening data connection"
     assert 150 in sent_codes  # "Opening data connection"
     assert 226 in sent_codes  # "Transfer complete"
     assert 226 in sent_codes  # "Transfer complete"
+    assert 426 not in sent_codes
+
+
+@pytest.mark.asyncio
+async def test_stor_rejects_truncated_3mf(tmp_path):
+    """#1896: a .3mf whose tail was lost (uvloop ragged-EOF data loss, or any
+    other silent truncation) must NOT be acked with 226 — the read loop sees a
+    clean EOF and no write error, so only a ZIP-integrity check catches it.
+    Reject with 426, drop the file, and never fire the on_file_received
+    callback that would archive/queue/forward the corrupt job."""
+    payload = _valid_3mf_bytes()
+    truncated = payload[: len(payload) - 4096]  # drop the EOCD-bearing tail
+    chunks = [truncated[i : i + 65536] for i in range(0, len(truncated), 65536)]
+
+    callback = AsyncMock()
+    session = _make_session(tmp_path, data_chunks=chunks)
+    session.on_file_received = callback
+    session.send = AsyncMock()
+
+    await session.cmd_STOR("truncated.gcode.3mf")
+
+    # Corrupt file dropped, not left in the upload dir.
+    assert not (session.upload_dir / "truncated.gcode.3mf").exists()
+    sent_codes = [args[0][0] for args in session.send.call_args_list]
+    assert 426 in sent_codes
+    assert 226 not in sent_codes
+    # The archive/queue/forward callback must never run for a corrupt upload.
+    callback.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_stor_skips_zip_validation_for_non_3mf(tmp_path):
+    """The ZIP-integrity gate is scoped to .3mf uploads. A non-3MF file (e.g.
+    a plain .gcode some slicers still send) is not a ZIP and must keep the
+    prior pass-through behaviour — 226, not a false-positive 426."""
+    payload = b"G1 X0 Y0\n" * 5000  # plain text, deliberately not a ZIP
+    chunks = [payload[i : i + 65536] for i in range(0, len(payload), 65536)]
+    session = _make_session(tmp_path, data_chunks=chunks)
+    session.send = AsyncMock()
+
+    await session.cmd_STOR("plain.gcode")
+
+    saved = session.upload_dir / "plain.gcode"
+    assert saved.exists()
+    assert saved.read_bytes() == payload
+    sent_codes = [args[0][0] for args in session.send.call_args_list]
+    assert 226 in sent_codes
+    assert 426 not in sent_codes
 
 
 
 
 @pytest.mark.asyncio
 @pytest.mark.asyncio

+ 3 - 1
deploy/bambuddy.service

@@ -32,7 +32,9 @@ EnvironmentFile=-INSTALL_PATH/.env
 Environment="PATH=INSTALL_PATH/venv/bin:/usr/local/bin:/usr/bin:/bin"
 Environment="PATH=INSTALL_PATH/venv/bin:/usr/local/bin:/usr/bin:/bin"
 
 
 # Server configuration
 # Server configuration
-ExecStart=INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host 0.0.0.0 --port ${PORT:-8000}
+# --loop asyncio is required: uvloop's SSL layer can silently truncate VP FTP
+# uploads on a ragged client close over slow storage (#1896). Do not remove.
+ExecStart=INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host 0.0.0.0 --port ${PORT:-8000} --loop asyncio
 
 
 # Restart policy
 # Restart policy
 Restart=on-failure
 Restart=on-failure

+ 5 - 1
install/install.sh

@@ -548,7 +548,8 @@ Environment="DATA_DIR=$DATA_DIR"
 Environment="LOG_DIR=$LOG_DIR"
 Environment="LOG_DIR=$LOG_DIR"
 Environment="TZ=$TIMEZONE"
 Environment="TZ=$TIMEZONE"
 
 
-ExecStart=$INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host $BIND_ADDRESS --port $PORT
+# --loop asyncio required: uvloop can truncate VP FTP uploads (#1896)
+ExecStart=$INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host $BIND_ADDRESS --port $PORT --loop asyncio
 Restart=on-failure
 Restart=on-failure
 RestartSec=5
 RestartSec=5
 StandardOutput=journal
 StandardOutput=journal
@@ -613,6 +614,9 @@ create_launchd_service() {
         <string>$BIND_ADDRESS</string>
         <string>$BIND_ADDRESS</string>
         <string>--port</string>
         <string>--port</string>
         <string>$PORT</string>
         <string>$PORT</string>
+        <!-- the loop asyncio flag below is required: uvloop can truncate VP FTP uploads, #1896 -->
+        <string>--loop</string>
+        <string>asyncio</string>
     </array>
     </array>
     <key>WorkingDirectory</key>
     <key>WorkingDirectory</key>
     <string>$INSTALL_PATH</string>
     <string>$INSTALL_PATH</string>

+ 1 - 1
installers/windows/README.md

@@ -10,7 +10,7 @@ service. No Python or Node installation required on the target machine.
 - **Data target:** `C:\ProgramData\Bambuddy\data\` (preserved on uninstall by default)
 - **Data target:** `C:\ProgramData\Bambuddy\data\` (preserved on uninstall by default)
 - **Logs target:** `C:\ProgramData\Bambuddy\logs\`
 - **Logs target:** `C:\ProgramData\Bambuddy\logs\`
 - **Service:** registered via NSSM, runs as `LocalSystem`, autostart on boot
 - **Service:** registered via NSSM, runs as `LocalSystem`, autostart on boot
-- **Service command:** `python.exe -m uvicorn backend.app.main:app --host 0.0.0.0 --port 8000`
+- **Service command:** `python.exe -m uvicorn backend.app.main:app --host 0.0.0.0 --port 8000 --loop asyncio` (`--loop asyncio` avoids a uvloop TLS bug that can truncate VP FTP uploads, #1896)
 - **Bundled binaries:** Python 3.13 embeddable, NSSM, ffmpeg static build
 - **Bundled binaries:** Python 3.13 embeddable, NSSM, ffmpeg static build
 
 
 Browser is the UI. Start Menu shortcut opens `http://localhost:8000`.
 Browser is the UI. Start Menu shortcut opens `http://localhost:8000`.

+ 2 - 1
installers/windows/service/install-service.bat

@@ -29,7 +29,8 @@ REM "service not found" returns non-zero and we want to proceed.
 
 
 REM Register the service. NSSM wraps uvicorn so Windows treats it as a
 REM Register the service. NSSM wraps uvicorn so Windows treats it as a
 REM proper service (autostart, recovery, supervised restart).
 REM proper service (autostart, recovery, supervised restart).
-"%NSSM%" install Bambuddy "%PYTHON%" "-m uvicorn backend.app.main:app --host 0.0.0.0 --port %PORT%"
+REM --loop asyncio required: uvloop can truncate VP FTP uploads (#1896).
+"%NSSM%" install Bambuddy "%PYTHON%" "-m uvicorn backend.app.main:app --host 0.0.0.0 --port %PORT% --loop asyncio"
 if errorlevel 1 (
 if errorlevel 1 (
     echo [install-service] nssm install failed
     echo [install-service] nssm install failed
     exit /b 1
     exit /b 1

+ 2 - 1
spoolbuddy/install/install.sh

@@ -770,7 +770,8 @@ WorkingDirectory=$INSTALL_PATH
 EnvironmentFile=$INSTALL_PATH/.env
 EnvironmentFile=$INSTALL_PATH/.env
 Environment="DATA_DIR=$INSTALL_PATH/data"
 Environment="DATA_DIR=$INSTALL_PATH/data"
 Environment="LOG_DIR=$INSTALL_PATH/logs"
 Environment="LOG_DIR=$INSTALL_PATH/logs"
-ExecStart=$INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host 0.0.0.0 --port $BAMBUDDY_PORT
+# --loop asyncio required: uvloop can truncate VP FTP uploads (#1896)
+ExecStart=$INSTALL_PATH/venv/bin/uvicorn backend.app.main:app --host 0.0.0.0 --port $BAMBUDDY_PORT --loop asyncio
 Restart=on-failure
 Restart=on-failure
 RestartSec=5
 RestartSec=5
 StandardOutput=journal
 StandardOutput=journal