From 8646c4095790c3dcc846fce75f54ab652ee3122a Mon Sep 17 00:00:00 2001 From: maziggy Date: Mon, 27 Jul 2026 11:18:54 +0200 Subject: [PATCH] fix(slicer): reject invalid sidecar output instead of storing a corrupt slice (#2671) The slice client only checked the sidecar's HTTP status, not its body. When the sidecar -- or a reverse proxy in front of it -- returned 200 OK with a body that wasn't a real 3MF (a stock/misconfigured sidecar, a proxy error page, a truncated response, or an OrcaSlicer/Bambu Studio CLI crash emitting no output), Bambuddy wrote that tiny blob to a .gcode.3mf, stored it as a valid sliced file (the 3MF-parse failure was swallowed as "no thumbnail"), and let it be queued and FTP'd to the printer -- producing the ~28-byte files that "did nothing" and then failed at print time. Separately, a genuine 413 comes from the proxy in front of the sidecar rejecting the multi-MB upload (model + profiles), so raising the body limit on the wrong proxy layer had no effect. - Factor the duplicated status handling in slice_with_profiles / slice_without_profiles into one _handle_slice_response. - When a 3MF export was requested, validate the body is a real ZIP; otherwise raise SlicerApiServerError with an actionable message instead of persisting a corrupt file. - Special-case 413 with a message naming client_max_body_size on the proxy directly in front of the sidecar (Cloudflare cap noted). --- CHANGELOG.md | 1 + backend/app/services/slicer_api.py | 82 ++++++++++---- .../integration/test_library_slice_api.py | 14 +-- .../tests/unit/services/test_slicer_api.py | 102 +++++++++++++++++- 4 files changed, 169 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8f3c89ec..a2bf02920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [1.2.6b1] - Unreleased ### Fixed +- **A broken slicer sidecar silently produced tiny corrupt files that were queued and printed anyway, and a reverse-proxy 413 wasn't self-explanatory (#2671, reporter @Austinzveare)** — With the slicer-API sidecar behind a reverse proxy, slicing produced ~28-byte files that "did nothing" (and could still be sent to the printer), while a separate proxy attempt failed with a bare **413 Request Entity Too Large** that the recommended nginx fix didn't seem to resolve. **Root cause.** Bambuddy's slice client only validated the sidecar's HTTP *status*, not its body. When the sidecar — or a proxy in front of it — returned `200 OK` with a body that wasn't a real 3MF (a stock/misconfigured sidecar, a proxy error page, a truncated response, or an OrcaSlicer/Bambu Studio CLI crash that emitted no output), Bambuddy wrote that tiny blob straight to a `.gcode.3mf`, stored it as a valid sliced file (the 3MF-parse failure was swallowed as merely "no thumbnail"), and let it be queued and FTP'd to the printer. Separately, a genuine 413 comes from the reverse proxy in front of the sidecar rejecting the multi-MB upload (model + profiles), not from the slicer — so raising the body limit on the wrong proxy layer had no effect. **Fix.** The slice client now validates the sidecar's output: when a 3MF export was requested, the response body must be a real ZIP (3MF container) or the job fails loudly with an actionable message ("…the body is not a valid 3MF (N bytes) — check the sidecar URL and any proxy in front of it") instead of persisting a corrupt file. A 413 now yields a targeted message naming the fix — raise `client_max_body_size` (or equivalent) on the proxy directly in front of the sidecar. Covered by tests: a 200 with a non-3MF body raises a server error (both the profile and embedded-settings paths), a 413 surfaces the reverse-proxy guidance, a valid 3MF still slices, and raw-gcode preview output is not zip-validated. Wiki troubleshooting updated with both scenarios. - **File Manager "sort by recent activity" didn't match `ls -t`, and there was no way to see a file's modified date (#2680 / #1770 follow-up, reporter @Kingbuzz0)** — For external (mapped/NAS) folders the folder tree's activity sort and the file pane's date sort put things in a seemingly random order — some entries roughly right, most not — instead of the real newest-first order shown by `ls -t` or Windows Explorer. **Root cause.** Nothing captured the files' actual on-disk modification time. The sort keyed off Bambuddy's own database `updated_at`/`created_at` timestamps, which for a bulk external scan are all the same instant (the scan time), so a whole block of files tied and sorted arbitrarily; only the few rows Bambuddy had later touched individually looked "partially correct." The folder tree also only bubbled up *immediate* child-file activity, so a file added deep in a subtree never lifted its parent folders. **Fix.** External scans now record each file's and each directory's real filesystem mtime (`os.stat().st_mtime`), refreshing it on every re-scan so a file edited over the mount re-sorts correctly. The folder tree's "recent activity" is now a **recursive** newest-descendant roll-up — a freshly-added file anywhere inside a folder lifts every ancestor — and both the tree sort and the file pane's date sort use the real mtime (falling back to `created_at` for managed uploads that have none). A new toolbar toggle shows/hides each item's **last-modified date** in the right-hand pane (grid and list views). Existing external folders backfill their mtimes on the next scan. Covered by tests: scan captures real file/folder mtimes, a re-scan refreshes a changed file, and a deep file bubbles its subtree's root ahead of a sibling with only a middle-aged file. - **An AMS-HT slot kept showing the removed filament and never cleared (#2670, reporter @needo37)** — After the #2594 fix, every empty-slot clearing path skipped AMS-HT units, so once a spool was removed the HT slot on the printer card stayed stuck on the old filament (Bambu Studio correctly showed it as Empty). The root cause was the HT's presence signal: firmware reports it as a single consecutive bit in `tray_exist_bits` at `16 + (ams_id − 128)` (HT-A = bit 16, HT-B = bit 17, …), not the regular `ams_id × 4` position — so the bitmask cleanup skipped the HT entirely, and the HT's `state` field is firmware-variant and can't be used instead. Confirmed against a live H2D capture (loaded HT reports the bit set, empty reports it clear) and cross-checked with the OrcaSlicer reference. **Fix.** The bitmask cleanup now understands the HT's real bit position and clears an empty HT slot the same way it clears a regular one, using firmware's own authoritative presence bit — so a loaded HT is never wrongly cleared (its bit stays set, keeping the #2594 fix intact). The AMS change detection now hashes the merged state, so a removal signalled only by the bitmask still unbinds the slot's spool assignment; and the websocket status now carries the presence bit so the card renders "Empty" (not "?") consistently. Verified for both single- and dual-HT setups. - **The print dialog clipped the per-filament gram usage when the material name was long, especially on mobile (#2669, reporter @apizz)** — In the Print dialog's Filament Mapping, each required filament shows its name and the grams the job needs, e.g. `Bambu PLA Basic (281.2g)`. The name and the gram figure lived in a single fixed-width column that truncated as one unit, so a long name (e.g. `Polymaker PLA Matte`) pushed the `(…g)` off the end and cut it off — partially on a wide screen, entirely in mobile portrait. The gram usage is the more important number here (it's what tells you whether a spool has enough left), so hiding it was the wrong thing to drop. **Fix.** The gram usage is now pinned and never shrinks or truncates; only the material name truncates (with the full name on hover), so the `(…g)` stays fully visible at every width. Applied to both the Specific-Printer and "Any [model]" mapping panels. Frontend-only, no behaviour change beyond layout. Covered by a test asserting the gram figure renders in its own non-truncating element separate from the truncating name. diff --git a/backend/app/services/slicer_api.py b/backend/app/services/slicer_api.py index fb7f05789..13d2b9359 100644 --- a/backend/app/services/slicer_api.py +++ b/backend/app/services/slicer_api.py @@ -9,7 +9,9 @@ under the hood, response body is raw G-code or 3MF with metadata in the """ import asyncio +import io import logging +import zipfile from collections.abc import Callable from typing import NamedTuple @@ -74,6 +76,62 @@ def _format_sidecar_error(response: httpx.Response) -> str: return (message or details or response.text)[:500] +def _handle_slice_response(response: httpx.Response, *, export_3mf: bool) -> SliceResult: + """Turn a sidecar ``/slice`` HTTP response into a validated ``SliceResult``. + + Shared by ``slice_with_profiles`` / ``slice_without_profiles`` so the status + handling and output validation live in one place. + + Beyond the status check, this guards against the sidecar (or a reverse proxy + in front of it) returning **HTTP 200 with a body that isn't a real slice** + (#2671): a stock/misconfigured sidecar, a proxy interstitial or truncated + response, or an OrcaSlicer/BambuStudio CLI crash that produces empty output. + Without this check Bambuddy would store that tiny blob as a ``.gcode.3mf``, + let it be queued, and FTP it to the printer — a silently-broken print. When + a 3MF export was requested the body must be a valid ZIP (3MF container); + anything else is treated as a sidecar failure. + + Raises: + SlicerInputError: 4xx from the sidecar (bad input / proxy body limit). + SlicerApiServerError: 5xx, or a 2xx whose body is not a valid 3MF. + """ + if response.status_code == 413: + # A 413 almost never comes from the slicer itself — it's a reverse proxy + # (nginx/SWAG/Traefik) or a CDN capping the multipart upload (model + + # profiles). Name the real fix so the user doesn't tweak the wrong layer. + raise SlicerInputError( + "The slice request was rejected as too large (HTTP 413). A reverse proxy " + "in front of the slicer sidecar is capping the request body — raise " + "'client_max_body_size' (nginx/SWAG) or the equivalent on the proxy that " + "sits directly in front of the sidecar, then reload it. If the sidecar is " + "behind Cloudflare, note its request-size cap." + ) + if response.status_code >= 500: + raise SlicerApiServerError(f"Slicer CLI failed ({response.status_code}): {_format_sidecar_error(response)}") + if response.status_code >= 400: + raise SlicerInputError(f"Slicer rejected input ({response.status_code}): {_format_sidecar_error(response)}") + + content = response.content + if export_3mf and not zipfile.is_zipfile(io.BytesIO(content)): + # 200 OK but the body is not a 3MF zip → the sidecar did not produce a + # usable slice. Surface it loudly instead of persisting a corrupt file. + detail = _format_sidecar_error(response) if len(content) <= 500 else "" + raise SlicerApiServerError( + f"Slicer sidecar returned HTTP {response.status_code} but the body is not a valid " + f"3MF ({len(content)} bytes). This usually means a misconfigured sidecar, an " + f"OrcaSlicer/BambuStudio CLI crash producing no output, or a reverse proxy returning " + f"an error page or truncating the response — verify the sidecar URL and any proxy in " + f"front of it." + (f" Body: {detail}" if detail else "") + ) + + return SliceResult( + content=content, + print_time_seconds=_safe_int(response.headers.get("x-print-time-seconds")), + filament_used_g=_safe_float(response.headers.get("x-filament-used-g")), + filament_used_mm=_safe_float(response.headers.get("x-filament-used-mm")), + ) + + def set_shared_http_client(client: httpx.AsyncClient | None) -> None: """Register an app-scoped client so per-request services can pool transport.""" global _shared_http_client @@ -294,17 +352,7 @@ class SlicerApiService: except (asyncio.CancelledError, Exception): pass # Polling errors must not fail the slice. - if response.status_code >= 500: - raise SlicerApiServerError(f"Slicer CLI failed ({response.status_code}): {_format_sidecar_error(response)}") - if response.status_code >= 400: - raise SlicerInputError(f"Slicer rejected input ({response.status_code}): {_format_sidecar_error(response)}") - - return SliceResult( - content=response.content, - print_time_seconds=_safe_int(response.headers.get("x-print-time-seconds")), - filament_used_g=_safe_float(response.headers.get("x-filament-used-g")), - filament_used_mm=_safe_float(response.headers.get("x-filament-used-mm")), - ) + return _handle_slice_response(response, export_3mf=export_3mf) async def slice_without_profiles( self, @@ -372,17 +420,7 @@ class SlicerApiService: except (asyncio.CancelledError, Exception): pass - if response.status_code >= 500: - raise SlicerApiServerError(f"Slicer CLI failed ({response.status_code}): {_format_sidecar_error(response)}") - if response.status_code >= 400: - raise SlicerInputError(f"Slicer rejected input ({response.status_code}): {_format_sidecar_error(response)}") - - return SliceResult( - content=response.content, - print_time_seconds=_safe_int(response.headers.get("x-print-time-seconds")), - filament_used_g=_safe_float(response.headers.get("x-filament-used-g")), - filament_used_mm=_safe_float(response.headers.get("x-filament-used-mm")), - ) + return _handle_slice_response(response, export_3mf=export_3mf) def _safe_int(value: str | None) -> int: diff --git a/backend/tests/integration/test_library_slice_api.py b/backend/tests/integration/test_library_slice_api.py index 5e3f03583..8cf838ad0 100644 --- a/backend/tests/integration/test_library_slice_api.py +++ b/backend/tests/integration/test_library_slice_api.py @@ -206,7 +206,7 @@ class TestSliceLibraryFile: captured["url"] = str(request.url) return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake-3mf", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "656", "x-filament-used-g": "0.94", @@ -249,7 +249,7 @@ class TestSliceLibraryFile: captured["body"] = bytes(request.content) return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "10", "x-filament-used-g": "0.1", @@ -291,7 +291,7 @@ class TestSliceLibraryFile: captured["body"] = bytes(request.content) return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "10", "x-filament-used-g": "0.1", @@ -416,7 +416,7 @@ class TestSliceLibraryFile: # Retry: no profile triplet → succeed with embedded settings return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake-3mf", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "100", "x-filament-used-g": "1.0", @@ -499,7 +499,7 @@ class TestSliceLibraryFile: captured["body"] = request.content return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake-3mf", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "1", "x-filament-used-g": "0", @@ -564,7 +564,7 @@ class TestSliceLibraryFile: captured["body"] = request.content return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake-3mf", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "100", "x-filament-used-g": "1.0", @@ -607,7 +607,7 @@ class TestSliceLibraryFile: captured["body"] = request.content return httpx.Response( status_code=200, - content=b"PK\x03\x04 fake-3mf", + content=_make_3mf_with_settings(), # #2671: real zip; validation rejects non-3MF bodies headers={ "x-print-time-seconds": "1", "x-filament-used-g": "0", diff --git a/backend/tests/unit/services/test_slicer_api.py b/backend/tests/unit/services/test_slicer_api.py index 2dd448d25..fd50ffaf2 100644 --- a/backend/tests/unit/services/test_slicer_api.py +++ b/backend/tests/unit/services/test_slicer_api.py @@ -226,9 +226,11 @@ class TestSliceWithProfiles: def handler(request: httpx.Request) -> httpx.Response: captured["body"] = request.content + # export_3mf=True → the response body must be a valid 3MF zip, or the + # #2671 output validation rejects it. This test is about the request. return httpx.Response( status_code=200, - content=b"3MF-BYTES", + content=_valid_3mf_zip(), headers={"x-print-time-seconds": "0", "x-filament-used-g": "0", "x-filament-used-mm": "0"}, ) @@ -362,6 +364,104 @@ class TestSliceWithProfiles: assert result.filament_used_mm == 0.0 +def _valid_3mf_zip() -> bytes: + """Minimal-but-valid ZIP so is_zipfile() accepts it as a 3MF container.""" + import io + import zipfile + + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("[Content_Types].xml", "") + zf.writestr("Metadata/plate_1.gcode", "; G-CODE\nG28\n") + return buf.getvalue() + + +class TestSliceOutputValidation: + """#2671: a 200 with a non-3MF body must not be persisted as a slice.""" + + _SLICE_KW = { + "model_bytes": b"solid Cube\n", + "model_filename": "Cube.stl", + "printer_profile_json": "{}", + "process_profile_json": "{}", + "filament_profile_jsons": ["{}"], + } + + @pytest.mark.asyncio + async def test_413_gives_actionable_reverse_proxy_message(self): + # A 413 is a proxy/CDN body-size cap, not the slicer — the message must + # point at the right layer so the user stops editing the wrong one. + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status_code=413, content=b"413 Request Entity Too Large") + + service = SlicerApiService("http://sidecar:3000", client=_mock_client(handler)) + with pytest.raises(SlicerInputError) as exc_info: + await service.slice_with_profiles(export_3mf=True, **self._SLICE_KW) + msg = str(exc_info.value) + assert "413" in msg + assert "client_max_body_size" in msg + assert "proxy" in msg.lower() + + @pytest.mark.asyncio + async def test_export_3mf_rejects_non_zip_200_body(self): + # The exact failure from #2671: sidecar/proxy returns 200 with a tiny + # garbage body; Bambuddy must NOT accept it as a sliced 3MF. + body = b'{"detail":"Not Found"}xxxxxx' # 28 bytes, not a zip + assert len(body) == 28 + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status_code=200, content=body) + + service = SlicerApiService("http://sidecar:3000", client=_mock_client(handler)) + with pytest.raises(SlicerApiServerError) as exc_info: + await service.slice_with_profiles(export_3mf=True, **self._SLICE_KW) + msg = str(exc_info.value) + assert "not a valid" in msg.lower() + assert "28 bytes" in msg + + @pytest.mark.asyncio + async def test_export_3mf_accepts_valid_zip_body(self): + zip_bytes = _valid_3mf_zip() + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + status_code=200, + content=zip_bytes, + headers={"x-print-time-seconds": "656"}, + ) + + service = SlicerApiService("http://sidecar:3000", client=_mock_client(handler)) + result = await service.slice_with_profiles(export_3mf=True, **self._SLICE_KW) + assert result.content == zip_bytes + assert result.print_time_seconds == 656 + + @pytest.mark.asyncio + async def test_raw_gcode_body_not_zip_validated(self): + # export_3mf defaults False (preview / raw-gcode callers): the body is + # legitimately not a zip, so the validation must NOT fire. + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status_code=200, content=b"; G-CODE\nG28\n") + + service = SlicerApiService("http://sidecar:3000", client=_mock_client(handler)) + result = await service.slice_with_profiles(**self._SLICE_KW) + assert result.content == b"; G-CODE\nG28\n" + + @pytest.mark.asyncio + async def test_without_profiles_also_rejects_non_zip_200_body(self): + # The validation lives in the shared response handler, so the + # embedded-settings path (slice_without_profiles) is covered too. + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status_code=200, content=b"nope") + + service = SlicerApiService("http://sidecar:3000", client=_mock_client(handler)) + with pytest.raises(SlicerApiServerError): + await service.slice_without_profiles( + model_bytes=b"solid Cube\n", + model_filename="Cube.stl", + export_3mf=True, + ) + + class TestHealth: @pytest.mark.asyncio async def test_health_returns_body(self):