Browse Source

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).
maziggy 1 tháng trước cách đây
mục cha
commit
8646c40957

Những thai đổi đã bị hủy bỏ vì nó quá lớn
+ 0 - 0
CHANGELOG.md


+ 60 - 22
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:

+ 7 - 7
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",

+ 101 - 1
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", "<Types/>")
+        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"<html>413 Request Entity Too Large</html>")
+
+        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):

Một số tệp đã không được hiển thị bởi vì quá nhiều tập tin thay đổi trong này khác