diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 02083ad31..1590a9893 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -117,8 +117,9 @@ pip install -r requirements-dev.txt # Dev/test dependencies (pytest, ruff, band pip install pre-commit 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 diff --git a/README.md b/README.md index fe26bbf4c..316759718 100644 --- a/README.md +++ b/README.md @@ -665,8 +665,8 @@ python3 -m venv venv source venv/bin/activate 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! diff --git a/backend/app/services/virtual_printer/ftp_server.py b/backend/app/services/virtual_printer/ftp_server.py index d31b4dc06..0a242ccf6 100644 --- a/backend/app/services/virtual_printer/ftp_server.py +++ b/backend/app/services/virtual_printer/ftp_server.py @@ -13,6 +13,7 @@ import logging import os import random import ssl +import zipfile from collections.abc import Callable from pathlib import Path @@ -476,6 +477,43 @@ class FTPSession: await self.send(426, f"Transfer failed: {write_failed}") 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 logger.info("FTP saved file: %s (%s bytes)", file_path, total_received) await self.send(226, "Transfer complete") diff --git a/backend/tests/unit/test_vp_ftp_stor.py b/backend/tests/unit/test_vp_ftp_stor.py index 41ccd8b66..280d4d2f6 100644 --- a/backend/tests/unit/test_vp_ftp_stor.py +++ b/backend/tests/unit/test_vp_ftp_stor.py @@ -10,7 +10,9 @@ up a real TLS/FTP server. """ import asyncio +import io import ssl +import zipfile from unittest.mock import AsyncMock, MagicMock import pytest @@ -18,6 +20,22 @@ import pytest 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", "") + zf.writestr("3D/3dmodel.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: """Build an FTPSession primed with a pre-fed StreamReader so cmd_STOR 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): """Happy path: chunks fed to the data reader land in the upload_dir 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)] + assert len(chunks) > 3 # exercise the multi-chunk read loop session = _make_session(tmp_path, data_chunks=chunks) 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] assert 150 in sent_codes # "Opening data connection" 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 diff --git a/deploy/bambuddy.service b/deploy/bambuddy.service index f679088d1..3daa6f9e8 100644 --- a/deploy/bambuddy.service +++ b/deploy/bambuddy.service @@ -32,7 +32,9 @@ EnvironmentFile=-INSTALL_PATH/.env Environment="PATH=INSTALL_PATH/venv/bin:/usr/local/bin:/usr/bin:/bin" # 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=on-failure diff --git a/install/install.sh b/install/install.sh index 0a2700e10..8d6fb773a 100755 --- a/install/install.sh +++ b/install/install.sh @@ -548,7 +548,8 @@ Environment="DATA_DIR=$DATA_DIR" Environment="LOG_DIR=$LOG_DIR" 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 RestartSec=5 StandardOutput=journal @@ -613,6 +614,9 @@ create_launchd_service() { $BIND_ADDRESS --port $PORT + + --loop + asyncio WorkingDirectory $INSTALL_PATH diff --git a/installers/windows/README.md b/installers/windows/README.md index efaab6949..dcb243da4 100644 --- a/installers/windows/README.md +++ b/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) - **Logs target:** `C:\ProgramData\Bambuddy\logs\` - **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 Browser is the UI. Start Menu shortcut opens `http://localhost:8000`. diff --git a/installers/windows/service/install-service.bat b/installers/windows/service/install-service.bat index eb15f050a..5d8e5b35e 100644 --- a/installers/windows/service/install-service.bat +++ b/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 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 ( echo [install-service] nssm install failed exit /b 1 diff --git a/spoolbuddy/install/install.sh b/spoolbuddy/install/install.sh index f8b5aaf46..a69b230ba 100755 --- a/spoolbuddy/install/install.sh +++ b/spoolbuddy/install/install.sh @@ -770,7 +770,8 @@ WorkingDirectory=$INSTALL_PATH EnvironmentFile=$INSTALL_PATH/.env Environment="DATA_DIR=$INSTALL_PATH/data" 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 RestartSec=5 StandardOutput=journal