From 6f050708da53eefb67779680466b74c1ca0ea212 Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 20 May 2026 08:52:10 +0200 Subject: [PATCH] Fix: cap TLS to v1.2 for P2S FTPS to dodge vsFTPd session-reuse bug (#1401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python 3.13 negotiates TLS 1.3 by default. The P2S firmware 01.02.00.00 vsFTPd build doesn't tolerate TLS 1.3's async session-ticket model on the FTPS data channel — session resumption races, the data channel gets torn down mid-stream, uploads land truncated at a chunk boundary, and the printer replies 426 instead of 226. Visible to the user as "unable to parse 3mf file" 30 s into the print. Capping the SSL context's maximum_version to TLS 1.2 makes session resumption synchronous and uploads complete normally. Follow the per-model pattern established by camera_profiles.py in the #1395 follow-up: add backend/app/services/ftp_profiles.py with a frozen FTPProfile dataclass and a per-model registry. Only P2S (display name + N7 SSDP code) gets the cap today. X1C, H2D, P1S, A1 stay on negotiated TLS 1.3 — the maintainer's dogfooded printers see zero behaviour change. --- CHANGELOG.md | 2 + backend/app/services/bambu_ftp.py | 18 +++- backend/app/services/ftp_profiles.py | 99 +++++++++++++++++++ .../tests/unit/services/test_ftp_profiles.py | 93 +++++++++++++++++ 4 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 backend/app/services/ftp_profiles.py create mode 100644 backend/tests/unit/services/test_ftp_profiles.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b62bd21b0..87c88a6d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased ### Fixed +- **FTP: P2S upload truncates / 426 "Failure reading network stream" on Python 3.13 (#1401, reported and root-caused by @iitazz)** — Reporter on a P2S running firmware 01.02.00.00 saw every Bambuddy-initiated print fail with the printer's on-screen "unable to parse 3mf file" error ~30 s in; downloading the file back off the printer's SD card confirmed it was truncated at exactly 7 × 64 KB (clean chunk-boundary cut). Initial #1417 follow-up tightened our 426 handling so we'd surface upload failures instead of silently dispatching a print of a partial 3MF — but that only stopped Bambuddy from hiding the problem; the actual upload still failed. The reporter then dug into it with Gemini and identified the real cause: Python 3.13's default `ssl.create_default_context()` negotiates TLS 1.3 when both peers support it, but the printer's vsFTPd build implements session reuse on the FTPS data channel against an old OpenSSL that doesn't tolerate TLS 1.3's asynchronous session-ticket model. The control-channel handshake completes, the data channel tries to resume the session, the resumption races, the data channel gets torn down mid-stream — first ~448 KB of bytes already in the TCP buffer land on the SD card, the rest never make it, printer's vsFTPd replies 426 instead of 226. **Fix** caps the SSL context's `maximum_version` to TLS 1.2 so session resumption is synchronous and the upload completes normally. Implementation follows the pattern just established by `camera_profiles.py` in the #1395 follow-up: a new `backend/app/services/ftp_profiles.py` module with an `FTPProfile` frozen dataclass (one field today, `cap_tls_v1_2: bool = False`) and a per-model registry. Default profile keeps the historical TLS-1.3 negotiation; P2S (display name + internal SSDP code N7) overrides with `cap_tls_v1_2=True`. `ImplicitFTP_TLS.__init__` gains a matching `cap_tls_v1_2` kwarg; `BambuFTPClient.connect()` looks up the profile and threads the flag through. **Deliberately scoped to P2S only** — X1C / P1S / H2D installs that work today stay on the negotiated TLS 1.3; flipping a future model to the capped path is a one-line entry in `_PROFILES` when a new reporter surfaces the same symptom. Considered but rejected the reporter's second proposed change (revert manual `transfercmd` + `sendall` back to `storbinary`) — the stated rationale ("raw sendall breaks OpenSSL 3.x framing") is incorrect (CPython's `storbinary` itself uses `sendall` internally; the actual socket-level behaviour is identical), the move to manual `transfercmd` was deliberate to dodge A1 hanging in `storbinary`'s synchronous `voidresp()`, and the #1417 SIZE-check escape for the "data is intact on the SD card despite the 426" race lives in the manual-transfer path — a switch to `storbinary` would lose that protection. **Tests**: 9 new in `test_ftp_profiles.py` (default profile doesn't cap; unknown / empty model falls back; P2S display name and N7 SSDP code both resolve to capped; lookup is case-insensitive; X1C / H2D / P1S / A1 stay uncapped; dataclass is frozen; **integration test pins the wiring** — `ImplicitFTP_TLS(cap_tls_v1_2=True)` actually sets `ssl_context.maximum_version == TLSVersion.TLSv1_2`, guards against a future refactor that drops the profile→context wiring while keeping the registry looking correct). 87 existing `test_bambu_ftp.py` tests still green; ruff clean. + - **Library 3D preview: complex multi-part 3MFs no longer freeze the page (#1412, reported by @anthonyma94)** — Reporter opened the 3D preview on a multi-color parted MakerWorld statue ("Mecha Mewtwo No AMS Multi Color Parted Statue") and the whole Bambuddy UI locked up — modal close button unresponsive, had to kill the tab. Root cause was in `frontend/src/components/ModelViewer.tsx`: the 3MF parse runs entirely on the browser main thread (JSZip extract + DOMParser + `getElementsByTagName('vertex')` / `('triangle')` iteration + `mergeGeometries`), with no yield points between iterations. Bambu Studio's external-component shape (`` per part) compounds this — each component triggers another async file extract + DOM parse + vertex/triangle loop, all chained without surrendering control to the event loop between phases. For trivial models (the towel hook and Bambu scraper the reporter cited as working) the total wall-clock is short enough that the freeze isn't visible; for parted statues with dozens of components and high-poly meshes, the main thread is pegged for tens of seconds → browser shows "page unresponsive" and the close button can't fire. **Stopgap that shipped here** adds explicit `nextTick()` yields (`await new Promise(r => setTimeout(r, 0))`) at four hot spots: every 20 000 vertex iterations, every 20 000 triangle iterations, once per top-level `` iteration, and once per `` iteration. Parse wall-clock is unchanged — these yields don't make parsing faster, they just surrender the main thread back to the browser between batches so the modal can be closed, the page can scroll, and the loading spinner can actually render. Constants live next to the helper at the top of the file with a comment justifying the picked period (~5–10 ms of work per batch — fine-grained enough to keep frames flowing, coarse enough not to drown the loop in setTimeout dispatch overhead). The proper fix for this — moving 3MF parse + geometry build into a Web Worker so the main thread is never touched at all — is a tracked follow-up; this stopgap unblocks Anthony's reproduction case today without the worker refactor risk. The earlier close as `invalid` was a misdiagnosis (initial reading was that 3D preview only works on sliced files, which the reporter correctly disproved with a separate MakerWorld URL); reopened, fixed, lesson noted. **Tests:** 21 existing `ModelViewerModal.test.tsx` tests stay green — the yields are in `parseMeshFromDoc` and `parse3MF` which the tests mock around, and the `nextTick` helper has no observable side effects beyond timing. Frontend build clean. - **Archives: timelapse auto-attach now works for VP-queue / dispatch prints (#1403 follow-up, reported by @pwostran)** — Bambuddy uses a snapshot-diff strategy to pick the right MP4 off the printer's SD card after a print (Bambu printers in LAN-only mode don't sync NTP, so file mtimes are unreliable — `_scan_for_timelapse_with_retries` snapshots existing video filenames at print start and looks for any NEW filename at completion). The baseline-capture call was inline at the bottom of `on_print_start`'s **new-archive branch only** — the **expected-archive branch** (which queue / VP-dispatched / reprinted jobs take, anything registered via `register_expected_print`) exited at its own `return` without ever snapshotting. So queue prints had `_timelapse_baselines[printer_id]` unset; the completion-time scan fell into its "take baseline now" fallback that snapshots the SD card *after* the new MP4 has already landed → the new file sits in the "baseline" set → no diff ever matches → auto-attach silently does nothing. The reporter's `bambuddy-support-20260518-185935.zip` shows the failure verbatim: `Using expected archive 3 for print (skipping duplicate)` at `18:41:10`, `Timelapse was active during print, scheduling auto-scan for archive 3` at `18:58:55`, and `[TIMELAPSE] Archive 3 has no printer, aborting` (a separate bug already fixed by the printer_id-assignment commit in this same train) — and `grep -i baseline` across both his bundles returns zero hits, confirming the snapshot never ran. Fix extracts the inline baseline-capture into `_capture_timelapse_baseline_at_start(printer, printer_id, logger)` and calls it from BOTH branches of `on_print_start` (mirroring the existing site in the new-archive branch with a matching call just before the expected-archive branch's `return`). Helper is best-effort with a `try / except Exception` wrapping `_list_timelapse_videos`, so a transient FTP failure at print-start logs `[TIMELAPSE] Failed to capture baseline at print start: …` and the print proceeds — the completion-time fallback still kicks in (with its known limitation), behaviour matching what the new-archive branch had all along. **Tests:** new `test_expected_archive_path_captures_timelapse_baseline` in the existing `test_print_start_assigns_printer_id_to_vp_archive.py` patches `_list_timelapse_videos` to return two pre-existing videos, runs `on_print_start` through the expected-archive branch, and asserts `_timelapse_baselines[1] == {"earlier_print_a.mp4", "earlier_print_b.mp4"}` — a future refactor that removes the call from one of the branches now fails CI. The existing 2 regressions (`test_expected_archive_path_assigns_printer_id_when_unset`, `test_expected_archive_path_preserves_existing_printer_id`) plus 50 adjacent expected-archive / layer-timelapse / archive-filtering tests still green. The fixture clearing `_expected_prints` etc also clears `_timelapse_baselines` now so test isolation holds. diff --git a/backend/app/services/bambu_ftp.py b/backend/app/services/bambu_ftp.py index 9fb8a3be6..1fd909a31 100644 --- a/backend/app/services/bambu_ftp.py +++ b/backend/app/services/bambu_ftp.py @@ -35,15 +35,20 @@ class ImplicitFTP_TLS(FTP_TLS): A1/A1 Mini printers have issues with SSL on the data channel entirely and timeout waiting for transfer completion. Set skip_session_reuse=True for A1 printers to skip SSL on the data channel (control channel remains encrypted). + + Optionally caps the SSL context's maximum TLS version to v1.2 (P2S firmware + 01.02.00.00 needs this — see :mod:`ftp_profiles` and #1401). """ - def __init__(self, *args, skip_session_reuse: bool = False, **kwargs): + def __init__(self, *args, skip_session_reuse: bool = False, cap_tls_v1_2: bool = False, **kwargs): super().__init__(*args, **kwargs) self._sock = None self.skip_session_reuse = skip_session_reuse self.ssl_context = ssl.create_default_context() self.ssl_context.check_hostname = False self.ssl_context.verify_mode = ssl.CERT_NONE + if cap_tls_v1_2: + self.ssl_context.maximum_version = ssl.TLSVersion.TLSv1_2 def connect(self, host="", port=990, timeout=-999, source_address=None): """Connect to host, wrapping socket in TLS immediately (implicit FTPS).""" @@ -150,11 +155,18 @@ class BambuFTPClient: """Connect to the printer FTP server (implicit FTPS on port 990).""" try: use_prot_c = self._should_use_prot_c() + from backend.app.services.ftp_profiles import get_ftp_profile + + profile = get_ftp_profile(self.printer_model) logger.debug( f"FTP connecting to {self.ip_address}:{self.FTP_PORT} " - f"(timeout={self.timeout}s, model={self.printer_model}, prot_c={use_prot_c})" + f"(timeout={self.timeout}s, model={self.printer_model}, prot_c={use_prot_c}, " + f"cap_tls_v1_2={profile.cap_tls_v1_2})" + ) + self._ftp = ImplicitFTP_TLS( + skip_session_reuse=use_prot_c, + cap_tls_v1_2=profile.cap_tls_v1_2, ) - self._ftp = ImplicitFTP_TLS(skip_session_reuse=use_prot_c) self._ftp.connect(self.ip_address, self.FTP_PORT, timeout=self.timeout) logger.debug("FTP connected, logging in as bblp") self._ftp.login("bblp", self.access_code) diff --git a/backend/app/services/ftp_profiles.py b/backend/app/services/ftp_profiles.py new file mode 100644 index 000000000..0f553d3a0 --- /dev/null +++ b/backend/app/services/ftp_profiles.py @@ -0,0 +1,99 @@ +"""Per-printer-model FTP tuning knobs. + +Mirrors the shape of :mod:`backend.app.services.camera_profiles` — a +small registry of per-model overrides so quirky firmwares can be +tuned without sprinkling ``if model == "X":`` branches through +``bambu_ftp.py``. Adding a new model's quirk is a config edit (an +entry in ``_PROFILES`` plus the alias for its internal SSDP code if +needed), not another hard-coded branch. + +The default profile matches the historical pre-fix behaviour, so +every model that doesn't have an entry here keeps its existing FTP +behaviour byte-for-byte. + +Currently only the TLS-version cap lives here (P2S firmware +01.02.00.00 needs it — see ``cap_tls_v1_2`` below). The A1 +data-channel-plaintext quirk still lives in :class:`BambuFTPClient` +via ``A1_MODELS`` / ``skip_session_reuse``; folding that into a +profile field is a future cleanup, not load-bearing for this fix. +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class FTPProfile: + """Tuning knobs for one printer model's FTP path. + + All defaults reflect the historical behaviour. Models with quirky + firmware override individual fields rather than re-defining the + whole profile. + """ + + # Pin the SSL context's ``maximum_version`` to TLS 1.2. + # + # Python 3.13's default ``ssl.create_default_context()`` negotiates + # TLS 1.3 when both peers support it. The Bambuddy Docker image is + # ``python:3.13-slim-trixie``, so every Docker user gets 1.3 by + # default. Some Bambu printer firmwares (P2S 01.02.00.00 confirmed + # by @iitazz, #1401) implement session reuse on the FTPS data + # channel against an old vsFTPd build that doesn't tolerate TLS + # 1.3's asynchronous session-ticket model: the data channel gets + # torn down mid-stream and the upload aborts with 426 "Failure + # reading network stream" — visible as a clean truncation at a + # chunk boundary (one reporter saw exactly 7 × 64 KB landed on + # the printer). Capping to TLS 1.2 makes session resumption + # synchronous and the upload completes normally. + # + # **Defaults to False** — only applied to printer models where a + # reporter has confirmed the symptom. Existing P1S / X1C / H2D + # installs that work fine today stay on the negotiated TLS 1.3. + # This is deliberately conservative; flipping a printer to the + # capped path is a config edit when a new model surfaces the + # same bug. + cap_tls_v1_2: bool = False + + +# --------------------------------------------------------------------------- +# Profile registry +# --------------------------------------------------------------------------- + +# Default profile = historical behaviour. Used for every model that +# doesn't have an entry in ``_PROFILES``. +DEFAULT_PROFILE = FTPProfile() + +# Per-model overrides. Keys are uppercase display names (e.g. "P2S") +# AFTER alias normalisation, so internal SSDP codes ("N7") resolve via +# ``_MODEL_ALIASES`` below. +_PROFILES: dict[str, FTPProfile] = { + # P2S firmware 01.02.00.00 trips the vsFTPd + TLS 1.3 session-reuse + # bug on the FTPS data channel (#1401, reporter @iitazz). Cap to + # TLS 1.2 so session resumption is synchronous and the upload + # completes. + "P2S": FTPProfile( + cap_tls_v1_2=True, + ), +} + +# SSDP internal codes that should resolve to a display-name profile. +# Mirrors the same map in :mod:`camera_profiles`. +_MODEL_ALIASES: dict[str, str] = { + "N7": "P2S", # P2S internal SSDP code +} + + +def get_ftp_profile(model: str | None) -> FTPProfile: + """Return the :class:`FTPProfile` for *model*, or the default. + + ``model`` can be either a display name (e.g. ``"P2S"``) or an + internal SSDP code (e.g. ``"N7"``). Unknown / missing models fall + back to :data:`DEFAULT_PROFILE` so the FTP path is never blocked + on a missing entry. + """ + if not model: + return DEFAULT_PROFILE + key = model.upper().strip() + key = _MODEL_ALIASES.get(key, key) + return _PROFILES.get(key, DEFAULT_PROFILE) diff --git a/backend/tests/unit/services/test_ftp_profiles.py b/backend/tests/unit/services/test_ftp_profiles.py new file mode 100644 index 000000000..40592fcd1 --- /dev/null +++ b/backend/tests/unit/services/test_ftp_profiles.py @@ -0,0 +1,93 @@ +"""Per-model FTP profile registry (#1401). + +Mirrors ``test_camera_profiles.py`` in shape — the FTP profile module +follows the same pattern. +""" + +import ssl + +from backend.app.services.ftp_profiles import ( + DEFAULT_PROFILE, + FTPProfile, + get_ftp_profile, +) + + +def test_default_profile_does_not_cap_tls(): + """Default profile keeps the historical Python-default TLS negotiation + (typically TLS 1.3 on Python 3.13). Capping would be a silent + regression for users who work fine today.""" + assert DEFAULT_PROFILE.cap_tls_v1_2 is False + + +def test_unknown_model_returns_default(): + """Unknown / missing models fall back to DEFAULT_PROFILE so the FTP + path is never blocked on a missing entry.""" + assert get_ftp_profile(None) is DEFAULT_PROFILE + assert get_ftp_profile("") is DEFAULT_PROFILE + assert get_ftp_profile("Unknown Future Model") is DEFAULT_PROFILE + + +def test_p2s_caps_tls_v1_2(): + """P2S firmware 01.02.00.00 trips a vsFTPd + TLS 1.3 session-reuse + bug on the data channel; the profile must cap to TLS 1.2 so session + resumption is synchronous (#1401, reporter @iitazz).""" + profile = get_ftp_profile("P2S") + assert profile.cap_tls_v1_2 is True + + +def test_p2s_internal_ssdp_code_resolves_to_p2s(): + """SSDP internal code N7 → P2S profile. Camera profiles do the same + thing — keeps callers free of the code↔display-name mapping.""" + profile = get_ftp_profile("N7") + assert profile.cap_tls_v1_2 is True + + +def test_lookup_is_case_insensitive(): + """Printer.model may carry mixed case; the lookup normalises.""" + assert get_ftp_profile("p2s").cap_tls_v1_2 is True + assert get_ftp_profile("P2s").cap_tls_v1_2 is True + + +def test_non_capped_models_still_default(): + """Spot-check: the models the user dogfoods today (X1C, H2D) stay on + the default profile. Adding the P2S override must not accidentally + flip these.""" + assert get_ftp_profile("X1C").cap_tls_v1_2 is False + assert get_ftp_profile("H2D").cap_tls_v1_2 is False + assert get_ftp_profile("P1S").cap_tls_v1_2 is False + assert get_ftp_profile("A1").cap_tls_v1_2 is False + + +def test_profile_is_frozen(): + """FTPProfile is a frozen dataclass — runtime mutation should raise. + Same guarantee CameraProfile has.""" + try: + DEFAULT_PROFILE.cap_tls_v1_2 = True # type: ignore[misc] + except Exception as e: + assert "frozen" in str(e).lower() or "FrozenInstanceError" in type(e).__name__ + return + raise AssertionError("FTPProfile should be frozen but assignment succeeded") + + +def test_cap_tls_v1_2_actually_applied_to_ssl_context(): + """Pins the integration: when ``cap_tls_v1_2=True`` is passed to the + FTPS subclass, the SSL context's ``maximum_version`` is set to + TLSv1.2. Guards against a future refactor that drops the wiring + between profile and context (the registry would still look + correct, but the cap would silently stop applying).""" + from backend.app.services.bambu_ftp import ImplicitFTP_TLS + + capped = ImplicitFTP_TLS(cap_tls_v1_2=True) + assert capped.ssl_context.maximum_version == ssl.TLSVersion.TLSv1_2 + + uncapped = ImplicitFTP_TLS(cap_tls_v1_2=False) + # MAXIMUM_SUPPORTED is the "no cap applied" sentinel for SSLContext. + assert uncapped.ssl_context.maximum_version == ssl.TLSVersion.MAXIMUM_SUPPORTED + + +def test_ftp_profile_dataclass_default_constructible(): + """Sanity: FTPProfile() with no args yields the default profile + (every field has a default).""" + fresh = FTPProfile() + assert fresh == DEFAULT_PROFILE