From 8edc12c8f1bac4a2aab0cc39fa8ea432ad99ff19 Mon Sep 17 00:00:00 2001
From: Tommaso Bianchi <tommaso.b.bianchi@gmail.com>
Date: Thu, 23 Jul 2026 22:32:56 +0200
Subject: [PATCH] CAD recipe v2: legible version-mismatch errors, refuse old
 files cleanly

Port of snaporca 04c3d579a7. Bump SNAPORCA_CAD_RECIPE_VERSION 1 -> 2;
deserialize_recipe sets a user-facing error distinguishing too-new / too-old
/ corrupt instead of a silent bare false. No per-version migration by design.

Golden fixture regenerated at v2 (v1 retired), field-value reorder tripwire
unchanged. Fork adjustment: Catch2 v3 string matcher ContainsSubstring
(not v2's Contains) in the two new version-mismatch tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVzKmX6Y1aEteit1HTXG4Q
---
 src/libslic3r/CadDocument.cpp                 |  21 ++++-
 src/libslic3r/CadDocument.hpp                 |   6 +-
 .../{cad_recipe_v1.bin => cad_recipe_v2.bin}  | Bin 13492 -> 13492 bytes
 tests/libslic3r/test_caddocument.cpp          |  72 +++++++++++++++++-
 4 files changed, 93 insertions(+), 6 deletions(-)
 rename tests/data/{cad_recipe_v1.bin => cad_recipe_v2.bin} (99%)

diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp
index 3879c8dd15..226db5d6c2 100644
--- a/src/libslic3r/CadDocument.cpp
+++ b/src/libslic3r/CadDocument.cpp
@@ -1775,18 +1775,35 @@ std::string CadDocument::serialize_recipe() const
 
 bool CadDocument::deserialize_recipe(const std::string& blob)
 {
+    error.clear();
     try {
         std::istringstream iss(blob);
         cereal::BinaryInputArchive ar(iss);
         uint32_t v;
         ar(v);
-        if (v > SNAPORCA_CAD_RECIPE_VERSION)
+        if (v > SNAPORCA_CAD_RECIPE_VERSION) {
+            error = "saved with a newer version of SnapOrca CAD (format v"
+                  + std::to_string(v) + ", this build reads up to v"
+                  + std::to_string(SNAPORCA_CAD_RECIPE_VERSION) + ")";
             return false;
+        }
+        if (v < SNAPORCA_CAD_RECIPE_VERSION) {
+            error = "saved with an older version of SnapOrca CAD (format v"
+                  + std::to_string(v) + "); this project cannot be opened by this build";
+            return false;
+        }
         ar(features);
         return recompute();
-    } catch (const Standard_Failure&) {
+    } catch (const Standard_Failure& e) {
+        const char* what = e.GetMessageString();
+        error = std::string("CAD data could not be read")
+              + (what && *what ? ": " + std::string(what) : "");
+        return false;
+    } catch (const std::exception& e) {
+        error = std::string("CAD data could not be read: ") + e.what();
         return false;
     } catch (...) {
+        error = "CAD data could not be read";
         return false;
     }
 }
diff --git a/src/libslic3r/CadDocument.hpp b/src/libslic3r/CadDocument.hpp
index d8e0a57cd5..7d7bc9c968 100644
--- a/src/libslic3r/CadDocument.hpp
+++ b/src/libslic3r/CadDocument.hpp
@@ -356,7 +356,11 @@ public:
     void clear();
     bool recompute();   // replay features -> body + display_mesh; false on error
 
-    static constexpr uint32_t SNAPORCA_CAD_RECIPE_VERSION = 1;
+    // CadRecipe serialization contract:
+    // - bump this whenever CadFeature::save/load gains or loses a field
+    // - v1 blobs are deliberately not loadable; there is no migration path by design
+    // - append fields ONLY at the end of save/load, never reorder (golden fixture enforces this)
+    static constexpr uint32_t SNAPORCA_CAD_RECIPE_VERSION = 2;
     std::string serialize_recipe() const;
     bool deserialize_recipe(const std::string& blob);
 
diff --git a/tests/data/cad_recipe_v1.bin b/tests/data/cad_recipe_v2.bin
similarity index 99%
rename from tests/data/cad_recipe_v1.bin
rename to tests/data/cad_recipe_v2.bin
index 2a35b3c0dd1fa560d6aa8cd139265ed57767baab..437095ed39b595e201040c8448acc189ebca24ee 100644
GIT binary patch
delta 10
Rcmdmzxh0d4X(Qui695;p1J?im

delta 10
Rcmdmzxh0d4aU<hq695;k1J(cl

diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp
index 60c6d2ec11..ab0deb404c 100644
--- a/tests/libslic3r/test_caddocument.cpp
+++ b/tests/libslic3r/test_caddocument.cpp
@@ -1422,7 +1422,7 @@ TEST_CASE("serialize_recipe roundtrip with two bodies", "[CadDocument]")
     }
 }
 
-TEST_CASE("deserialize_recipe rejects future version", "[CadDocument]")
+TEST_CASE("deserialize_recipe rejects future version with error", "[CadDocument]")
 {
     CadDocument doc;
     std::ostringstream oss;
@@ -1432,6 +1432,72 @@ TEST_CASE("deserialize_recipe rejects future version", "[CadDocument]")
         ar(v);
     }
     REQUIRE_FALSE(doc.deserialize_recipe(oss.str()));
+    REQUIRE_FALSE(doc.error.empty());
+    CHECK_THAT(doc.error, Catch::Matchers::ContainsSubstring("newer version"));
+}
+
+TEST_CASE("deserialize_recipe rejects older version with error", "[CadDocument]")
+{
+    CadDocument doc;
+    std::ostringstream oss;
+    {
+        cereal::BinaryOutputArchive ar(oss);
+        uint32_t v = 1;
+        ar(v);
+    }
+    REQUIRE_FALSE(doc.deserialize_recipe(oss.str()));
+    REQUIRE_FALSE(doc.error.empty());
+    CHECK_THAT(doc.error, Catch::Matchers::ContainsSubstring("older version"));
+}
+
+TEST_CASE("deserialize_recipe handles truncated blob without throwing", "[CadDocument]")
+{
+    CadDocument doc;
+    std::string garbage = "this is not a valid cereal blob";
+    REQUIRE_FALSE(doc.deserialize_recipe(garbage));
+    REQUIRE_FALSE(doc.error.empty());
+}
+
+TEST_CASE("deserialize_recipe handles empty blob without throwing", "[CadDocument]")
+{
+    CadDocument doc;
+    REQUIRE_FALSE(doc.deserialize_recipe(""));
+    REQUIRE_FALSE(doc.error.empty());
+}
+
+TEST_CASE("deserialize_recipe error is non-empty on every failure path", "[CadDocument]")
+{
+    CadDocument doc;
+    auto reset = [&]() { doc = CadDocument{}; };
+
+    // too new
+    {
+        std::ostringstream oss;
+        { cereal::BinaryOutputArchive ar(oss); uint32_t v = 999; ar(v); }
+        reset();
+        REQUIRE_FALSE(doc.deserialize_recipe(oss.str()));
+        REQUIRE_FALSE(doc.error.empty());
+    }
+    // too old
+    {
+        std::ostringstream oss;
+        { cereal::BinaryOutputArchive ar(oss); uint32_t v = 1; ar(v); }
+        reset();
+        REQUIRE_FALSE(doc.deserialize_recipe(oss.str()));
+        REQUIRE_FALSE(doc.error.empty());
+    }
+    // truncated / garbage
+    {
+        reset();
+        REQUIRE_FALSE(doc.deserialize_recipe("not a valid blob \x00\x01\x02"));
+        REQUIRE_FALSE(doc.error.empty());
+    }
+    // empty
+    {
+        reset();
+        REQUIRE_FALSE(doc.deserialize_recipe(""));
+        REQUIRE_FALSE(doc.error.empty());
+    }
 }
 
 TEST_CASE("re-edit: editing a mid-timeline feature rebuilds downstream", "[CadDocument]")
@@ -1853,7 +1919,7 @@ TEST_CASE("regenerate golden recipe fixture", "[.regen]")
     auto blob = doc.serialize_recipe();
     REQUIRE_FALSE(blob.empty());
 
-    std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v1.bin";
+    std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v2.bin";
     std::ofstream ofs(path, std::ios::binary);
     REQUIRE(ofs.is_open());
     ofs.write(blob.data(), static_cast<std::streamsize>(blob.size()));
@@ -1867,7 +1933,7 @@ TEST_CASE("golden recipe v1 still deserialises", "[CadDocument]")
     using Catch::Matchers::WithinAbs;
 
     // Read the golden blob from disk
-    std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v1.bin";
+    std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v2.bin";
     std::ifstream ifs(path, std::ios::binary);
     REQUIRE(ifs.is_open());
     std::string blob((std::istreambuf_iterator<char>(ifs)),