فهرست منبع

fix(slicer): strip "-1" inherit sentinels from project_settings.config (#1201)

MakerWorld 3MFs sliced for P2S (and likely other Bambu printers) ship
project_settings.config entries with "-1" on fields BambuStudio writes
to mean "inherit from the parent process preset". The headless slicer
CLI's StaticPrintConfig validator runs against the embedded settings
*before* --load-settings overrides apply, so the sentinel trips the
field's lower-bound check and the CLI exits non-zero before the supplied
profile triplet is consulted. Both slice paths (with-profiles and the
embedded-settings fallback) read the same config and fail the same way,
so the user surfaces the error.

Surgical fix: open Metadata/project_settings.config, remove allowlisted
keys when their value is exactly "-1", re-zip. Allowlist starts with the
two from this report (raft_first_layer_expansion, tree_support_wall_count)
plus prime_tower_brim_width (a known sentinel cited in earlier reports).
Non-allowlisted "-1" values are left alone so a blanket strip can't
corrupt legitimate negative values. Other zip entries pass through
byte-identical — no risk of the failure mode the previous full-strip
experiment hit (silent CLI exit on initialisation).

Sanitiser runs before both slice_with_profiles and slice_without_profiles
paths since both fail on the same sentinel.

13 new unit tests in test_project_settings_sentinel_sanitiser.py cover
the allowlist removal contract (parametrised across all sentinel keys),
preservation of unaffected values, byte-identical round-trip of unrelated
zip entries, and defensive fallbacks for non-zip / malformed / no-config
inputs.

Adding new sentinel keys is one-line: append to
_PROJECT_SETTINGS_SENTINEL_KEYS in library.py.
maziggy 3 هفته پیش
والد
کامیت
aecf8283b5
3فایلهای تغییر یافته به همراه294 افزوده شده و 2 حذف شده
  1. 0 0
      CHANGELOG.md
  2. 93 2
      backend/app/api/routes/library.py
  3. 201 0
      backend/tests/unit/test_project_settings_sentinel_sanitiser.py

تفاوت فایلی نمایش داده نمی شود زیرا این فایل بسیار بزرگ است
+ 0 - 0
CHANGELOG.md


+ 93 - 2
backend/app/api/routes/library.py

@@ -4,6 +4,7 @@ import base64
 import binascii
 import contextlib
 import hashlib
+import json
 import logging
 import os
 import re
@@ -2632,6 +2633,84 @@ def _strip_3mf_embedded_settings(zip_bytes: bytes) -> bytes:
     return dst.getvalue()
 
 
+# Keys in ``Metadata/project_settings.config`` that BambuStudio writes ``"-1"``
+# to when the user wants the value inherited from the parent process preset.
+# The CLI's ``StaticPrintConfig`` validator runs against the embedded settings
+# *before* ``--load-settings`` overrides apply, so a sentinel ``"-1"`` trips
+# the field's lower-bound range check and the CLI exits non-zero before our
+# profile triplet is ever consulted (#1201 — MakerWorld P2S models).
+#
+# Allowlisted (rather than "strip every '-1' value") because some fields
+# legitimately accept negative numbers (z_offset, translation values, etc.)
+# and a blanket strip would silently corrupt those.
+#
+# Add new entries here as more reports surface — the slicer's error message
+# names the offending field directly (`<field>: -1 not in range [...]`).
+_PROJECT_SETTINGS_SENTINEL_KEYS = frozenset(
+    {
+        # Reported in #1201 (MakerWorld P2S 3MFs).
+        "raft_first_layer_expansion",
+        "tree_support_wall_count",
+        # Cited in the strip-experiment comment block above as a known sentinel
+        # case from earlier reports.
+        "prime_tower_brim_width",
+    }
+)
+
+
+def _sanitize_project_settings_sentinels(zip_bytes: bytes) -> bytes:
+    """Strip ``"-1"`` inherit-from-parent sentinels from the 3MF's
+    ``Metadata/project_settings.config`` so the slicer CLI's range validator
+    accepts the file (#1201).
+
+    Removes only allowlisted keys (see ``_PROJECT_SETTINGS_SENTINEL_KEYS``)
+    when their value is exactly ``"-1"``. The rest of the config — and every
+    other entry in the zip — is preserved byte-for-byte. Unlike the earlier
+    full-strip experiment (see ``_strip_3mf_embedded_settings`` and the
+    cautionary comment in ``_run_slicer_with_fallback``) this leaves
+    ``StaticPrintConfig`` initialisation intact: the file is still present,
+    still parses, and the slicer falls back to the supplied
+    ``--load-settings`` value for the removed key.
+
+    Returns the original bytes unchanged when no sanitisation is needed
+    (input isn't a valid zip, no ``project_settings.config``, no allowlisted
+    sentinels present, or any other parse failure) so the caller can pass
+    the result on without further checks.
+    """
+    from io import BytesIO
+
+    try:
+        with zipfile.ZipFile(BytesIO(zip_bytes), "r") as zin:
+            if "Metadata/project_settings.config" not in zin.namelist():
+                return zip_bytes
+            try:
+                config = json.loads(zin.read("Metadata/project_settings.config").decode("utf-8"))
+            except (json.JSONDecodeError, UnicodeDecodeError):
+                return zip_bytes
+            if not isinstance(config, dict):
+                return zip_bytes
+            removed = [key for key in _PROJECT_SETTINGS_SENTINEL_KEYS if config.get(key) == "-1"]
+            if not removed:
+                return zip_bytes
+            for key in removed:
+                config.pop(key, None)
+            patched = json.dumps(config)
+            logger.info(
+                "3MF sanitiser: removed sentinel '-1' for keys %s — slicer will use --load-settings defaults",
+                sorted(removed),
+            )
+            dst = BytesIO()
+            with zipfile.ZipFile(dst, "w", zipfile.ZIP_DEFLATED) as zout:
+                for item in zin.infolist():
+                    if item.filename == "Metadata/project_settings.config":
+                        zout.writestr(item, patched)
+                    else:
+                        zout.writestr(item, zin.read(item.filename))
+            return dst.getvalue()
+    except (zipfile.BadZipFile, OSError):
+        return zip_bytes
+
+
 async def _run_slicer_with_fallback(
     db: AsyncSession,
     *,
@@ -2725,6 +2804,14 @@ async def _run_slicer_with_fallback(
     # the embedded plate / model definitions remain intact.
     is_3mf = model_filename.lower().endswith(".3mf")
     primary_bytes = model_bytes
+    if is_3mf:
+        # Strip "-1" inherit-from-parent sentinels from
+        # Metadata/project_settings.config so the CLI's StaticPrintConfig
+        # range validator accepts the file (#1201). Surgical — keeps the
+        # config present, just removes the offending keys; the supplied
+        # --load-settings (and the fallback's embedded values for keys we
+        # didn't touch) still drive the slice.
+        primary_bytes = _sanitize_project_settings_sentinels(primary_bytes)
 
     used_embedded_settings = False
     service = SlicerApiService(api_url)
@@ -2768,9 +2855,13 @@ async def _run_slicer_with_fallback(
             )
             # Forward the same request_id + callback so the toast's live
             # progress keeps updating across the fallback retry instead
-            # of going blank for the rest of the slice.
+            # of going blank for the rest of the slice. Use the sanitised
+            # bytes — the embedded-settings path also reads the same
+            # project_settings.config and the same range validator runs
+            # there too, so without sanitisation the fallback would die
+            # on the same sentinel error (#1201).
             result = await service.slice_without_profiles(
-                model_bytes=model_bytes,
+                model_bytes=primary_bytes,
                 model_filename=model_filename,
                 plate=request.plate,
                 export_3mf=request.export_3mf,

+ 201 - 0
backend/tests/unit/test_project_settings_sentinel_sanitiser.py

@@ -0,0 +1,201 @@
+"""Unit tests for ``_sanitize_project_settings_sentinels`` (#1201).
+
+MakerWorld 3MFs sliced for the P2S (and potentially other Bambu printers)
+ship ``Metadata/project_settings.config`` entries with ``"-1"`` values on
+fields that BambuStudio's GUI internally interprets as "inherit from the
+parent process preset" — but the headless slicer CLI's
+``StaticPrintConfig`` validator runs *before* ``--load-settings`` overrides
+apply, so the sentinel trips the field's lower-bound range check and the
+CLI exits non-zero. The user sees::
+
+    Param values in 3mf/config error:
+    raft_first_layer_expansion: -1 not in range [0.0, 3.4e+38]
+    tree_support_wall_count: -1 not in range [0.0, 2.0]
+
+Earlier the codebase tried to fix this by stripping
+``Metadata/project_settings.config`` (and its sibling configs) entirely.
+That broke ``StaticPrintConfig`` initialisation — see the comment block
+inside ``_run_slicer_with_fallback`` — so the strip-everything path was
+reverted. The current fix is surgical: open the embedded config, drop
+*only* the allowlisted keys when their value is exactly ``"-1"``, and
+re-zip. The slicer then falls back to the supplied ``--load-settings``
+default for the removed keys, while every other entry in the zip stays
+byte-identical.
+
+Pinning the contract here rather than via the slicer integration tests
+because the fix is purely about the bytes we hand to the sidecar — no
+slicer mock needed.
+"""
+
+import io
+import json
+import zipfile
+
+import pytest
+
+from backend.app.api.routes.library import (
+    _PROJECT_SETTINGS_SENTINEL_KEYS,
+    _sanitize_project_settings_sentinels,
+)
+
+
+def _make_3mf(*, settings: dict | None = None, extra_files: dict | None = None) -> bytes:
+    """Build a tiny in-memory 3MF zip with project_settings.config + a model
+    payload, plus any caller-supplied extra entries (e.g., model_settings.config)
+    that should round-trip byte-identical through the sanitiser.
+    """
+    buf = io.BytesIO()
+    with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf:
+        zf.writestr("3D/3dmodel.model", "<model><resources/></model>")
+        if settings is not None:
+            zf.writestr("Metadata/project_settings.config", json.dumps(settings))
+        for name, content in (extra_files or {}).items():
+            zf.writestr(name, content)
+    return buf.getvalue()
+
+
+def _read_settings(zip_bytes: bytes) -> dict:
+    with zipfile.ZipFile(io.BytesIO(zip_bytes), "r") as zf:
+        return json.loads(zf.read("Metadata/project_settings.config").decode("utf-8"))
+
+
+def _zip_namelist(zip_bytes: bytes) -> list[str]:
+    with zipfile.ZipFile(io.BytesIO(zip_bytes), "r") as zf:
+        return zf.namelist()
+
+
+class TestRemovesSentinelValues:
+    @pytest.mark.parametrize("key", sorted(_PROJECT_SETTINGS_SENTINEL_KEYS))
+    def test_removes_each_allowlisted_key_when_value_is_minus_one(self, key):
+        original = _make_3mf(settings={key: "-1", "layer_height": "0.2"})
+        sanitised = _sanitize_project_settings_sentinels(original)
+
+        cfg = _read_settings(sanitised)
+        assert key not in cfg, f"Sentinel key {key!r} should have been removed"
+        # Non-sentinel settings stay untouched so --load-settings can layer
+        # cleanly on top of what the user actually configured.
+        assert cfg["layer_height"] == "0.2"
+
+    def test_removes_multiple_sentinels_at_once(self):
+        original = _make_3mf(
+            settings={
+                "raft_first_layer_expansion": "-1",
+                "tree_support_wall_count": "-1",
+                "prime_tower_brim_width": "-1",
+                "layer_height": "0.2",
+            }
+        )
+        sanitised = _sanitize_project_settings_sentinels(original)
+
+        cfg = _read_settings(sanitised)
+        assert "raft_first_layer_expansion" not in cfg
+        assert "tree_support_wall_count" not in cfg
+        assert "prime_tower_brim_width" not in cfg
+        assert cfg["layer_height"] == "0.2"
+
+
+class TestPreservesUnaffectedValues:
+    def test_preserves_allowlisted_key_with_legitimate_non_sentinel_value(self):
+        # A user who deliberately configured raft_first_layer_expansion=0 must
+        # see that 0 forwarded to the slicer — only literal "-1" gets stripped.
+        original = _make_3mf(settings={"raft_first_layer_expansion": "0"})
+        sanitised = _sanitize_project_settings_sentinels(original)
+        assert _read_settings(sanitised)["raft_first_layer_expansion"] == "0"
+
+    def test_does_not_touch_non_allowlisted_keys_with_minus_one(self):
+        # Non-allowlisted keys are left alone even when they hold "-1".
+        # Some Bambu fields legitimately allow negative values (z_offset,
+        # translation, etc.) and a blanket "-1" strip would corrupt those.
+        original = _make_3mf(settings={"z_offset": "-1", "layer_height": "0.2"})
+        sanitised = _sanitize_project_settings_sentinels(original)
+
+        cfg = _read_settings(sanitised)
+        assert cfg["z_offset"] == "-1"
+        assert cfg["layer_height"] == "0.2"
+
+    def test_returns_original_bytes_when_no_sentinel_present(self):
+        # If nothing needs sanitising, return the input identity-equal so
+        # the caller's downstream comparisons / hashes don't churn.
+        original = _make_3mf(settings={"layer_height": "0.2", "z_offset": "0"})
+        sanitised = _sanitize_project_settings_sentinels(original)
+        assert sanitised is original
+
+    def test_does_not_strip_array_value_even_if_includes_minus_one(self):
+        # Bambu sometimes stores per-filament/per-extruder values as JSON
+        # arrays of strings. v1 of the sanitiser deliberately handles only
+        # scalar strings — array forms are left alone so a per-filament
+        # legitimate "-1" inside a list isn't mistaken for the inherit
+        # sentinel and removed wholesale. If a future report shows the CLI
+        # rejects array-form sentinels, expand this then.
+        original = _make_3mf(settings={"raft_first_layer_expansion": ["-1", "0"]})
+        sanitised = _sanitize_project_settings_sentinels(original)
+        cfg = _read_settings(sanitised)
+        assert cfg["raft_first_layer_expansion"] == ["-1", "0"]
+
+
+class TestZipPreservation:
+    def test_other_zip_entries_pass_through_unchanged(self):
+        original = _make_3mf(
+            settings={"raft_first_layer_expansion": "-1"},
+            extra_files={
+                "Metadata/model_settings.config": "<config><object id='1'/></config>",
+                "Metadata/slice_info.config": "<config><plate/></config>",
+                "Metadata/_rels/model_settings.rels": "<rels/>",
+            },
+        )
+        sanitised = _sanitize_project_settings_sentinels(original)
+        assert sanitised is not original
+
+        names = _zip_namelist(sanitised)
+        # Every entry from the original zip must survive — the previous
+        # full-strip experiment broke StaticPrintConfig by dropping these,
+        # so the new sanitiser leaves them alone (#1201).
+        for required in (
+            "3D/3dmodel.model",
+            "Metadata/project_settings.config",
+            "Metadata/model_settings.config",
+            "Metadata/slice_info.config",
+            "Metadata/_rels/model_settings.rels",
+        ):
+            assert required in names, f"{required} must be preserved in the rebuilt zip"
+
+        # Content of unrelated entries is byte-identical.
+        with zipfile.ZipFile(io.BytesIO(sanitised), "r") as zf:
+            assert zf.read("Metadata/model_settings.config").decode() == "<config><object id='1'/></config>"
+            assert zf.read("3D/3dmodel.model").decode() == "<model><resources/></model>"
+
+
+class TestDefensiveFallbacks:
+    def test_returns_original_when_input_is_not_a_zip(self):
+        # An STL or any other non-zip input: pass through. The slicer
+        # routing decides whether 3MF sanitisation runs anyway, but
+        # defending here means a misrouted call can't corrupt the bytes.
+        garbage = b"not a zip file"
+        assert _sanitize_project_settings_sentinels(garbage) is garbage
+
+    def test_returns_original_when_settings_config_absent(self):
+        # 3MF without an embedded project_settings.config — nothing to do.
+        original = _make_3mf(settings=None)
+        assert _sanitize_project_settings_sentinels(original) is original
+
+    def test_returns_original_on_malformed_json(self):
+        # Settings file present but not valid JSON. We don't risk rebuilding
+        # the zip with synthesised content; the CLI will surface its own
+        # error and that's better than silent corruption.
+        buf = io.BytesIO()
+        with zipfile.ZipFile(buf, "w") as zf:
+            zf.writestr("3D/3dmodel.model", "<model/>")
+            zf.writestr("Metadata/project_settings.config", "{not valid json")
+        original = buf.getvalue()
+        assert _sanitize_project_settings_sentinels(original) is original
+
+    def test_returns_original_when_settings_root_is_not_a_dict(self):
+        # Real-world configs are objects, but defend against an array root
+        # (some legacy tooling produced these). Returning unchanged is
+        # safer than fabricating a dict.
+        buf = io.BytesIO()
+        with zipfile.ZipFile(buf, "w") as zf:
+            zf.writestr("3D/3dmodel.model", "<model/>")
+            zf.writestr("Metadata/project_settings.config", "[]")
+        original = buf.getvalue()
+        assert _sanitize_project_settings_sentinels(original) is original

برخی فایل ها در این مقایسه diff نمایش داده نمی شوند زیرا تعداد فایل ها بسیار زیاد است