Reject Paths with an Embedded NUL When Confining Extraction

is_path_within_root compared each component with "..", so a name such
as "..\0" passed the check. The filesystem calls stop at the NUL and
act on a shorter path than the one that was checked: a symlink target
read from a plugin archive as raw bytes was created as "..", pointing
out of the plugin directory.

A path containing a NUL is now rejected before anything touches the
filesystem, which covers every caller, including entry names taken from
the Unicode Path extra field.

docs/HLSD/preset-cache.md

@@ -28,8 +28,8 @@ Per-vendor granularity is what makes the system practical:
 - A vendor whose profile is bumped invalidates only its own cache. The other 60-odd
   vendors keep theirs — even when the bumped vendor is the shared Orca filament
   library everyone else inherits from.
-- The setup wizard loads its vendors through the same routine as startup, so it
+- The setup wizard, which loads vendors one at a time, gets the same speedup as
-  gets the same speedup without a second code path.
+  startup without a second code path.
 - A vendor with no cache, or a broken one, costs only that vendor a parse.
 
 A cache holds *system* presets only. User presets, project settings and modified
@@ -162,20 +162,15 @@ cache nothing can invalidate is worse than no cache.
 
 Vendors load in a fixed order, because filament inheritance crosses exactly one
 boundary: any vendor's filament may inherit from the shared Orca filament library,
-and nothing else reaches across vendors — an `include` is always vendor-local. Only
+and nothing else reaches across vendors — an `include` is always vendor-local. The
-installing a vendor's presets crosses it; reading the vendor, from its cache or its
+library therefore goes first, alone; every other vendor follows in parallel, resolving
-JSONs, needs nothing from the library. So every other vendor is read while the library
+against it; and the results are merged in a stable order:
-loads, each is installed against it as soon as both are done, and the results are
-merged in a stable order:
 
 ```mermaid
 flowchart LR
-    lib["1 · OrcaFilamentLibrary loaded;<br/>meanwhile every other vendor read<br/>from its cache or its JSONs"] --> par["2 · every other vendor installed<br/>in parallel, each into its own bundle,<br/>filaments resolving against the library"] --> merge["3 · bundles merged into one,<br/>in one pass per collection,<br/>in stable vendor order"]
+    lib["1 · OrcaFilamentLibrary<br/>loaded first, synchronously"] --> par["2 · every other vendor in parallel,<br/>each into its own bundle, filaments<br/>resolving against the loaded library"] --> merge["3 · bundles merged into one,<br/>sequentially, in stable vendor order"]
 ```
 
-`PresetBundle::load_vendors` runs these steps for startup and for the setup wizard,
-which hand it the vendors to load and the directory each is installed in.
-
 Whether a vendor comes from its cache or from a parse changes nothing in that
 order — both produce the same bundle, so cached and parsed vendors mix freely in
 one startup.
@@ -216,12 +211,11 @@ shipped cache answered first, so the profile in `<data_dir>/system/` was never p
 and its cache was never written back.
 
 Serving from a cache is not a memory-image restore. The entries are deserialized and
-then installed by `install_vendor`, the routine the JSON path hands the vendor's entries
+then installed one by one — inheritance resolved against the presets installed before
-to once it has parsed the sub-files: inheritance resolved against the presets installed
+them and the currently loaded filament library, includes layered in, configs flattened
-before them and the currently loaded filament library, includes layered in, configs
+onto the collection defaults, validated and registered — by the same function the JSON
-flattened onto the collection defaults, validated and registered. An `include` layers
+path calls straight after parsing a sub-file. An `include` layers what the included
-what the included base states, between the parent and the preset's own keys: the base's
+base states, between the parent and the preset's own keys: the base's diff against the
-diff against the
 default, taken when the base itself was installed and before the per-variant padding
 `inherits` sees, so only what a template sets reaches the presets including it. The two
 paths share everything below the parse, which is what makes a cache-loaded bundle
@@ -229,16 +223,6 @@ indistinguishable from a JSON-loaded one by construction rather than by test cov
 Installation also rebuilds each preset's file path from the local data directory, so a
 shipped cache never carries the generating machine's paths.
 
-Installing an entry is split in two. `resolve_vendor_preset` flattens it, reading only
-what is registered under the names it inherits and includes, and `commit_vendor_preset`
-registers it, the only step that writes anything shared. Entries resolve across threads
-in runs and commit in the order the vendor lists them. A run ends before an entry that
-inherits or includes one already in it, since that one's commit registers what the
-entry resolves against, so no entry in a run reads what another in it registers. An
-entry's parse messages are held until it commits. The bundle, the log's parse and
-install messages and the error count therefore come out as parsing and installing one
-entry at a time would leave them, whatever the listing order.
-
 App upgrades work because a cache normally survives one. Only a deliberate
 `CACHE_VERSION` bump makes an installed cache unreadable, and that is handled at
 install time rather than at load: a vendor whose cache this build cannot read counts
@@ -270,20 +254,17 @@ the wizard caches the *derived JSON*, not another form of the inputs:
 open, the wizard computes the current stamps (one version peek per vendor) and, when
 they match, serves the catalog from the file — no bundle built, no preset installed.
 Caching bundle inputs instead was tried and measured: rebuilding the bundle from
-per-vendor caches costs over a second of preset installation whatever feeds it, so
+per-vendor caches costs ~2 s of preset installation whatever feeds it, so only
-only skipping the rebuild entirely wins.
+skipping the rebuild entirely wins.
 
 Any change to the set — a vendor added, removed or updated, or its cache-only
 `.opc` replaced by a newer one — changes the stamps and retires the whole file;
-the wizard then rebuilds the bundle with `PresetBundle::load_vendors`, the load
+the wizard then rebuilds the bundle vendor by vendor (per-vendor caches serving where
-startup uses (per-vendor caches serving where they cover), and writes the catalog
+they cover) and writes the catalog back. Selections, region and per-open decorations
-back. When a vendor fails to load, the filament library included, that open falls
+are applied downstream of the cache either way, so a served catalog is
-back to the wizard's own scan of the vendor JSONs, as when no bundle can be built,
+indistinguishable from a rebuilt one. Nothing ships this file and the updater never
-and writes nothing. Selections, region and per-open decorations are applied
+touches it; it is a locally written artifact, re-derived whenever stale, written
-downstream of the cache either way, so a served catalog is indistinguishable from a
+through a temp file and rename so half a cache is never readable.
-rebuilt one. Nothing ships this file and the updater never touches it; it is a
-locally written artifact, re-derived whenever stale, written through a temp file and
-rename so half a cache is never readable.
 
 The cache lives under `<data_dir>/cache/`, not beside the vendors: everything that
 scans `<data_dir>/system/` treats any `.opc` there as a vendor, so a non-vendor
@@ -397,13 +378,6 @@ enumerates only `*.json` will find no vendors at all in a packaged build.
   the `CachedPreset` field list — written and read by `visit_entry` in
   `PresetCacheFormat.cpp`, one list for the save, the load and the name peek alike — or
   the cache's own layout or stamps, requires bumping `CACHE_VERSION` by hand.
-- **Adding a kind of reference between presets**, as `inherits` and `include` are:
-  parse the names into `CachedPreset` (a field change, so `CACHE_VERSION` is bumped),
-  have `install_vendor_entries` end a run before an entry that names one already in it
-  and retain what the names point at, look them up only in `resolve_vendor_preset`, and
-  register what they point at only in `commit_vendor_preset`. The listing-order test in
-  `test_vendor_cache.cpp` fails for a kind the runs do not check once its fixture uses
-  it.
 - **The dictionary indexes with a `uint16`**, so `print_config_def` may hold at most
   65535 options and one cache at most 65535 distinct enum value names.
   `CacheDictionary::save` throws past that, which surfaces when CI generates the

src/libslic3r/AppConfig.cpp

@@ -5,7 +5,6 @@
 //BBS
 #include "Preset.hpp"
 #include "Exception.hpp"
-#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Thread.hpp"
 #include "format.hpp"
@@ -740,13 +739,10 @@ static bool verify_config_file_checksum(boost::nowide::ifstream &ifs)
 
 
 #ifdef USE_JSON_CONFIG
-std::string AppConfig::load(bool read_only)
+std::string AppConfig::load()
 {
     json j;
 
-    // Keep another instance from replacing or restoring the file mid-read.
-    InstanceLock instance_lock(read_only ? std::string() : lock_path());
-
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -996,6 +992,7 @@ void AppConfig::save()
     // The config is first written to a file with a PID suffix and then moved
     // to avoid race conditions with multiple instances of Slic3r
     const auto path = config_path();
+    std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     json j;
 
@@ -1125,18 +1122,43 @@ void AppConfig::save()
 
         j["local_machines"][local_machine.first] = m_json;
     }
-    const std::string config_str = j.dump(1, '\t');
+    boost::nowide::ofstream c;
-    if (write_config_file(path, config_str + "\n", config_str))
+    c.open(path_pid, std::ios::out | std::ios::trunc);
+    c << j.dump(1, '\t') << std::endl;
+
+#ifdef WIN32
+    // WIN32 specific: The final "rename_file()" call is not safe in case of an application crash, there is no atomic "rename file" API
+    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
+    // we save the config file into a backup first before moving it to the final destination.
+    c << appconfig_md5_hash_line(j.dump(1, '\t'));
+#endif
+
+    c.close();
+    if (c.fail()) {
+      BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
+      return;
+    }
+
+#ifdef WIN32
+    // Make a backup of the configuration file before copying it to the final destination.
+    std::string error_message;
+    std::string backup_path = (boost::format("%1%.bak") % path).str();
+    // Copy configuration file with PID suffix into the configuration file with "bak" suffix.
+    if (copy_file(path_pid, backup_path, error_message, false) != SUCCESS)
+        BOOST_LOG_TRIVIAL(error) << "Copying from " << path_pid << " to " << backup_path << " failed. Failed to create a backup configuration.";
+#endif
+
+    // Rename the config atomically.
+    // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile.
+    // To cope with that, we already made a backup of the config on Windows.
+    rename_file(path_pid, path);
     m_dirty = false;
 }
 
 #else
 
-std::string AppConfig::load(bool read_only)
+std::string AppConfig::load()
 {
-    // Keep another instance from replacing or restoring the file mid-read.
-    InstanceLock instance_lock(read_only ? std::string() : lock_path());
-
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -1274,6 +1296,7 @@ void AppConfig::save()
     // The config is first written to a file with a PID suffix and then moved
     // to avoid race conditions with multiple instances of Slic3r
     const auto path = config_path();
+    std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     std::stringstream config_ss;
     if (m_mode == EAppMode::Editor)
@@ -1309,39 +1332,39 @@ void AppConfig::save()
     // One empty line before the MD5 sum.
     config_ss << std::endl;
 
-    const std::string config_str = config_ss.str();
+    std::string config_str = config_ss.str();
-    if (write_config_file(path, config_str, config_str))
+    boost::nowide::ofstream c;
+    c.open(path_pid, std::ios::out | std::ios::trunc);
+    c << config_str;
+#ifdef WIN32
+    // WIN32 specific: The final "rename_file()" call is not safe in case of an application crash, there is no atomic "rename file" API
+    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
+    // we save the config file into a backup first before moving it to the final destination.
+    c << appconfig_md5_hash_line(config_str);
+#endif
+    c.close();
+    if (c.fail()) {
+      BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
+      return;
+    }
+
+#ifdef WIN32
+    // Make a backup of the configuration file before copying it to the final destination.
+    std::string error_message;
+    std::string backup_path = (boost::format("%1%.bak") % path).str();
+    // Copy configuration file with PID suffix into the configuration file with "bak" suffix.
+    if (copy_file(path_pid, backup_path, error_message, false) != SUCCESS)
+        BOOST_LOG_TRIVIAL(error) << "Copying from " << path_pid << " to " << backup_path << " failed. Failed to create a backup configuration.";
+#endif
+
+    // Rename the config atomically.
+    // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile.
+    // To cope with that, we already made a backup of the config on Windows.
+    rename_file(path_pid, path);
     m_dirty = false;
 }
 #endif
 
-bool AppConfig::write_config_file(const std::string &path, std::string body, const std::string &checksum_source)
-{
-    // Everything before this is assembly; only the writes need the other instances kept out.
-    InstanceLock instance_lock(lock_path());
-#ifdef WIN32
-    // WIN32 specific: the final replace is not safe in case of an application crash, there is no atomic "rename file" API
-    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
-    // we save the config file into a backup first before moving it to the final destination.
-    body += appconfig_md5_hash_line(checksum_source);
-#endif
-    // Not flushed to the device: the idle handler saves on the GUI thread after
-    // any change, and the rename already gives a complete old or new file.
-    if (const std::error_code ec = write_file_atomically(path, body)) {
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the configuration " << path << ": " << ec.message() << "; trying again in 10 s";
-        m_retry_save_at = std::chrono::steady_clock::now() + std::chrono::seconds(10);
-        return false;
-    }
-    m_retry_save_at = {};
-#ifdef WIN32
-    // Written after the config, so the backup never holds a state that was not confirmed written.
-    const std::string backup_path = (boost::format("%1%.bak") % path).str();
-    if (const std::error_code ec = write_file_atomically(backup_path, body))
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the backup configuration " << backup_path << ": " << ec.message();
-#endif
-    return true;
-}
-
 bool AppConfig::get_variant(const std::string &vendor, const std::string &model, const std::string &variant) const
 {
     const auto it_v = m_vendors.find(vendor);
@@ -1830,11 +1853,6 @@ void AppConfig::reset_selections()
     }
 }
 
-std::string AppConfig::lock_path()
-{
-    return Slic3r::data_dir().empty() ? std::string() : config_path() + ".lock";
-}
-
 std::string AppConfig::config_path()
 {
 #ifdef USE_JSON_CONFIG
@@ -1871,7 +1889,7 @@ bool AppConfig::exists()
 
 std::string AppConfig::load_if_exists()
 {
-    return boost::filesystem::exists(loading_path()) ? load(/*read_only=*/true) : std::string();
+    return boost::filesystem::exists(loading_path()) ? load() : std::string();
 }
 
 }; // namespace Slic3r

src/libslic3r/AppConfig.hpp

@@ -2,7 +2,6 @@
 #define slic3r_AppConfig_hpp_
 
 #include <set>
-#include <chrono>
 #include <map>
 #include <string>
 #include "nlohmann/json.hpp"
@@ -123,18 +122,14 @@ public:
 
 	// Load the slic3r.ini from a user profile directory (or a datadir, if configured).
 	// Return an error string, or an empty string on success.
-	std::string         load(bool read_only = false);
+	std::string         load();
 	// Treat a missing config as default state; otherwise load it normally.
-	// The CLI's load: it never saves, so it takes no lock and creates no lock file.
 	std::string         load_if_exists();
 	// Store the slic3r.ini into a user profile directory (or a datadir, if configured).
 	void 			   	save();
 
 	// Does this config need to be saved?
 	bool 				dirty() const { return m_dirty; }
-	// False for ten seconds after a failed write, so the idle handler does not
-	// repeat a hopeless attempt on every event; an explicit save() always tries.
-	bool 				save_due() const { return std::chrono::steady_clock::now() >= m_retry_save_at; }
 
 
 	void				set_dirty() { m_dirty = true; }
@@ -344,8 +339,6 @@ public:
 
 	// Get the default config path from Slic3r::data_dir().
 	std::string			config_path();
-	// Lock file guarding config_path() against other running instances; empty without a data dir.
-	std::string			lock_path();
 
 	// Returns true if the user's data directory comes from before Slic3r 1.40.0 (no updating)
 	bool 				legacy_datadir() const { return m_legacy_datadir; }
@@ -456,16 +449,8 @@ private:
 
 	// Preset for each machine
 	MachineSettingMap											m_printer_settings;
-	// Writes the assembled config text, and on Windows its checksum and a backup copy; false when the
-	// config itself could not be written, in which case the caller stays dirty and retries. `checksum_source`
-	// is the text load() will verify, which for the JSON config ends before the trailing newline.
-	bool												write_config_file(const std::string &path, std::string body, const std::string &checksum_source);
-
 	// Has any value been modified since the config.ini has been last saved or loaded?
 	bool														m_dirty;
-	// After a failed write, save_due() is false for the next ten seconds, so the
-	// idle handler does not repeat a hopeless write on every event.
-	std::chrono::steady_clock::time_point						m_retry_save_at{};
 	// Original version found in the ini file before it was overwritten
 	Semver                                                      m_orig_version;
 	// Whether the existing version is before system profiles & configuration updating

src/libslic3r/CMakeLists.txt

@@ -306,8 +306,6 @@ set(lisbslic3r_sources
     Geometry/VoronoiUtils.cpp
     Geometry/VoronoiUtils.hpp
     Geometry/VoronoiVisualUtils.hpp
-    InstanceLock.cpp
-    InstanceLock.hpp
     Int128.hpp
     KDTreeIndirect.hpp
     Layer.cpp
@@ -357,7 +355,6 @@ set(lisbslic3r_sources
     Optimize/Optimizer.hpp
     Orient.cpp
     Orient.hpp
-    ParallelResolve.hpp
     ParameterUtils.cpp
     ParameterUtils.hpp
     pchheader.cpp

src/libslic3r/Config.cpp

@@ -849,19 +849,6 @@ ConfigSubstitutions ConfigBase::load_from_json(const std::string &file, ForwardC
     return std::move(substitutions_ctxt.substitutions);
 }
 
-// Case-insensitive compare of a JSON key against a fixed ASCII one, without
-// boost::iequals, whose std::locale() takes a lock the whole process shares in the
-// MSVC runtime.
-static bool ascii_iequals(const std::string &key, const char *literal)
-{
-    auto lower = [](char c) { return (c >= 'A' && c <= 'Z') ? char(c - 'A' + 'a') : c; };
-    size_t i = 0;
-    for (; i < key.size() && literal[i] != '\0'; ++ i)
-        if (lower(key[i]) != lower(literal[i]))
-            return false;
-    return i == key.size() && literal[i] == '\0';
-}
-
 int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContext& substitution_context, bool load_inherits_to_config, std::map<std::string, std::string>& key_values, std::string& reason)
 {
     json j;
@@ -929,44 +916,44 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex
         }
         //parse the json elements
         for (auto it = j.begin(); it != j.end(); it++) {
-            if (ascii_iequals(it.key(), BBL_JSON_KEY_VERSION)) {
+            if (boost::iequals(it.key(),BBL_JSON_KEY_VERSION)) {
                 key_values.emplace(BBL_JSON_KEY_VERSION, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_IS_CUSTOM)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_IS_CUSTOM)) {
                 //skip it
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_NAME)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_NAME)) {
                 key_values.emplace(BBL_JSON_KEY_NAME, it.value());
                 if (it.value() == "project_settings")
                     is_project_settings = true;
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_URL)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_URL)) {
                 key_values.emplace(BBL_JSON_KEY_URL, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_TYPE)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_TYPE)) {
                 key_values.emplace(BBL_JSON_KEY_TYPE, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_SETTING_ID)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_SETTING_ID)) {
                 key_values.emplace(BBL_JSON_KEY_SETTING_ID, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_FILAMENT_ID)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_FILAMENT_ID)) {
                 key_values.emplace(BBL_JSON_KEY_FILAMENT_ID, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_FROM)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_FROM)) {
                 key_values.emplace(BBL_JSON_KEY_FROM, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_DESCRIPTION)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_DESCRIPTION)) {
                 key_values.emplace(BBL_JSON_KEY_DESCRIPTION, it.value());
             }
-            else if (ascii_iequals(it.key(), BBL_JSON_KEY_INSTANTIATION)) {
+            else if (boost::iequals(it.key(), BBL_JSON_KEY_INSTANTIATION)) {
                 key_values.emplace(BBL_JSON_KEY_INSTANTIATION, it.value());
             }
-            else if (!load_inherits_to_config && ascii_iequals(it.key(), BBL_JSON_KEY_INHERITS)) {
+            else if (!load_inherits_to_config && boost::iequals(it.key(), BBL_JSON_KEY_INHERITS)) {
                 key_values.emplace(BBL_JSON_KEY_INHERITS, it.value());
             }
-            else if (!load_inherits_to_config && ascii_iequals(it.key(), BBL_JSON_KEY_INCLUDES)) {
+            else if (!load_inherits_to_config && boost::iequals(it.key(), BBL_JSON_KEY_INCLUDES)) {
                 key_values.emplace(BBL_JSON_KEY_INCLUDES, it.value().dump());
-            } else if (ascii_iequals(it.key(), ORCA_JSON_KEY_RENAMED_FROM)) {
+            } else if (boost::iequals(it.key(), ORCA_JSON_KEY_RENAMED_FROM)) {
                 key_values.emplace(ORCA_JSON_KEY_RENAMED_FROM, it.value());
             } else {
                 t_config_option_key opt_key = it.key();
@@ -1538,9 +1525,11 @@ void ConfigBase::save_to_json(const std::string &file, const std::string &name,
     // Serialize first: if that throws (invalid UTF-8), the existing file stays untouched.
     std::ostringstream ss;
     this->save_to_json(ss, name, from, version);
-    if (const std::error_code ec = write_file_atomically(file, ss.str()))
+    boost::nowide::ofstream c;
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(": failed to save config to %1%: %2%") % file % ec.message();
+    c.open(file, std::ios::out | std::ios::trunc);
-    else
+    c << ss.str();
+    c.close();
+
     BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ":" <<__LINE__ << boost::format(", saved config to %1%\n")%file;
 }
 

src/libslic3r/InstanceLock.cpp

@@ -1,165 +0,0 @@
-#include "InstanceLock.hpp"
-
-#include <map>
-#include <memory>
-#include <system_error>
-#include <thread>
-
-#include <boost/filesystem.hpp>
-#include <boost/log/trivial.hpp>
-#include <boost/nowide/fstream.hpp>
-#ifdef _WIN32
-#include <boost/interprocess/sync/file_lock.hpp>
-#include <boost/nowide/convert.hpp>
-#else
-#include <cerrno>
-#include <fcntl.h>
-#include <sys/file.h>
-#include <unistd.h>
-#endif
-
-namespace Slic3r {
-
-#ifdef _WIN32
-// LockFileEx, held by this handle alone.
-using NativeFileLock = boost::interprocess::file_lock;
-#else
-// flock(2) rather than an fcntl lock: it belongs to this open file description,
-// so any other code in the process that opens and closes the lock file, as a
-// backup or an export walking the data dir might, cannot drop it. An fcntl
-// lock would go with the first such close.
-class NativeFileLock
-{
-public:
-    explicit NativeFileLock(const char *path) : m_fd(::open(path, O_RDWR | O_CREAT | O_CLOEXEC, 0644))
-    {
-        if (m_fd < 0)
-            throw std::system_error(errno, std::generic_category(), path);
-    }
-    ~NativeFileLock() { ::close(m_fd); }
-    bool try_lock()
-    {
-        if (::flock(m_fd, LOCK_EX | LOCK_NB) == 0)
-            return true;
-        // A signal (a child exiting, for one) interrupts the call like any other; the caller polls again.
-        if (errno == EWOULDBLOCK || errno == EINTR)
-            return false;
-        throw std::system_error(errno, std::generic_category(), "flock");
-    }
-    void unlock() { ::flock(m_fd, LOCK_UN); }
-private:
-    int m_fd;
-};
-#endif
-
-// One slot per lock file, shared by every guard in the process: one lock
-// object per path behind a mutex is what makes the guard re-entrant and safe
-// to use from the preset sync thread and the GUI thread at once. The lock
-// file is opened by the outermost guard and closed when it goes, so the file
-// is never held open between guards: whatever is at the path is what gets
-// locked, and a data dir can be removed once nothing is saving into it. The
-// file is kept rather than deleted on release because the lock state lives in
-// the kernel on the open file, and deleting it would let a third instance
-// lock a fresh file while the second still holds the old one.
-struct InstanceLock::Slot
-{
-    std::recursive_mutex mutex;
-    // Non-null exactly while this process holds the file lock.
-    std::unique_ptr<NativeFileLock> file_lock;
-    int  depth{0};
-    // Until this point, after a guard could not open, lock or wait out the
-    // file, guards do not touch it.
-    std::chrono::steady_clock::time_point cooldown_until{};
-};
-
-InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path)
-{
-    // Never freed: a save during static destruction still needs its slot.
-    static auto *registry_mutex = new std::mutex();
-    static auto *registry       = new std::map<std::string, std::unique_ptr<Slot>>();
-
-    std::lock_guard<std::mutex> guard(*registry_mutex);
-    std::unique_ptr<Slot> &slot = (*registry)[lock_file_path];
-    if (! slot)
-        slot = std::make_unique<Slot>();
-    return *slot;
-}
-
-// Starts the cool-down. Called with the slot mutex held.
-void InstanceLock::defer(Slot &slot, const std::string &reason)
-{
-    slot.cooldown_until = std::chrono::steady_clock::now() + cooldown;
-    BOOST_LOG_TRIVIAL(warning) << reason << "; proceeding without the lock for the next " << cooldown.count() << " ms";
-}
-
-// Creates the lock file if needed and opens it, or starts the cool-down.
-// Called with the slot mutex held.
-bool InstanceLock::open_lock_file(Slot &slot, const std::string &lock_file_path)
-{
-    try {
-#ifdef _WIN32
-        // The lock opens an existing file; created once, on the first miss.
-        const std::wstring wide_path = boost::nowide::widen(lock_file_path);
-        try {
-            slot.file_lock = std::make_unique<NativeFileLock>(wide_path.c_str());
-        } catch (const std::exception &) {
-            boost::nowide::ofstream(lock_file_path, std::ios::app).close();
-            slot.file_lock = std::make_unique<NativeFileLock>(wide_path.c_str());
-        }
-#else
-        slot.file_lock = std::make_unique<NativeFileLock>(lock_file_path.c_str());
-#endif
-        return true;
-    } catch (const std::exception &e) {
-        defer(slot, "Cannot open lock file " + lock_file_path + ": " + e.what() + " (check its owner and permissions)");
-        return false;
-    }
-}
-
-InstanceLock::InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout)
-{
-    if (lock_file_path.empty())
-        return;
-    m_slot       = &slot_for(lock_file_path);
-    m_slot_guard = std::unique_lock<std::recursive_mutex>(m_slot->mutex);
-    const auto now = std::chrono::steady_clock::now();
-    if (m_slot->depth == 0 && now >= m_slot->cooldown_until && open_lock_file(*m_slot, lock_file_path)) {
-        const auto deadline = now + timeout;
-        bool       taken    = false;
-        for (;;) {
-            try {
-                if ((taken = m_slot->file_lock->try_lock()))
-                    break;
-            } catch (const std::exception &e) {
-                defer(*m_slot, "Cannot lock " + lock_file_path + ": " + e.what());
-                break;
-            }
-            if (std::chrono::steady_clock::now() >= deadline) {
-                defer(*m_slot, "Another instance has held " + lock_file_path + " for over " + std::to_string(timeout.count()) + " ms");
-                break;
-            }
-            std::this_thread::sleep_for(std::chrono::milliseconds(5));
-        }
-        if (! taken)
-            m_slot->file_lock.reset();
-    }
-    // Counted last, so a throw above leaves the slot exactly as it was found.
-    ++ m_slot->depth;
-    m_locked = m_slot->file_lock != nullptr;
-}
-
-InstanceLock::~InstanceLock()
-{
-    if (m_slot == nullptr)
-        return;
-    if (-- m_slot->depth == 0 && m_slot->file_lock) {
-        try {
-            m_slot->file_lock->unlock();
-        } catch (const std::exception &e) {
-            BOOST_LOG_TRIVIAL(warning) << "Cannot unlock instance lock: " << e.what();
-        }
-        m_slot->file_lock.reset();
-    }
-}
-
-} // namespace Slic3r

src/libslic3r/InstanceLock.hpp

@@ -1,60 +0,0 @@
-#pragma once
-
-#include <chrono>
-#include <mutex>
-#include <string>
-
-namespace Slic3r {
-
-// Scoped write lock on a file shared by every running instance of the
-// application, such as the app config or the user preset directory: threads
-// of this process are serialised through a recursive mutex, other processes
-// through an advisory OS file lock on `lock_file_path`. The lock file is
-// created on first use and kept; the OS releases the lock when its holder
-// exits, so a crashed instance never leaves a stale lock behind.
-//
-// Best effort: when the lock file cannot be opened or locked, or another
-// instance still holds it after `timeout`, the guard keeps only the in-process
-// mutex, locked() reports false and the write proceeds, since a hung instance
-// must never block another one from saving. For `cooldown` afterwards guards
-// leave the file alone. The wait for the in-process mutex is bounded only by
-// the longest critical section, so a guard covers a few file operations and
-// nothing slower.
-//
-// Lock order: the preset collection mutex may be held when a guard is taken
-// (set_sync_info_and_save() calls save_info() under it), never the reverse;
-// that is why the guards sit at the leaf readers and writers and why a guard
-// must not be added around save_user_presets(), which takes the collection
-// mutex through delete_preset().
-class InstanceLock
-{
-public:
-    // Long against a critical section of milliseconds, short against the GUI
-    // thread, which is where most guards are taken.
-    static constexpr std::chrono::milliseconds default_timeout{1000};
-    // Long enough that a holder stuck in a debugger does not cost a stall per
-    // save; mutable so tests can shorten it.
-    static inline std::chrono::milliseconds cooldown{10000};
-
-    // An empty path makes the guard a no-op.
-    explicit InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout = default_timeout);
-    ~InstanceLock();
-
-    InstanceLock(const InstanceLock &) = delete;
-    InstanceLock &operator=(const InstanceLock &) = delete;
-
-    // True while this process holds the cross-process file lock.
-    bool locked() const { return m_locked; }
-
-private:
-    struct Slot;
-    static Slot &slot_for(const std::string &lock_file_path);
-    static bool  open_lock_file(Slot &slot, const std::string &lock_file_path);
-    static void  defer(Slot &slot, const std::string &reason);
-
-    Slot *m_slot{nullptr};
-    std::unique_lock<std::recursive_mutex> m_slot_guard;
-    bool  m_locked{false};
-};
-
-} // namespace Slic3r

src/libslic3r/LocalesUtils.cpp

@@ -13,19 +13,8 @@
 namespace Slic3r {
 
 
-// How many setters this thread holds, so the ones nested in another can skip
-// setlocale, which takes a lock the whole process shares on Windows.
-static thread_local int s_numeric_locale_depth = 0;
-
 CNumericLocalesSetter::CNumericLocalesSetter()
 {
-    // Nested in another setter on this thread, whose "C" the separator check
-    // confirms is still set.
-    if (s_numeric_locale_depth > 0 && is_decimal_separator_point()) {
-        m_nested = true;
-        ++ s_numeric_locale_depth;
-        return;
-    }
 #ifdef _WIN32
     _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
     m_orig_numeric_locale = std::setlocale(LC_NUMERIC, nullptr);
@@ -40,17 +29,12 @@ CNumericLocalesSetter::CNumericLocalesSetter()
     m_new_locale = newlocale(LC_NUMERIC_MASK, "C", m_new_locale);
     uselocale(m_new_locale);
 #endif
-    // Counted last, since the destructor does not run for a constructor that throws.
-    ++ s_numeric_locale_depth;
 }
 
 
 
 CNumericLocalesSetter::~CNumericLocalesSetter()
 {
-    -- s_numeric_locale_depth;
-    if (m_nested)
-        return;
 #ifdef _WIN32
     std::setlocale(LC_NUMERIC, m_orig_numeric_locale.data());
 #else

src/libslic3r/LocalesUtils.hpp

@@ -19,13 +19,8 @@ class CNumericLocalesSetter {
 public:
     CNumericLocalesSetter();
     ~CNumericLocalesSetter();
-    // A copy would restore the locale twice, and count down once more than up.
-    CNumericLocalesSetter(const CNumericLocalesSetter&) = delete;
-    CNumericLocalesSetter& operator=(const CNumericLocalesSetter&) = delete;
 
 private:
-    // Inside another setter on this thread, which does the setting and restoring.
-    bool m_nested { false };
 #ifdef _WIN32
     std::string m_orig_numeric_locale;
 #else

src/libslic3r/ParallelResolve.hpp

@@ -1,65 +0,0 @@
-#pragma once
-
-#include <algorithm>
-#include <cstddef>
-#include <type_traits>
-#include <utility>
-#include <variant>
-#include <vector>
-
-#include <tbb/blocked_range.h>
-#include <tbb/parallel_for.h>
-#include <tbb/task_arena.h>
-#include <tbb/task_group.h>
-
-#include "Exception.hpp"
-
-namespace Slic3r {
-
-// A batch of resolved presets, each a whole config, is what resolve_then_commit adds
-// to peak memory, so it stays far below a vendor's preset count and above any core count.
-inline constexpr size_t resolve_batch_size = 64;
-
-// Resolve `count` items that do not depend on each other and install them one at
-// a time.
-//
-// `resolve(i)` runs on any thread and must read only, since the items are
-// resolved side by side. `commit(i, resolved)` is called for every item in index
-// order, on the calling thread, and is where shared state is written.
-//
-// The items are worked through in batches, so what is resolved and held at once
-// does not grow with `count`. An exception from either callable propagates after
-// the batches before it have been committed. A cancellation of the caller's task
-// group, which stops a batch partway without an exception, throws RuntimeError
-// before that batch commits.
-//
-// `ChunkSetup`, when given, is constructed once for each piece of a batch TBB hands
-// out, for per-thread state a resolve would otherwise set up per item, such as the
-// C numeric locale, whose setting takes a lock the whole process shares.
-template<class ChunkSetup = std::monostate, class Resolve, class Commit>
-void resolve_then_commit(size_t count, Resolve resolve, Commit commit)
-{
-    using Resolved = std::invoke_result_t<Resolve&, size_t>;
-
-    std::vector<Resolved> resolved(std::min(count, resolve_batch_size));
-    for (size_t first = 0; first < count; first += resolve_batch_size) {
-        const size_t last = std::min(first + resolve_batch_size, count);
-        // Isolated, so a thread waiting on the batch runs none of the caller's other
-        // tasks before finishing it.
-        tbb::this_task_arena::isolate([&] {
-            tbb::parallel_for(tbb::blocked_range<size_t>(first, last),
-                [&](const tbb::blocked_range<size_t>& range) {
-                    ChunkSetup setup;
-                    (void) setup;
-                    for (size_t i = range.begin(); i < range.end(); ++ i)
-                        resolved[i - first] = resolve(i);
-                });
-        });
-        if (tbb::is_current_task_group_canceling())
-            throw RuntimeError("resolve_then_commit: canceled before the batch was resolved");
-        for (size_t i = first; i < last; ++ i)
-            commit(i, std::move(resolved[i - first]));
-    }
-}
-
-} // namespace Slic3r

src/libslic3r/Preset.cpp

@@ -5,8 +5,6 @@
 #include "Preset.hpp"
 #include "PresetBundle.hpp"
 #include "AppConfig.hpp"
-#include "LocalesUtils.hpp"
-#include "ParallelResolve.hpp"
 
 #ifdef _MSC_VER
     #define WIN32_LEAN_AND_MEAN
@@ -51,9 +49,6 @@
 #include "libslic3r.h"
 #include "LifecycleEvents.hpp"
 #include "Utils.hpp"
-#include "InstanceLock.hpp"
-
-#include <sstream>
 #include "Time.hpp"
 #include "PlaceholderParser.hpp"
 #include "libslic3r/GCode/Thumbnails.hpp"
@@ -110,32 +105,6 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const
     }
 }
 
-std::string user_presets_lock_path(bool read_only)
-{
-    return read_only || data_dir().empty() ? std::string() : (fs::path(data_dir()) / (PRESET_USER_DIR ".lock")).string();
-}
-
-// Removes a preset file the scan could not load, and its .info, under the lock.
-// The scan passes read_only when it could not take the lock, since the file may
-// then be another instance's fresh write that it merely raced.
-static void remove_preset_files(const std::string &preset_file, bool read_only)
-{
-    if (read_only)
-        return;
-    InstanceLock instance_lock(user_presets_lock_path());
-    // A symlink whose target is missing stays, since the target may come back; one
-    // that cannot even be followed is removed like any other unreadable file.
-    auto remove = [](const fs::path &file) {
-        boost::system::error_code ec;
-        if (fs::status(file, ec).type() != fs::file_not_found)
-            fs::remove(file, ec);
-    };
-    fs::path file_path(preset_file);
-    remove(file_path);
-    file_path.replace_extension(".info");
-    remove(file_path);
-}
-
 std::string get_preset_bare_name(const std::string &canonical_name)
 {
     const auto pos = canonical_name.find_last_of('/');
@@ -537,11 +506,10 @@ void Preset::normalize(DynamicPrintConfig &config)
     handle_legacy_sla(config);
 }
 
-std::string Preset::remove_invalid_keys(DynamicPrintConfig &config, const DynamicPrintConfig &default_config,
+std::string Preset::remove_invalid_keys(DynamicPrintConfig &config, const DynamicPrintConfig &default_config)
-                                        const DynamicPrintConfig *added)
 {
     std::string incorrect_keys;
-    for (const std::string &key : (added != nullptr ? *added : config).keys())
+    for (const std::string &key : config.keys())
         if (! default_config.has(key)) {
             if (incorrect_keys.empty())
                 incorrect_keys = key;
@@ -653,6 +621,7 @@ void Preset::load_info(const std::string& file)
             }
             else if (v.first.compare("base_id") == 0) {
                 this->base_id = v.second.get_value<std::string>();
+                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load info from: " << file << " and base_id: " << this->base_id;
                 if (this->base_id.compare("null") == 0)
                     this->base_id.clear();
             }
@@ -678,20 +647,18 @@ void Preset::save_info(std::string file)
         file = idx_file.string();
     }
 
+    boost::nowide::ofstream c;
+    c.open(file, std::ios::out | std::ios::trunc);
     std::string sync_info_to_save;
     //BBS: hold is used for stop requesting to server this time
     if (this->sync_info.compare("hold") != 0)
         sync_info_to_save = this->sync_info;
-    std::ostringstream c;
     c << "sync_info" << " = " << sync_info_to_save << std::endl;
     c << "user_id" << " = " << this->user_id << std::endl;
     c << "setting_id" << " = " << this->setting_id << std::endl;
     c << "base_id" << " = " << this->base_id << std::endl;
     c << "updated_time" << " = " << std::to_string(this->updated_time) << std::endl;
-
+    c.close();
-    InstanceLock instance_lock(user_presets_lock_path());
-    if (const std::error_code ec = write_file_atomically(file, c.str()))
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << file << ": " << ec.message();
 }
 
 void Preset::remove_files(bool cloud_already_deleted)
@@ -700,7 +667,6 @@ void Preset::remove_files(bool cloud_already_deleted)
     if (this->is_project_embedded) {
         return;
     }
-    InstanceLock instance_lock(user_presets_lock_path());
     // Erase the preset file.
     boost::nowide::remove(this->file.c_str());
     fs::path idx_path(this->file);
@@ -718,11 +684,11 @@ void Preset::remove_files(bool cloud_already_deleted)
 }
 
 //BBS: add logic for only difference save
-bool Preset::save(DynamicPrintConfig* parent_config)
+void Preset::save(DynamicPrintConfig* parent_config)
 {
     //BBS: add project embedded preset logic
     if (this->is_project_embedded)
-        return true;
+        return;
     //BBS: change to json format
     //this->config.save(this->file);
     std::string from_str;
@@ -737,16 +703,12 @@ bool Preset::save(DynamicPrintConfig* parent_config)
     else
         from_str = std::string("Default");
 
+    boost::filesystem::create_directories(fs::path(this->file).parent_path());
     const std::string bare_name = get_preset_bare_name(this->name);
 
-    // What gets written: the diff against the parent, the config plus its
-    // filament id, or the config as is. Built before the lock is taken so the
-    // exclusive window covers only the file writes.
-    DynamicPrintConfig        temp_config;
-    const DynamicPrintConfig *to_save = &this->config;
-
     //BBS: only save difference if it has parent
     if (parent_config) {
+        DynamicPrintConfig temp_config;
         std::vector<std::string> dirty_options = config.diff(*parent_config);
 
         std::string extruder_id_name, extruder_variant_name;
@@ -782,22 +744,13 @@ bool Preset::save(DynamicPrintConfig* parent_config)
                     opt_dst->set(opt_src);
             }
         }
-        to_save = &temp_config;
+        temp_config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
     } else if (!filament_id.empty() && inherits().empty()) {
-        temp_config = config;
+        DynamicPrintConfig temp_config = config;
         temp_config.set_key_value(BBL_JSON_KEY_FILAMENT_ID, new ConfigOptionString(filament_id));
-        to_save = &temp_config;
+        temp_config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
-    }
+    } else {
-
+        this->config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
-    std::ostringstream json;
-    to_save->save_to_json(json, bare_name, from_str, this->version.to_string());
-
-    InstanceLock instance_lock(user_presets_lock_path());
-    boost::filesystem::create_directories(fs::path(this->file).parent_path());
-    if (const std::error_code ec = write_file_atomically(this->file, json.str())) {
-        // No .info either: one without its preset reads as a cloud deletion request.
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << this->file << ": " << ec.message();
-        return false;
     }
     BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " save config for: " << this->name << " and filament_id: " << filament_id << " and base_id: " << this->base_id;
 
@@ -807,7 +760,6 @@ bool Preset::save(DynamicPrintConfig* parent_config)
         idx_file.replace_extension(".info");
         this->save_info(idx_file.string());
     }
-    return true;
 }
 
 void Preset::reload(Preset const &parent)
@@ -819,7 +771,6 @@ void Preset::reload(Preset const &parent)
     std::string                        reason;
     ForwardCompatibilitySubstitutionRule substitution_rule    = ForwardCompatibilitySubstitutionRule::Disable;
     try {
-        InstanceLock instance_lock(user_presets_lock_path());
         ConfigSubstitutions                config_substitutions = config.load_from_json(file, substitution_rule, key_values, reason);
         this->config = parent.config;
         this->config.apply(std::move(config));
@@ -1718,185 +1669,6 @@ std::string PresetCollection::canonical_preset_name(const std::string &name, con
     return get_preset_canonical_name(parsed.bare, origin);
 }
 
-PresetCollection::PresetFilesOnDisk PresetCollection::PresetFilesOnDisk::read(const boost::filesystem::path &file)
-{
-    auto read_if_present = [](const fs::path &path) -> std::optional<std::string> {
-        std::string bytes;
-        try {
-            load_string_file(path, bytes);
-        } catch (const std::exception &) {
-            return std::nullopt;
-        }
-        return bytes;
-    };
-    fs::path info_path(file);
-    info_path.replace_extension(".info");
-    return { read_if_present(file), read_if_present(info_path) };
-}
-
-PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
-    const boost::filesystem::path &file, const std::string &canonical_name,
-    const PresetOrigin &load_origin, ForwardCompatibilitySubstitutionRule substitution_rule,
-    const std::string &extruder_id_name, const std::string &extruder_variant_name,
-    std::set<std::string> *key_set1, std::set<std::string> *key_set2) const
-{
-    UserPresetLoad out;
-    out.preset = Preset(m_type, canonical_name, false);
-    Preset &preset = out.preset;
-    preset.bundle_id = load_origin.bundle_id;
-    preset.file = file.string();
-    // Before either file is parsed, so a save that lands during the parse still shows
-    // up as a difference when commit compares.
-    out.on_disk = PresetFilesOnDisk::read(file);
-    // Load the preset file, apply preset values on top of defaults.
-    try {
-        fs::path idx_path(preset.file);
-        idx_path.replace_extension(".info");
-        if (fs::exists(idx_path)) {
-            out.info_file = idx_path.string();
-            preset.load_info(out.info_file);
-        }
-        DynamicPrintConfig config;
-        //BBS: change to json format
-        //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
-        std::map<std::string, std::string> key_values;
-        std::string reason;
-        ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
-        if (! config_substitutions.empty())
-            out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
-        if (!reason.empty()) {
-            out.discard_file = true;
-            out.errors.push_back((boost::format("parse config %1% failed") % preset.file).str());
-            return out;
-        }
-
-        std::string version_str = key_values[BBL_JSON_KEY_VERSION];
-        boost::optional<Semver> version = Semver::parse(version_str);
-        if (!version) return out;
-        preset.version = *version;
-
-        if (key_values.find(BBL_JSON_KEY_FILAMENT_ID) != key_values.end())
-            preset.filament_id = key_values[BBL_JSON_KEY_FILAMENT_ID];
-        if (key_values.find(BBL_JSON_KEY_DESCRIPTION) != key_values.end())
-            preset.description = key_values[BBL_JSON_KEY_DESCRIPTION];
-        if (key_values.find(BBL_JSON_KEY_INSTANTIATION) != key_values.end())
-            preset.is_visible = key_values[BBL_JSON_KEY_INSTANTIATION] != "false";
-
-        //Orca: find and use the inherit config as the base
-        const Preset* inherit_preset = nullptr;
-        ConfigOption* inherits_config = config.option(BBL_JSON_KEY_INHERITS);
-
-        // check inherits_config
-        if (inherits_config) {
-            ConfigOptionString * option_str = dynamic_cast<ConfigOptionString *> (inherits_config);
-            std::string inherits_value = option_str->value;
-            // Orca: try to find if the parent preset has been renamed
-            inherit_preset = this->find_preset2(inherits_value);
-            Preset::normalize_inherits(config, inherit_preset);
-        }
-        const Preset& default_preset = this->default_preset_for(config);
-        if (inherit_preset) {
-            preset.config = inherit_preset->config;
-            preset.filament_id = inherit_preset->filament_id;
-            extend_default_config_length(config, false, {});
-            preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
-        }
-        else {
-            auto inherits_config2 = dynamic_cast<ConfigOptionString *>(inherits_config);
-            if ((inherits_config2 && !inherits_config2->value.empty())) {
-                out.errors.push_back((boost::format("can not find parent %1% for config %2%!") % inherits_config2->value % preset.file).str());
-                return out;
-            }
-            // We support custom root preset now
-            // Find a default preset for the config. The PrintPresetCollection provides different default preset based on the "printer_technology" field.
-            preset.config = default_preset.config;
-            preset.config.apply(std::move(config));
-            extend_default_config_length(preset.config, true, default_preset.config);
-        }
-
-        Preset::normalize(preset.config);
-        // Report configuration fields, which are misplaced into a wrong group.
-        std::string incorrect_keys = Preset::remove_invalid_keys(preset.config, default_preset.config);
-        if (!incorrect_keys.empty())
-            out.errors.push_back("Error in a preset file: The preset \"" + preset.file +
-                                 "\" contains the following incorrect keys: " + incorrect_keys + ", which were removed");
-
-        if (preset.type == Preset::TYPE_FILAMENT && preset.is_user() && preset.inherits().empty()) {
-            auto compatible_printers = dynamic_cast<ConfigOptionStrings *>(preset.config.option("compatible_printers", true));
-            if (compatible_printers && compatible_printers->values.empty()) {
-                size_t at_pos = canonical_name.find('@');
-                if (at_pos != std::string::npos && at_pos + 1 < canonical_name.length()) {
-                    compatible_printers->values.push_back(canonical_name.substr(at_pos + 1));
-                    out.save_compatible_printers = true;
-                }
-            }
-        }
-
-        preset.loaded = true;
-        out.complete = true;
-    } catch (const std::ifstream::failure &err) {
-        out.discard_file = true;
-        out.errors.push_back((boost::format("The user-config cannot be loaded: %1%. Reason: %2%") % preset.file % err.what()).str());
-        //throw Slic3r::RuntimeError(std::string("The selected preset cannot be loaded: ") + preset.file + "\n\tReason: " + err.what());
-    } catch (const std::runtime_error &err) {
-        out.discard_file = true;
-        out.errors.push_back((boost::format("Failed loading the user-config file: %1%. Reason: %2%") % preset.file % err.what()).str());
-        //throw Slic3r::RuntimeError(std::string("Failed loading the preset file: ") + preset.file + "\n\tReason: " + err.what());
-    }
-    out.install = true;
-    return out;
-}
-
-void PresetCollection::commit_user_preset(UserPresetLoad &&loaded, std::deque<Preset> &presets_loaded,
-                                          PresetsConfigSubstitutions &substitutions,
-                                          const std::function<void(Preset&)> &preset_loaded_fn,
-                                          bool read_only)
-{
-    Preset &preset = loaded.preset;
-    if (! loaded.info_file.empty())
-        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load info from: " << loaded.info_file << " and base_id: " << preset.base_id;
-    append(substitutions, std::move(loaded.substitutions));
-    for (const std::string &error : loaded.errors) {
-        ++m_errors;
-        BOOST_LOG_TRIVIAL(error) << error;
-    }
-    if (loaded.discard_file)
-        remove_preset_files(loaded.preset.file, read_only);
-    if (!loaded.install)
-        return;
-
-    if (loaded.complete) {
-        if (loaded.save_compatible_printers) {
-            // A filesystem error from the rewrite is counted, and the preset still loads.
-            try {
-                if (read_only)
-                    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << preset.name << " (not written back)";
-                else if (preset.save(nullptr))
-                    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << preset.name;
-                else
-                    ++m_errors; // save() logged why
-            } catch (const std::runtime_error &err) {
-                ++m_errors;
-                BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " can not write compatible_printers back to " << preset.file << ": " << err.what();
-            }
-        }
-        //BBS: add some workaround for previous incorrect settings
-        if ((!preset.setting_id.empty())&&(preset.setting_id == preset.base_id))
-            preset.setting_id.clear();
-        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load preset: " << preset.name << " and filament_id: " << preset.filament_id << " and base_id: " << preset.base_id;
-        //BBS: add config related logs
-        BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(", preset type %1%, name %2%, path %3%, is_system %4%, is_default %5% is_visible %6%")%Preset::get_type_string(m_type) %preset.name %preset.file %preset.is_system %preset.is_default %preset.is_visible;
-        // add alias for custom filament preset
-        set_custom_preset_alias(preset);
-    }
-
-    if (preset_loaded_fn != nullptr)
-        preset_loaded_fn(preset);
-
-    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << __LINE__ << " load config successful and preset name is:" << preset.name;
-    presets_loaded.emplace_back(std::move(preset));
-}
-
 // Load all presets found in dir_path.
 // Throws an exception on error.
 void PresetCollection::load_presets(
@@ -1934,8 +1706,6 @@ void PresetCollection::load_presets(
     std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
     Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
 
-    struct UserPresetFile { fs::path path; std::string canonical_name; };
-    std::vector<UserPresetFile> files;
     //BBS: change to json format
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
     {
@@ -1951,47 +1721,149 @@ void PresetCollection::load_presets(
                 BOOST_LOG_TRIVIAL(warning) << "Preset already present, not loading: " << canonical_name;
                 continue;
             }
-            files.push_back({ dir_entry.path(), std::move(canonical_name) });
+            try {
+                Preset preset(m_type, canonical_name, false);
+                preset.bundle_id = resolved_origin.bundle_id;
+                preset.file = dir_entry.path().string();
+                // Load the preset file, apply preset values on top of defaults.
+                try {
+                    fs::path idx_path(preset.file);
+                    idx_path.replace_extension(".info");
+                    if (fs::exists(idx_path)) {
+                        preset.load_info(idx_path.string());
+                    }
+                    DynamicPrintConfig config;
+                    //BBS: change to json format
+                    //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
+                    std::map<std::string, std::string> key_values;
+                    std::string reason;
+                    ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
+                    if (! config_substitutions.empty())
+                        substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
+                    if (!reason.empty()) {
+                        fs::path file_path(preset.file);
+                        if (!read_only && fs::exists(file_path))
+                            fs::remove(file_path);
+                        file_path.replace_extension(".info");
+                        if (!read_only && fs::exists(file_path))
+                            fs::remove(file_path);
+                        BOOST_LOG_TRIVIAL(error) << boost::format("parse config %1% failed")%preset.file;
+                        ++m_errors;
+                        continue;
+                    }
+
+                    std::string version_str = key_values[BBL_JSON_KEY_VERSION];
+                    boost::optional<Semver> version = Semver::parse(version_str);
+                    if (!version) continue;
+                    preset.version = *version;
+
+                    if (key_values.find(BBL_JSON_KEY_FILAMENT_ID) != key_values.end())
+                        preset.filament_id = key_values[BBL_JSON_KEY_FILAMENT_ID];
+                    if (key_values.find(BBL_JSON_KEY_DESCRIPTION) != key_values.end())
+                        preset.description = key_values[BBL_JSON_KEY_DESCRIPTION];
+                    if (key_values.find(BBL_JSON_KEY_INSTANTIATION) != key_values.end())
+                        preset.is_visible = key_values[BBL_JSON_KEY_INSTANTIATION] != "false";
+
+                    //Orca: find and use the inherit config as the base
+                    Preset* inherit_preset = nullptr;
+                    ConfigOption* inherits_config = config.option(BBL_JSON_KEY_INHERITS);
+
+                    // check inherits_config
+                    if (inherits_config) {
+                        ConfigOptionString * option_str = dynamic_cast<ConfigOptionString *> (inherits_config);
+                        std::string inherits_value = option_str->value;
+                        // Orca: try to find if the parent preset has been renamed
+                        inherit_preset = this->find_preset2(inherits_value);
+                        Preset::normalize_inherits(config, inherit_preset);
+                    } else {
+                        ;
+                    }
+                    const Preset& default_preset = this->default_preset_for(config);
+                    if (inherit_preset) {
+                        preset.config = inherit_preset->config;
+                        preset.filament_id = inherit_preset->filament_id;
+                        extend_default_config_length(config, false, {});
+                        preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
+                    }
+                    else {
+                        auto inherits_config2 = dynamic_cast<ConfigOptionString *>(inherits_config);
+                        if ((inherits_config2 && !inherits_config2->value.empty())) {
+                            BOOST_LOG_TRIVIAL(error) << boost::format("can not find parent %1% for config %2%!")%inherits_config2->value %preset.file;
+                            ++m_errors;
+                            continue;
+                        }
+                        // We support custom root preset now
+                        // Find a default preset for the config. The PrintPresetCollection provides different default preset based on the "printer_technology" field.
+                        preset.config = default_preset.config;
+                        preset.config.apply(std::move(config));
+                        extend_default_config_length(preset.config, true, default_preset.config);
+                    }
+                    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load preset: " << name << " and filament_id: " << preset.filament_id << " and base_id: " << preset.base_id;
+
+                    Preset::normalize(preset.config);
+                    // Report configuration fields, which are misplaced into a wrong group.
+                    std::string incorrect_keys = Preset::remove_invalid_keys(preset.config, default_preset.config);
+                    if (!incorrect_keys.empty()) {
+                        ++m_errors;
+                        BOOST_LOG_TRIVIAL(error)
+                            << "Error in a preset file: The preset \"" << preset.file
+                            << "\" contains the following incorrect keys: " << incorrect_keys << ", which were removed";
+                    }
+
+                    if (preset.type == Preset::TYPE_FILAMENT && preset.is_user() && preset.inherits().empty()) {
+                        auto compatible_printers = dynamic_cast<ConfigOptionStrings *>(preset.config.option("compatible_printers", true));
+                        if (compatible_printers && compatible_printers->values.empty()) {
+                            size_t at_pos = name.find('@');
+                            if (at_pos != std::string::npos && at_pos + 1 < name.length()) {
+                                compatible_printers->values.push_back(name.substr(at_pos + 1));
+                                if (!read_only)
+                                    preset.save(nullptr);
+                                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << name;
+                            }
                         }
                     }
 
-    resolve_then_commit<CNumericLocalesSetter>(files.size(),
+                    preset.loaded = true;
-        [&](size_t i) {
+                    //BBS: add some workaround for previous incorrect settings
-            return this->resolve_user_preset(files[i].path, files[i].canonical_name, resolved_origin, substitution_rule,
+                    if ((!preset.setting_id.empty())&&(preset.setting_id == preset.base_id))
-                                             extruder_id_name, extruder_variant_name, key_set1, key_set2);
+                        preset.setting_id.clear();
-        },
+                    //BBS: add config related logs
-        [&](size_t i, UserPresetLoad &&loaded) {
+                    BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(", preset type %1%, name %2%, path %3%, is_system %4%, is_default %5% is_visible %6%")%Preset::get_type_string(m_type) %preset.name %preset.file %preset.is_system %preset.is_default %preset.is_visible;
-            // Resolve read the files without the lock, so another instance may have saved
+                    // add alias for custom filament preset
-            // over them since. A file that changed is resolved again under the lock before
+                    set_custom_preset_alias(preset);
-            // commit removes or rewrites it, which also keeps its .json and .info from two
+                } catch (const std::ifstream::failure &err) {
-            // different saves apart. Without the lock nothing is checked, so commit leaves
+                    ++m_errors;
-            // the files alone.
+                    BOOST_LOG_TRIVIAL(error) << boost::format("The user-config cannot be loaded: %1%. Reason: %2%")%preset.file %err.what();
-            const std::string lock_path = user_presets_lock_path(read_only);
+                    fs::path file_path(preset.file);
-            InstanceLock      instance_lock(lock_path);
+                    if (!read_only && fs::exists(file_path))
-            if (instance_lock.locked()) {
+                        fs::remove(file_path);
-                const PresetFilesOnDisk   on_disk = PresetFilesOnDisk::read(files[i].path);
+                    file_path.replace_extension(".info");
-                boost::system::error_code ec;
+                    if (!read_only && fs::exists(file_path))
-                // The link itself, and a stat error counts as present, so a file that is
+                        fs::remove(file_path);
-                // there but cannot be read is still counted and removed by commit.
+                    //throw Slic3r::RuntimeError(std::string("The selected preset cannot be loaded: ") + preset.file + "\n\tReason: " + err.what());
-                if (! on_disk.json && fs::symlink_status(files[i].path, ec).type() == fs::file_not_found)
+                } catch (const std::runtime_error &err) {
-                    return; // removed by another instance since it was read
+                    ++m_errors;
-                if (! (on_disk == loaded.on_disk)) {
+                    BOOST_LOG_TRIVIAL(error) << boost::format("Failed loading the user-config file: %1%. Reason: %2%")%preset.file %err.what();
-                    CNumericLocalesSetter locales_setter;
+                    //throw Slic3r::RuntimeError(std::string("Failed loading the preset file: ") + preset.file + "\n\tReason: " + err.what());
-                    loaded = this->resolve_user_preset(files[i].path, files[i].canonical_name, resolved_origin, substitution_rule,
+                    fs::path file_path(preset.file);
-                                                       extruder_id_name, extruder_variant_name, key_set1, key_set2);
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
+                    file_path.replace_extension(".info");
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
                 }
-            }
+
-            const bool leave_files = read_only || (! lock_path.empty() && ! instance_lock.locked());
+                if (preset_loaded_fn != nullptr)
-            // Committing can remove an unreadable preset's file, and a filesystem error
+                    preset_loaded_fn(preset);
-            // there is reported without stopping the rest of the directory.
+
-            try {
+                presets_loaded.emplace_back(preset);
-                this->commit_user_preset(std::move(loaded), presets_loaded, substitutions, preset_loaded_fn, leave_files);
+                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << __LINE__ << " load config successful and preset name is:" << preset.name;
             } catch (const std::runtime_error &err) {
                 errors_cummulative += err.what();
                 errors_cummulative += "\n";
             }
-        });
+        }
-
+    }
     if (presets_loaded.size() > 0)
         m_presets.insert(m_presets.end(), std::make_move_iterator(presets_loaded.begin()), std::make_move_iterator(presets_loaded.end()));
     sort_presets();
@@ -2295,10 +2167,7 @@ void PresetCollection::set_sync_info_and_save(std::string name, std::string sett
             preset->setting_id = setting_id;
             if (update_time > 0)
                 preset->updated_time = update_time;
-            if (preset->sync_info == "update")
+            preset->sync_info == "update" ? preset->save(nullptr) : preset->save_info();
-                preset->save(nullptr);
-            else
-                preset->save_info();
             break;
         }
     }
@@ -2958,18 +2827,6 @@ std::pair<Preset*, bool> PresetCollection::load_external_preset(
     return std::make_pair(&preset, false);
 }
 
-Preset& PresetCollection::append_preset(std::string &&path, const std::string &name, DynamicPrintConfig &&config)
-{
-    lock();
-    Preset &preset = m_presets.emplace_back(m_type, name, false);
-    preset.file = std::move(path);
-    preset.config = std::move(config);
-    preset.loaded = true;
-    preset.is_dirty = false;
-    unlock();
-    return preset;
-}
-
 Preset& PresetCollection::load_preset(const std::string &path, const std::string &name, DynamicPrintConfig &&config, bool select, Semver file_version)
 {
     lock();
@@ -3989,53 +3846,28 @@ bool PresetCollection::select_preset_by_name_strict(const std::string &name)
     return false;
 }
 
-std::vector<std::vector<std::string>> PresetCollection::merge_presets(const std::vector<PresetCollection*> &others, const VendorMap &new_vendors)
+// Merge one vendor's presets with the other vendor's presets, report duplicates.
+std::vector<std::string> PresetCollection::merge_presets(PresetCollection &&other, const VendorMap &new_vendors)
 {
-    auto less = [this](const Preset &a, const Preset &b) {
+    std::vector<std::string> duplicates;
-        return m_type == Preset::TYPE_FILAMENT ? filament_preset_less(a, b) : a < b;
+    for (Preset &preset : other.m_presets) {
-    };
+        if (preset.is_default || preset.is_external)
-    struct Incoming { Preset *preset; size_t source; };
+            continue;
-    auto incoming_less = [&less](const Incoming &a, const Incoming &b) { return less(*a.preset, *b.preset); };
+        Preset key(m_type, preset.name);
-    // Each of `others` is sorted, so its presets form one sorted run.
+        auto it = (m_type == Preset::TYPE_FILAMENT)
-    std::vector<Incoming> incoming;
+            ? std::lower_bound(m_presets.begin() + m_num_default_presets, m_presets.end(), key, filament_preset_less)
-    std::vector<size_t>   run_ends { 0 };
+            : std::lower_bound(m_presets.begin() + m_num_default_presets, m_presets.end(), key);
-    for (size_t source = 0; source < others.size(); ++ source) {
+        if (it == m_presets.end() || it->name != preset.name) {
-        for (Preset &preset : others[source]->m_presets)
-            if (! preset.is_default && ! preset.is_external)
-                incoming.push_back({ &preset, source });
-        assert(std::is_sorted(incoming.begin() + run_ends.back(), incoming.end(), incoming_less));
-        run_ends.push_back(incoming.size());
-    }
-    // Merged pairwise and stably, so equal names stay in the order of `others`.
-    const size_t runs = others.size();
-    for (size_t width = 1; width < runs; width *= 2)
-        for (size_t i = 0; i + width < runs; i += 2 * width)
-            std::inplace_merge(incoming.begin() + run_ends[i], incoming.begin() + run_ends[i + width],
-                               incoming.begin() + run_ends[std::min(i + 2 * width, runs)], incoming_less);
-
-    std::vector<std::vector<std::string>> duplicates(others.size());
-    std::deque<Preset> merged;
-    auto own = m_presets.begin() + m_num_default_presets;
-    std::move(m_presets.begin(), own, std::back_inserter(merged));
-    // On equal names this collection's preset is kept, else the earliest of `others`,
-    // and each repeat is listed under the collection it came from.
-    for (auto next = incoming.begin(); own != m_presets.end() || next != incoming.end();) {
-        if (next == incoming.end() || (own != m_presets.end() && ! less(*next->preset, *own)))
-            merged.emplace_back(std::move(*own ++));
-        else {
-            Preset &preset = *(next ++)->preset;
             if (preset.vendor != nullptr) {
                 // Re-assign a pointer to the vendor structure in the new PresetBundle.
                 auto it = new_vendors.find(preset.vendor->id);
                 assert(it != new_vendors.end());
                 preset.vendor = &it->second;
             }
-            merged.emplace_back(std::move(preset));
+            m_presets.emplace(it, std::move(preset));
+        } else
+            duplicates.emplace_back(std::move(preset.name));
     }
-        for (; next != incoming.end() && next->preset->name == merged.back().name; ++ next)
-            duplicates[next->source].emplace_back(next->preset->name);
-    }
-    m_presets = std::move(merged);
     return duplicates;
 }
 
@@ -4380,15 +4212,8 @@ void PhysicalPrinter::update_preset_names_in_config()
     }
 }
 
-void PhysicalPrinter::save(DynamicPrintConfig* /* parent_config */)
-{
-    InstanceLock instance_lock(user_presets_lock_path());
-    this->config.save_to_json(this->file, std::string("Physical_Printer"), std::string("User"), std::string(SLIC3R_VERSION));
-}
-
 void PhysicalPrinter::save(const std::string& file_name_from, const std::string& file_name_to)
 {
-    InstanceLock instance_lock(user_presets_lock_path());
     // rename the file
     boost::nowide::rename(file_name_from.data(), file_name_to.data());
     this->file = file_name_to;
@@ -4520,7 +4345,6 @@ void PhysicalPrinterCollection::load_printers(
                 continue;
             }
             try {
-                InstanceLock instance_lock(user_presets_lock_path());
                 PhysicalPrinter printer(name, this->default_config());
                 printer.file = dir_entry.path().string();
                 // Load the preset file, apply preset values on top of defaults.
@@ -4713,10 +4537,7 @@ bool PhysicalPrinterCollection::delete_printer(const std::string& name)
 
     const PhysicalPrinter& printer = *it;
     // Erase the preset file.
-    {
-        InstanceLock instance_lock(user_presets_lock_path());
     boost::nowide::remove(printer.file.c_str());
-    }
     m_printers.erase(it);
     return true;
 }
@@ -4728,10 +4549,7 @@ bool PhysicalPrinterCollection::delete_selected_printer()
     const PhysicalPrinter& printer = this->get_selected_printer();
 
     // Erase the preset file.
-    {
-        InstanceLock instance_lock(user_presets_lock_path());
     boost::nowide::remove(printer.file.c_str());
-    }
     // Remove the preset from the list.
     m_printers.erase(m_printers.begin() + m_idx_selected);
     // unselect all printers

src/libslic3r/Preset.hpp

@@ -8,7 +8,6 @@
 #include <unordered_set>
 #include <functional>
 #include <mutex>
-#include <optional>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/filesystem/path.hpp>
 #include <boost/property_tree/ptree_fwd.hpp>
@@ -331,8 +330,7 @@ public:
 
     //BBS: add logic for only difference save
     //if parent_config is null, save all keys, otherwise, only save difference
-    // Returns false when the preset file could not be written.
+    void                save(DynamicPrintConfig* parent_config);
-    bool                save(DynamicPrintConfig* parent_config);
     void                reload(Preset const & parent);
 
     // Return a label of this preset, consisting of a name and a "(modified)" suffix, if this preset is dirty.
@@ -447,10 +445,7 @@ public:
     static std::string                      remove_suffix_modified(const std::string& name);
     static void                             normalize(DynamicPrintConfig &config);
     // Report configuration fields, which are misplaced into a wrong group, remove them from the config.
-    // `added`, when given, is the diff applied over a copy of default_config, and only
+    static std::string                      remove_invalid_keys(DynamicPrintConfig &config, const DynamicPrintConfig &default_config);
-    // its keys are checked, since no other key can be missing from default_config.
-    static std::string                      remove_invalid_keys(DynamicPrintConfig &config, const DynamicPrintConfig &default_config,
-                                                                const DynamicPrintConfig *added = nullptr);
 
     // BBS: move constructor to public
     Preset(Type type, const std::string &name, bool is_default = false) : type(type), is_default(is_default), name(name) {}
@@ -490,12 +485,6 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const
 // Tail segment of a canonical name — what's written to the bundle's .json filename and JSON "name" field.
 std::string get_preset_bare_name(const std::string &canonical_name);
 
-// Lock file guarding every user preset file under data_dir() against other
-// running instances and the preset sync thread. Empty without a data dir, and
-// for a read-only load (the CLI), which never rewrites or deletes and may run
-// many jobs on one data dir.
-std::string user_presets_lock_path(bool read_only = false);
-
 // Resolve an origin from a directory path when the caller passes Kind::Auto.
 PresetOrigin detect_origin_from_path(const boost::filesystem::path &path, const PresetOrigin &explicit_origin = PresetOrigin());
 
@@ -812,8 +801,6 @@ public:
     // Return number of presets including the "- default -" preset.
     size_t          size() const                { return m_presets.size(); }
     bool            has_defaults_only() const   { return m_presets.size() <= m_num_default_presets; }
-    // How many presets this collection refused or repaired while loading.
-    int             error_count() const         { return m_errors; }
 
     // For Print / Filament presets, disable those, which are not compatible with the printer.
     template<typename PreferedCondition>
@@ -887,10 +874,8 @@ protected:
     // This is a temporary state, which shall be fixed immediately by the following step.
     bool            select_preset_by_name_strict(const std::string &name);
 
-    // Move the presets of `others` into this collection in one pass. A name this
+    // Merge one vendor's presets with the other vendor's presets, report duplicates.
-    // collection or an earlier one of `others` already has is left out, and reported
+    std::vector<std::string> merge_presets(PresetCollection &&other, const VendorMap &new_vendors);
-    // in the list of the collection that repeats it.
-    std::vector<std::vector<std::string>> merge_presets(const std::vector<PresetCollection*> &others, const VendorMap &new_vendors);
 
     // Update m_map_alias_to_profile_name from loaded system profiles.
 	void 			update_map_alias_to_profile_name();
@@ -905,56 +890,6 @@ protected:
     void            set_custom_preset_alias(Preset &preset);
 
 private:
-    // A preset file and its .info as read from disk, std::nullopt for one that is missing.
-    struct PresetFilesOnDisk
-    {
-        std::optional<std::string> json;
-        std::optional<std::string> info;
-
-        static PresetFilesOnDisk read(const boost::filesystem::path &file);
-        bool operator==(const PresetFilesOnDisk &rhs) const { return json == rhs.json && info == rhs.info; }
-    };
-
-    // One preset file read and flattened against the presets already in this
-    // collection, before anything the collection shares has been touched.
-    struct UserPresetLoad
-    {
-        Preset      preset;
-        // Joins the collection. A file that threw partway still joins it, without
-        // the steps that did not run.
-        bool        install { false };
-        // The whole of the load ran, so the preset is ready to be aliased.
-        bool        complete { false };
-        // A filament preset that named no compatible printer and was given one from
-        // its name, which commit writes back to its file.
-        bool        save_compatible_printers { false };
-        // Unreadable, so commit removes it and its .info file.
-        bool        discard_file { false };
-        // The .info file read beside the preset, which commit logs.
-        std::string info_file;
-        // Both files as they were before the preset was read from them. Resolve runs
-        // without the instance lock, so commit compares this with the disk under it.
-        PresetFilesOnDisk on_disk;
-        // Counted and logged by commit, in the order the directory listed the files.
-        std::vector<std::string>   errors;
-        PresetsConfigSubstitutions substitutions;
-    };
-
-    // Read and flatten one preset file. It reads only, and resolves against the presets
-    // loaded before this pass, never another file of the same pass, so the files of a
-    // pass are independent of each other.
-    UserPresetLoad  resolve_user_preset(const boost::filesystem::path &file, const std::string &canonical_name,
-                                        const PresetOrigin &load_origin, ForwardCompatibilitySubstitutionRule substitution_rule,
-                                        const std::string &extruder_id_name, const std::string &extruder_variant_name,
-                                        std::set<std::string> *key_set1, std::set<std::string> *key_set2) const;
-
-    // Install one resolved preset. The collection, its alias maps, the error count
-    // and the preset files on disk are touched here and only here.
-    void            commit_user_preset(UserPresetLoad &&loaded, std::deque<Preset> &presets_loaded,
-                                       PresetsConfigSubstitutions &substitutions,
-                                       const std::function<void(Preset&)> &preset_loaded_fn,
-                                       bool read_only);
-
     std::string canonical_preset_name(const std::string &name, const PresetOrigin &load_origin = PresetOrigin()) const;
 
     // Comparator that sorts "Generic " prefixed presets before others, then alphabetically within each group.
@@ -966,10 +901,6 @@ private:
         return a.name < b.name;
     }
 
-    // Append a preset without keeping the collection sorted, for a caller installing
-    // many at once; find_preset() is unusable until sort_presets() runs.
-    Preset& append_preset(std::string &&path, const std::string &name, DynamicPrintConfig &&config);
-
     // Sort presets: filament presets use generic-first ordering, others sort alphabetically.
     void sort_presets() {
         if (m_type == Preset::TYPE_FILAMENT)
@@ -1123,7 +1054,7 @@ public:
 
     //BBS: change to json format
     //void                save() { this->config.save(this->file); }
-    void                save(DynamicPrintConfig* parent_config);
+    void                save(DynamicPrintConfig* parent_config) { this->config.save_to_json(this->file, std::string("Physical_Printer"), std::string("User"), std::string(SLIC3R_VERSION)); }
     void                save(const std::string& file_name_from, const std::string& file_name_to);
 
     void                update_from_preset(const Preset& preset);

src/libslic3r/PresetBundle.hpp

@@ -15,7 +15,6 @@
 #include <unordered_map>
 #include <optional>
 #include <array>
-#include <atomic>
 #include <boost/filesystem/path.hpp>
 #include <unordered_set>
 
@@ -623,180 +622,36 @@ public:
     // default_filament_profile must resolve to a system filament.
     bool check_printer_default_materials() const;
 
-    // One vendor to load, and the directory it is installed in.
+    // Merge one vendor's presets with the other vendor's presets, report duplicates.
-    struct VendorSource
+    // Public so per-vendor-cache consumers (e.g. the setup wizard) can assemble a
-    {
+    // bundle out of several per-vendor caches loaded into separate PresetBundle instances.
-        std::string             name;
+    std::vector<std::string>    merge_presets(PresetBundle &&other);
-        boost::filesystem::path dir;
-    };
-
-    // Load `vendors` into this bundle, the Orca filament library directly and every
-    // other vendor in parallel into a bundle of its own that inherits from it, merged
-    // in the order given. A vendor that cannot be loaded has its error added to the
-    // returned text, or thrown in validation mode, and its name to `failed`; it is
-    // left out, except for the library, which keeps what it installed before the
-    // failure. Once `cancel` is set, no further vendor starts loading.
-    std::pair<PresetsConfigSubstitutions, std::string> load_vendors(const std::vector<VendorSource>& vendors,
-        ForwardCompatibilitySubstitutionRule compatibility_rule, bool allow_cache,
-        const std::atomic<bool>* cancel = nullptr, std::vector<std::string>* failed = nullptr);
 
 private:
-    // Move the presets and vendor profiles of `others` into this bundle, in one pass
+    // Load one vendor from the preset cache installed in `dir`, judged against
-    // over each collection. A preset whose name this bundle or an earlier one of
+    // the vendor profile there. False, with this bundle left clean, when there
-    // `others` already has is left out and listed under the bundle that repeats it.
+    // is no usable cache and the vendor has to be parsed. This is how
-    std::vector<std::vector<std::string>> merge_presets(const std::vector<PresetBundle*> &others);
+    // load_vendor_configs_from_json reads a cache.
+    bool load_vendor_cache(const boost::filesystem::path& dir, const std::string& vendor_name, const PresetBundle* base_bundle);
 
-    // What parsing one entry's JSON sub-file reported. Its errors and warnings are
+    // Load one source-form preset entry into this bundle: resolve `inherits`
-    // logged when the entry installs, so they come out in listing order with the
+    // and `include`, flatten, validate and register the preset. Returns the
-    // entry's install errors, as parsing and installing one entry at a time leaves them.
+    // reason loading failed, empty on success. See the definition for the
-    struct EntryParse
+    // sharing contract between the JSON parse and the cache load.
-    {
+    // retain_configs / retain_includes, when non-null, name the only presets
-        ConfigSubstitutions      substitutions;
+    // registered into config_maps / include_maps (a config copy each). The
-        // Counted in the bundle's error count.
+    // cache load passes the names its entries inherit / include — the only
-        std::vector<std::string> errors;
+    // ones ever looked up again; the JSON parse retains all, not knowing what
-        std::vector<std::string> warnings;
+    // later subfiles name.
-    };
+    std::string load_vendor_preset(const CachedPreset& entry,
-
+        const std::string& path, const std::string& vendor_name,
-    // An EntryParse for each entry of the VendorCacheData list of the same name.
+        const PresetBundle* base_bundle,
-    struct VendorParse
+        LoadConfigBundleAttributes flags,
-    {
+        ConfigSubstitutionContext& substitution_context, PresetsConfigSubstitutions& substitutions,
-        std::vector<EntryParse> process_entries;
+        std::map<std::string, DynamicPrintConfig>& config_maps, std::map<std::string, DynamicPrintConfig>& include_maps,
-        std::vector<EntryParse> filament_entries;
+        std::map<std::string, std::string>& filament_id_maps,
-        std::vector<EntryParse> machine_entries;
+        PresetCollection* presets_collection, size_t& count, bool is_from_lib,
-    };
+        const std::set<std::string>* retain_configs = nullptr, const std::set<std::string>* retain_includes = nullptr);
-
-    // One vendor read from its cache or its JSONs by read_vendor, for
-    // install_vendor_read to install.
-    struct VendorRead
-    {
-        std::string                          dir;
-        std::string                          vendor_name;
-        LoadConfigBundleAttributes           flags;
-        ForwardCompatibilitySubstitutionRule compatibility_rule;
-        // The errors this bundle had counted before the read, which the cache stamp leaves out.
-        int                                  errors_at_entry { 0 };
-        // A whole-vendor load, which can read a cache and write one.
-        bool                                 cacheable { false };
-        // Read from the cache at cache_path, which install can still reject.
-        bool                                 from_cache { false };
-        std::string                          cache_path;
-        // Only the vendor profile was asked for.
-        bool                                 vendor_only { false };
-        VendorCacheData                      data;
-        // What each entry's JSON parse reported, and whether and with which version
-        // the cache is written once the entries install.
-        VendorParse                          parsed;
-        bool                                 will_cache { false };
-        std::string                          version;
-        // The sub-file the parse stopped at, its kind, why, and the errors it reported.
-        std::string                          reason;
-        std::string                          failed_subfile;
-        const char*                          failed_kind { nullptr };
-        std::vector<std::string>             failed_errors;
-    };
-
-    // Read a vendor into this bundle's vendor profiles and `data`, from its cache
-    // when one covers it, else from its JSONs; nothing is installed. Throws
-    // ConfigurationError when the vendor's own JSON cannot be parsed.
-    VendorRead read_vendor(const std::string& dir, const std::string& vendor_name, LoadConfigBundleAttributes flags,
-                           ForwardCompatibilitySubstitutionRule compatibility_rule, bool allow_cache);
-
-    // Parse the vendor's JSONs into `read`, up to the first sub-file that fails.
-    void parse_vendor_json(VendorRead& read);
-
-    // Install what read_vendor read, against base_bundle's filament library. A cache
-    // that cannot be installed is replaced by a parse of the JSONs. Throws
-    // ConfigurationError at the first entry that cannot be installed, or after
-    // installing the entries before a sub-file that could not be parsed.
-    std::pair<PresetsConfigSubstitutions, size_t> install_vendor_read(VendorRead&& read, const PresetBundle* base_bundle);
-
-    // Install a cache's entries. False, with this bundle left clean, when one of them
-    // cannot be installed.
-    bool install_vendor_cache(const std::string& cache_path, const std::string& vendor_name, VendorCacheData&& data,
-                              const PresetBundle* base_bundle);
-
-    // Log and count errors reported by a resolve or a parse.
-    void log_errors(const std::vector<std::string>& errors);
-
-    // The state of installing one collection of one vendor, which
-    // resolve_vendor_preset reads through a const reference and only
-    // commit_vendor_preset writes.
-    struct VendorInstall
-    {
-        // The directory holding <vendor_name>/, whose sub-paths the entries name.
-        std::string                path;
-        std::string                vendor_name;
-        const VendorProfile*       vendor_profile;
-        const PresetBundle*        base_bundle;
-        LoadConfigBundleAttributes flags;
-        PresetCollection*          presets;
-        // The Orca filament library, which keeps every config for other vendors to
-        // resolve against.
-        bool                       is_from_lib;
-        PresetsConfigSubstitutions* substitutions;
-        // The names some entry inherits / includes, the only ones whose configs /
-        // include diffs are looked up again.
-        std::set<std::string>      inherited;
-        std::set<std::string>      included;
-        std::map<std::string, DynamicPrintConfig> config_maps;
-        std::map<std::string, DynamicPrintConfig> include_maps;
-        std::map<std::string, std::string>        filament_id_maps;
-        std::unordered_set<std::string>           installed_names;
-        size_t                     count { 0 };
-    };
-
-    // Install a vendor's source-form entries, parsed from its JSON or read from its
-    // cache: processes, then filaments, then printers. Both loads go through here,
-    // so a cache-loaded bundle cannot come out different from a JSON-loaded one.
-    // `parsed` is given for entries parsed just now. `complete` says the entries
-    // are the vendor's whole lists; only then are the filament library's configs
-    // and filament ids left in m_config_maps and m_filament_id_maps. Returns the
-    // number of presets installed, and throws ConfigurationError at the first
-    // entry that cannot be installed.
-    size_t install_vendor(const std::string& path, const std::string& vendor_name, const PresetBundle* base_bundle,
-                          LoadConfigBundleAttributes flags, const VendorCacheData& entries,
-                          VendorParse* parsed, bool complete, PresetsConfigSubstitutions& substitutions);
-
-    // Install one collection's entries in the order they are listed.
-    void install_vendor_entries(VendorInstall& install, const std::vector<CachedPreset>& entries,
-                                std::vector<EntryParse>* parsed);
-
-    // One entry flattened against the preset it inherits, before anything this
-    // bundle shares has been touched.
-    struct PresetInstall
-    {
-        DynamicPrintConfig       config;
-        std::string              file_path;
-        // Empty when the preset is its own alias.
-        std::string              alias;
-        std::string              filament_id;
-        std::vector<std::string> renamed_from;
-        // Reported by commit, so resolving entries together leaves the log and
-        // the error count as one entry at a time produces them.
-        std::vector<std::string> errors;
-        // What a base states for the presets that include it, when it is retained.
-        std::optional<DynamicPrintConfig> included;
-        // The config kept for the entries that inherit this one, or for other
-        // vendors when this is the filament library.
-        std::optional<DynamicPrintConfig> retained;
-        // Not instantiated, so it contributes a config and no preset.
-        bool                     config_only { false };
-        // Non-empty when the entry is rejected, and says why.
-        std::string              reason;
-    };
-
-    // Flatten one entry against the config it inherits, from this collection's
-    // config_maps or base_bundle's filament library, with the include diffs it
-    // names layered in. It looks up nothing but the names the entry inherits and
-    // includes, and writes nothing.
-    PresetInstall resolve_vendor_preset(const CachedPreset& entry, const VendorInstall& install) const;
-
-    // Install a resolved entry. The collections, the maps in `install` and the
-    // error count are touched here and only here, one entry at a time.
-    // Presets are appended, so a repeated name is caught with `installed_names`,
-    // and the collection is sorted once every entry is in.
-    std::string commit_vendor_preset(const CachedPreset& entry, PresetInstall&& resolved,
-                                     ConfigSubstitutions&& substitutions, VendorInstall& install);
 
     // Clear every collection's m_printer_hold_alias, which reset() leaves alone.
     void clear_printer_hold_aliases();

src/libslic3r/PresetCacheFormat.cpp

@@ -400,24 +400,46 @@ bool write_cache_blob(const std::string& path, const std::string& blob)
 {
     boost::crc_32_type crc;
     crc.process_bytes(blob.data(), blob.size());
-    // Written beside the target and moved into place: a cache is truncated and
+    // Written beside the target and moved into place, as AppConfig::save does:
-    // rewritten in full, so a write that dies partway would otherwise leave a
+    // a cache is truncated and rewritten in full, so a write that dies partway
-    // header claiming more body than the file holds.
+    // would otherwise leave a header claiming more body than the file holds.
+    // The PID suffix also keeps two instances writing the same vendor from
+    // interleaving.
+    const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + ".tmp";
     try {
         boost::filesystem::create_directories(boost::filesystem::path(path).parent_path());
+        {
+            boost::nowide::ofstream ofs(tmp_path, std::ios::binary | std::ios::trunc);
+            if (!ofs.is_open()) {
+                BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: cannot open for writing: " << tmp_path;
+                return false;
+            }
             CacheFileHeader fhdr;
             fhdr.magic     = CACHE_MAGIC;
             fhdr.version   = CACHE_VERSION;
             fhdr.data_size = static_cast<uint64_t>(blob.size());
             fhdr.crc32     = crc.checksum();
-        const std::string_view header(reinterpret_cast<const char*>(&fhdr), sizeof(fhdr));
+            ofs.write(reinterpret_cast<const char*>(&fhdr), sizeof(fhdr));
-        if (const std::error_code ec = write_file_atomically(path, { header, std::string_view(blob) }, /*binary=*/true)) {
+            ofs.write(blob.data(), static_cast<std::streamsize>(blob.size()));
-            BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << path << "): " << ec.message();
+            ofs.close();   // flush; close() raises failbit on error
+            if (! ofs.good()) {
+                BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << tmp_path << ")";
+                boost::system::error_code ec;
+                boost::filesystem::remove(tmp_path, ec);
+                return false;
+            }
+        }
+        if (const std::error_code ec = rename_file(tmp_path, path)) {
+            BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: could not move " << tmp_path << " into place: " << ec.message();
+            boost::system::error_code rm;
+            boost::filesystem::remove(tmp_path, rm);
             return false;
         }
         return true;
     } catch (const std::exception& e) {
         BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << path << "): " << e.what();
+        boost::system::error_code ec;
+        boost::filesystem::remove(tmp_path, ec);
         return false;
     }
 }

src/libslic3r/PresetCacheFormat.hpp

@@ -109,7 +109,7 @@ void skip_config(cereal::BinaryInputArchive& ar, const CacheDictionary& dict);
 
 // One preset as its JSON subfile states it: the config diff, the names of the
 // preset it inherits and the presets it includes, and the parse metadata —
-// everything PresetBundle::parse_vendor_json extracts and
+// everything the parse phase of load_vendor_configs_from_json extracts and
 // nothing it derives. Inheritance and includes are resolved when the entry is
 // installed, against whatever filament library is loaded then, so a cache
 // carries no other vendor's values and no other vendor's update can make it

src/libslic3r/Utils.hpp

@@ -8,8 +8,6 @@
 #include <functional>
 #include <type_traits>
 #include <system_error>
-#include <initializer_list>
-#include <string_view>
 #include <regex>
 
 #include <boost/system/error_code.hpp>
@@ -226,21 +224,6 @@ extern std::vector<std::string> split_string(const std::string &str, char delimi
 // On Windows, the file explorer (or anti-virus or whatever else) often locks the file
 // for a short while, so the file may not be movable. Retry while we see recoverable errors.
 extern std::error_code rename_file(const std::string &from, const std::string &to);
-// Write `chunks`, in order, to `path` through a temporary file beside it that is
-// then renamed over the target, so a concurrent reader sees the old or the new
-// file, never a partial one. The temporary is removed on failure and an existing
-// target keeps its permissions. Text mode unless `binary`, so Windows writes CRLF
-// as the streams this replaces did. A target that is not a regular file (a
-// device or pipe) is written in place, since replacing it would change what it
-// is, and so is an existing target beside which no temporary can be created or
-// whose replace the filesystem refuses; a symlink is followed and the file it
-// names is replaced. On Windows a reader holding the
-// target open without sharing its deletion, which the C runtime does not, makes
-// the replace fall back to the in-place write too, so an unlocked reader there
-// can still see a partial file.
-extern std::error_code write_file_atomically(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary = false);
-inline std::error_code write_file_atomically(const std::string &path, const std::string &content, bool binary = false)
-	{ return write_file_atomically(path, { std::string_view(content) }, binary); }
 
 enum CopyFileResult {
 	SUCCESS = 0,
@@ -273,10 +256,13 @@ extern bool is_gallery_file(const std::string& path, char const* type);
 extern bool is_shapes_dir(const std::string& dir);
 //BBS: add json support
 extern bool is_json_file(const std::string& path);
-// True if rel_path is relative, has no ".." component and, joined to root, still resolves inside it.
+// True if rel_path is relative, has no ".." component or embedded NUL and, joined to root, still resolves inside it.
 // Both '/' and '\\' are treated as separators on every platform, so an archive rejected on one OS
 // is rejected on all of them.
 extern bool is_path_within_root(const std::string &rel_path, const boost::filesystem::path &root);
+// True if a symlink stored at link_rel_path (relative to root) with this target stays inside root: the target
+// must be relative, and joined to the link's directory it must pass is_path_within_root.
+extern bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, const boost::filesystem::path &root);
 
 // Orca: custom protocal support utils
 inline bool is_orca_open(const std::string& url) { return boost::starts_with(url, "orcaslicer://open"); }

src/libslic3r/miniz_extension.cpp

@@ -4,6 +4,9 @@
 #include "miniz_extension.hpp"
 #include "Utils.hpp"
 
+#include <boost/filesystem.hpp>
+#include <boost/log/trivial.hpp>
+
 #if defined(_MSC_VER) || defined(__MINGW64__)
 #include "boost/nowide/cstdio.hpp"
 #endif
@@ -115,6 +118,66 @@ std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_
     return decode_zip_unicode_path_extra_field(extra.substr(0, extra_size > 0 ? extra_size - 1 : 0), stat.m_filename);
 }
 
+bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir)
+{
+    mz_zip_archive archive;
+    mz_zip_zero_struct(&archive);
+
+    if (!open_zip_reader(&archive, zip_path_utf8)) {
+        BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for " << zip_path_utf8;
+        return false;
+    }
+
+    const mz_uint num_entries = mz_zip_reader_get_num_files(&archive);
+    mz_zip_archive_file_stat stat;
+
+    // Validate every entry first so an archive with a single escaping entry leaves no partial output behind.
+    const boost::filesystem::path root(dest_dir);
+    for (mz_uint i = 0; i < num_entries; ++i) {
+        if (mz_zip_reader_file_stat(&archive, i, &stat) && !is_path_within_root(stat.m_filename, root)) {
+            BOOST_LOG_TRIVIAL(error) << "Unzip: rejecting " << zip_path_utf8 << ", entry " << stat.m_filename << " resolves outside " << dest_dir;
+            close_zip_reader(&archive);
+            return false;
+        }
+    }
+
+    for (mz_uint i = 0; i < num_entries; ++i) {
+        if (!mz_zip_reader_file_stat(&archive, i, &stat)) {
+            BOOST_LOG_TRIVIAL(warning) << "Unzip: read file stat failed";
+            continue;
+        }
+        const std::string dest_file = dest_dir + "/" + stat.m_filename;
+        try {
+            if (stat.m_is_directory) {
+                const boost::filesystem::path dest_path(dest_file);
+                if (!boost::filesystem::exists(dest_path))
+                    boost::filesystem::create_directories(dest_path);
+                continue;
+            }
+            if (stat.m_uncomp_size == 0) {
+                BOOST_LOG_TRIVIAL(warning) << "Unzip: invalid size for file " << stat.m_filename;
+                continue;
+            }
+            // Replace a symlink at the destination rather than writing through it.
+            const boost::filesystem::path dest_path(dest_file);
+            if (boost::filesystem::is_symlink(boost::filesystem::symlink_status(dest_path)))
+                boost::filesystem::remove(dest_path);
+            if (!mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0)) {
+                BOOST_LOG_TRIVIAL(error) << "Unzip: extract file " << stat.m_filename << " to dest " << dest_file << " failed";
+                close_zip_reader(&archive);
+                return false;
+            }
+            BOOST_LOG_TRIVIAL(info) << "Unzip: successfully extract file " << stat.m_file_index << " to " << dest_file;
+        } catch (const std::exception &e) {
+            close_zip_reader(&archive);
+            BOOST_LOG_TRIVIAL(error) << "Unzip: archive read exception: " << e.what();
+            return false;
+        }
+    }
+    close_zip_reader(&archive);
+    return true;
+}
+
 MZ_Archive::MZ_Archive()
 {
     mz_zip_zero_struct(&arch);

src/libslic3r/miniz_extension.hpp

@@ -11,6 +11,8 @@ bool open_zip_writer(mz_zip_archive *zip, const std::string &fname_utf8);
 bool close_zip_reader(mz_zip_archive *zip);
 bool close_zip_writer(mz_zip_archive *zip);
 std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_file_stat &stat);
+// Extracts every entry of the archive under dest_dir. Nothing is written if any entry would resolve outside dest_dir.
+bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir);
 
 class MZ_Archive {
 public:

src/libslic3r/utils.cpp

@@ -9,8 +9,6 @@
 #include <stdio.h>
 #include <filesystem>
 #include <sstream>
-#include <cerrno>
-#include <mutex>
 #include <iomanip>
 #include <algorithm>
 #include <cmath>
@@ -704,99 +702,13 @@ namespace WindowsSupport
 std::error_code rename_file(const std::string &from, const std::string &to)
 {
 #ifdef _WIN32
-	// Retries and moves an open destination aside itself.
 	return WindowsSupport::rename(from, to);
 #else
-	// rename(2) replaces an existing target atomically; removing it first would
+	boost::nowide::remove(to.c_str());
-	// leave a window in which the file does not exist at all.
+	return std::make_error_code(static_cast<std::errc>(boost::nowide::rename(from.c_str(), to.c_str())));
-	if (boost::nowide::rename(from.c_str(), to.c_str()) == 0)
-		return {};
-	const int err = errno;
-	// Some mounts (sshfs, gvfs, MTP and a few SMB setups) refuse to replace an
-	// existing target in one step, each with the error it sees fit; every error
-	// is worth the remove-then-rename this always did, except the ones no retry
-	// can help: nothing at the source, a different device, or a directory where
-	// a file was expected and the reverse.
-	const bool worth_retrying = err != ENOENT && err != EXDEV && err != ENOTDIR && err != EISDIR;
-	if (worth_retrying && boost::nowide::remove(to.c_str()) == 0 && boost::nowide::rename(from.c_str(), to.c_str()) == 0)
-		return {};
-	return std::make_error_code(static_cast<std::errc>(err));
 #endif
 }
 
-static std::error_code write_whole_file(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	errno = 0;
-	FILE *file = boost::nowide::fopen(path.c_str(), binary ? "wb" : "w");
-	if (file == nullptr)
-		return std::make_error_code(errno != 0 ? static_cast<std::errc>(errno) : std::errc::io_error);
-	bool ok = true;
-	for (const std::string_view chunk : chunks)
-		ok = ok && std::fwrite(chunk.data(), 1, chunk.size(), file) == chunk.size();
-	ok = ok && std::fflush(file) == 0;
-	const int err = ok ? 0 : errno;
-	ok = std::fclose(file) == 0 && ok;
-	if (ok)
-		return {};
-	return std::make_error_code(err != 0 ? static_cast<std::errc>(err) : std::errc::io_error);
-}
-
-// The in-place fallback truncates the target, so two threads of this process
-// on the same file must not both be in it. One mutex for all such writes: they
-// are the rare case. Never freed, like the InstanceLock registry, so a save
-// during static destruction still finds it.
-static std::error_code write_in_place(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	static auto *mutex = new std::mutex();
-	std::lock_guard<std::mutex> guard(*mutex);
-	return write_whole_file(path, chunks, binary);
-}
-
-std::error_code write_file_atomically(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	boost::system::error_code bec;
-	const boost::filesystem::file_status target = boost::filesystem::symlink_status(path, bec);
-	const bool target_exists = ! bec && boost::filesystem::exists(target);
-	if (target_exists && boost::filesystem::is_symlink(target)) {
-		// A config or preset kept in a dotfiles repository: the link stays,
-		// the file it points to is replaced like any other.
-		const boost::filesystem::path resolved = boost::filesystem::canonical(path, bec);
-		if (! bec && boost::filesystem::is_regular_file(resolved, bec))
-			return write_file_atomically(resolved.string(), chunks, binary);
-	}
-	if (target_exists && ! boost::filesystem::is_regular_file(target))
-		return write_in_place(path, chunks, binary);
-
-	// Unique per process and per call, so two threads writing one target
-	// without a lock never share a temporary.
-	static std::atomic<unsigned> counter{0};
-	const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + "." + std::to_string(counter++) + ".tmp";
-	if (const std::error_code ec = write_whole_file(tmp_path, chunks, binary)) {
-		boost::nowide::remove(tmp_path.c_str());
-		if (! target_exists)
-			return ec;
-		// A directory that lets this process write its files but not create
-		// one: losing the save is worse than a reader seeing a partial file.
-		BOOST_LOG_TRIVIAL(warning) << "Cannot create a temporary beside " << path << " (" << ec.message() << "); writing in place";
-		return write_in_place(path, chunks, binary);
-	}
-#ifndef _WIN32
-	// Not on Windows, where a read-only bit on the temporary would stop the rename itself.
-	if (target_exists)
-		boost::filesystem::permissions(tmp_path, target.permissions(), bec);
-#endif
-	if (const std::error_code ec = rename_file(tmp_path, path)) {
-		boost::nowide::remove(tmp_path.c_str());
-		// A reader on Windows holding the target open without FILE_SHARE_DELETE,
-		// or a mount that cannot replace a file at all. Losing the save is worse
-		// than a reader seeing a partial file, so write in place the way this
-		// used to work before the atomic path existed.
-		BOOST_LOG_TRIVIAL(warning) << "Cannot replace " << path << " (" << ec.message() << "); writing in place";
-		return write_in_place(path, chunks, binary);
-	}
-	return {};
-}
-
 #ifdef __linux__
 // Copied from boost::filesystem.
 // Called by copy_file_linux() in case linux sendfile() API is not supported.
@@ -1181,6 +1093,9 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
     auto is_separator = [](char c) { return c == '/' || c == '\\'; };
     if (rel_path.empty() || is_separator(rel_path.front()) || (rel_path.size() > 1 && rel_path[1] == ':'))
         return false;
+    // The filesystem calls stop at a NUL, so they would act on a shorter path than the one checked here.
+    if (rel_path.find('\0') != std::string::npos)
+        return false;
     for (size_t start = 0; start <= rel_path.size();) {
         size_t end = start;
         while (end < rel_path.size() && !is_separator(rel_path[end]))
@@ -1191,7 +1106,10 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
     }
     // Resolve against the canonical root so a symlink inside it cannot lead back out.
     try {
-        const std::string root_str = boost::filesystem::weakly_canonical(root).string();
+        std::string root_str = boost::filesystem::weakly_canonical(root).string();
+        // A trailing separator on root would otherwise fail the prefix match below for every path.
+        while (!root_str.empty() && (root_str.back() == '/' || root_str.back() == boost::filesystem::path::preferred_separator))
+            root_str.pop_back();
         const std::string full_str = boost::filesystem::weakly_canonical(root / rel_path).string();
         return full_str.compare(0, root_str.size(), root_str) == 0 &&
                (full_str.size() == root_str.size() || full_str[root_str.size()] == boost::filesystem::path::preferred_separator);
@@ -1200,6 +1118,15 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
     }
 }
 
+bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, const boost::filesystem::path &root)
+{
+    if (target.empty() || target.front() == '/' || target.front() == '\\' || (target.size() > 1 && target[1] == ':'))
+        return false;
+    // A relative target without ".." only descends from the link's directory, so no chain of such links can leave root.
+    const size_t sep = link_rel_path.find_last_of("/\\");
+    return is_path_within_root((sep == std::string::npos ? std::string() : link_rel_path.substr(0, sep + 1)) + target, root);
+}
+
 bool is_img_file(const std::string &path)
 {
 	return boost::iends_with(path, ".png") || boost::iends_with(path, ".svg");

src/slic3r/CMakeLists.txt

@@ -1019,14 +1019,9 @@ if (UNIX AND NOT APPLE)
     find_package(GTK${SLIC3R_GTK} REQUIRED)
     pkg_check_modules(LIBSECRET REQUIRED libsecret-1)
     pkg_check_modules(webkit2gtk REQUIRED webkit2gtk-4.1)
-    if (FLATPAK)
-        # I don't know why this is needed, but for whatever reason slic3r isn't
-        # linking to X11 and webkit2gtk. force it.
     find_package(X11 REQUIRED)
-        target_link_libraries(libslic3r_gui ${X11_LIBRARIES} ${webkit2gtk_LIBRARIES})
-    endif()
     target_include_directories(libslic3r_gui SYSTEM PRIVATE ${GTK${SLIC3R_GTK}_INCLUDE_DIRS} ${LIBSECRET_INCLUDE_DIRS} ${webkit2gtk_INCLUDE_DIRS})
-    target_link_libraries(libslic3r_gui ${GTK${SLIC3R_GTK}_LIBRARIES} fontconfig ${LIBSECRET_LIBRARIES})
+    target_link_libraries(libslic3r_gui ${GTK${SLIC3R_GTK}_LIBRARIES} fontconfig ${LIBSECRET_LIBRARIES} ${webkit2gtk_LIBRARIES} ${X11_LIBRARIES})
 
     # Propagate GDK backend detection results as compile definitions so that
     # LinuxDisplayBackend.cpp can include the right GDK headers.

src/slic3r/GUI/GUI.cpp

@@ -14,6 +14,9 @@
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/any.hpp>
 
+#include <wx/filename.h>
+#include <wx/filesys.h>
+
 #if __APPLE__
 #import <IOKit/pwr_mgt/IOPMLib.h>
 #elif _WIN32
@@ -531,6 +534,11 @@ boost::filesystem::path into_path(const wxString &str)
 	return boost::filesystem::path(str.wx_str());
 }
 
+wxString file_url_from_path(const boost::filesystem::path &path)
+{
+	return wxFileSystem::FileNameToURL(wxFileName(from_path(path)));
+}
+
 void about()
 {
     AboutDialog dlg;

src/slic3r/GUI/GUI.hpp

@@ -76,6 +76,9 @@ std::string	into_u8(const wxString &str);
 wxString	from_path(const boost::filesystem::path &path);
 // boost path from wxString
 boost::filesystem::path	into_path(const wxString &str);
+// file:// URL of a local path, percent-encoded so characters such as '#', '%' and '?' stay part of the path.
+// Append any query or fragment to the result.
+wxString	file_url_from_path(const boost::filesystem::path &path);
 
 // Display an About dialog
 extern void about();

src/slic3r/GUI/GUI_App.cpp

@@ -85,7 +85,6 @@
 #include "libslic3r/Model.hpp"
 #include "libslic3r/I18N.hpp"
 #include "libslic3r/PresetBundle.hpp"
-#include "libslic3r/InstanceLock.hpp"
 #include "libslic3r/Thread.hpp"
 #include "libslic3r/miniz_extension.hpp"
 #include "libslic3r/Utils.hpp"
@@ -1513,11 +1512,33 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP
                     size_t n = mz_zip_reader_get_extra(&archive, stat.m_file_index, extra.data(), extra.size());
                     dest_file = decode(extra.substr(0, n), stat.m_filename);
                 }
+                if (!is_path_within_root(dest_file, plugin_folder)) {
+                    BOOST_LOG_TRIVIAL(error) << "[install_plugin] entry " << dest_file << " resolves outside " << plugin_folder.string();
+                    close_zip_reader(&archive);
+                    if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); }
+                    return InstallStatusUnzipFailed;
+                }
                 auto dest_path = plugin_folder / dest_file;
-                boost::filesystem::create_directories(dest_path.parent_path());
                 std::string dest_zip_file = encode_path(dest_path.string().c_str());
+#ifndef WIN32
+                // Validate a symlink's target before anything at the destination is replaced.
+                const bool is_link = S_ISLNK(stat.m_external_attr >> 16);
+                std::string link;
+                if (is_link) {
+                    link.assign(stat.m_uncomp_size, 0);
+                    if (!mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0) ||
+                        !is_symlink_target_within_root(dest_file, link, plugin_folder)) {
+                        BOOST_LOG_TRIVIAL(error) << "[install_plugin] link " << dest_file << " -> " << link << " is unreadable or resolves outside " << plugin_folder.string();
+                        close_zip_reader(&archive);
+                        if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); }
+                        return InstallStatusUnzipFailed;
+                    }
+                }
+#endif
                 try {
-                    if (fs::exists(dest_path)) {
+                    boost::filesystem::create_directories(dest_path.parent_path());
+                    // symlink_status so that an existing symlink, dangling or not, is replaced rather than written through.
+                    if (fs::exists(fs::symlink_status(dest_path))) {
                         boost::system::error_code ec;
                         fs::remove(dest_path, ec);
                         if (ec) {
@@ -1545,9 +1566,8 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP
                     }
                     mz_bool res = 0;
 #ifndef WIN32
-                    if (S_ISLNK(stat.m_external_attr >> 16)) {
+                    if (is_link) {
-                        std::string link(stat.m_uncomp_size + 1, 0);
+                        res = 1;
-                        res = mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0);
                         try {
                             boost::filesystem::create_symlink(link, dest_path);
                         } catch (const std::exception &e) {
@@ -3550,7 +3570,7 @@ bool GUI_App::on_init_inner()
             update_publish_status();
         }
 
-        if (m_post_initialized && app_config->dirty() && app_config->save_due())
+        if (m_post_initialized && app_config->dirty())
             app_config->save();
 
     });
@@ -7644,11 +7664,8 @@ void GUI_App::start_sync_user_preset(bool with_progress_dlg)
 
                             // Delete the bundle folder and bundle
                             fs::path bundle_folder = fs::path(bundle.path.c_str()).parent_path();
-                            {
                             boost::system::error_code ec;
-                                InstanceLock instance_lock(user_presets_lock_path());
                             boost::filesystem::remove_all(bundle_folder, ec);
-                            }
 
                             preset_bundle->bundles.WriteLock();
                             preset_bundle->bundles.m_bundles.erase(bundle.id);
@@ -8935,7 +8952,6 @@ void GUI_App::preset_deleted_from_cloud(std::string setting_id)
 
     // Delete the .info file after cloud deletion is confirmed
     if (!preset_file_path.empty() && fs::exists(fs::path(preset_file_path))) {
-        InstanceLock instance_lock(user_presets_lock_path());
         boost::nowide::remove(preset_file_path.c_str());
         BOOST_LOG_TRIVIAL(info) << "Deleted .info file after cloud confirmation: " << preset_file_path;
     }
@@ -8998,21 +9014,17 @@ void GUI_App::scan_orphaned_info_files()
             fs::path preset_file = info_file;
             preset_file.replace_extension(".json");
 
-            // If .json doesn't exist, .info is orphaned. Read under the lock, so a
+            // If .json doesn't exist, .info is orphaned
-            // remove_files() in another instance is seen whole or not at all; the
+            if (!fs::exists(preset_file)) {
-            // delete queue's own mutex is taken after the lock is released.
+                // Extract setting_id from .info file
-            std::string setting_id;
+                std::string setting_id = extract_setting_id_from_info(info_file.string());
-            {
-                InstanceLock instance_lock(user_presets_lock_path());
-                if (!fs::exists(preset_file))
-                    setting_id = extract_setting_id_from_info(info_file.string());
-            }
                 if (!setting_id.empty()) {
                     // Add to need_delete_presets
                     delete_preset_from_cloud(setting_id, info_file.string());
                     BOOST_LOG_TRIVIAL(info) << "Found orphaned .info file on startup: " << info_file.string();
                 }
             }
+        }
         if (ec)
             BOOST_LOG_TRIVIAL(warning) << "scan_orphaned_info_files: failed to scan " << type_dir.string() << ": " << ec.message();
     }

src/slic3r/GUI/MarkdownTip.cpp

@@ -98,10 +98,7 @@ void MarkdownTip::LoadStyle()
         ph /= "tooltip/styled.html";
         _data_dir = false;
     }
-    auto url = ph.string();
+    _tipView->LoadURL(file_url_from_path(ph));
-    std::replace(url.begin(), url.end(), '\\', '/');
-    url = "file:///" + url;
-    _tipView->LoadURL(from_u8(url));
     _lastTip.clear();
 }
 

src/slic3r/GUI/Plater.cpp

@@ -3672,7 +3672,7 @@ void Sidebar::update_all_preset_comboboxes()
         wxString url = from_u8(PrintHost::get_print_host_webui(&cfg));
         wxString apikey;
         if(url.empty())
-            url = wxString::Format("file://%s/web/orca/missing_connection.html", from_u8(resources_dir()));
+            url = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/orca/missing_connection.html");
         else {
             const auto host_type = cfg.option<ConfigOptionEnum<PrintHostType>>("host_type")->value;
             if (cfg.has("printhost_apikey") && (host_type != htSimplyPrint))

src/slic3r/GUI/PrivacyUpdateDialog.cpp

@@ -57,9 +57,7 @@ PrivacyUpdateDialog::PrivacyUpdateDialog(wxWindow* parent, wxWindowID id, const
 
     fs::path ph(resources_dir());
     ph /= "tooltip/privacyupdate.html";
-    m_host_url = ph.string();
+    m_host_url = into_u8(file_url_from_path(ph));
-    std::replace(m_host_url.begin(), m_host_url.end(), '\\', '/');
-    m_host_url = "file:///" + m_host_url;
     m_vebview_release_note->LoadURL(from_u8(m_host_url));
     m_sizer_right->Add(m_vebview_release_note, 0, wxEXPAND | wxRIGHT | wxLEFT, FromDIP(15));
 

src/slic3r/GUI/Project.cpp

@@ -45,10 +45,10 @@ const std::vector<std::string> license_list = {
 ProjectPanel::ProjectPanel(wxWindow *parent, wxWindowID id, const wxPoint &pos, const wxSize &size, long style) : wxPanel(parent, id, pos, size, style)
 {
     SetBackgroundColour(*wxWHITE);
-    m_project_home_url = wxString::Format("file://%s/web/model/index.html", from_u8(resources_dir()));
+    m_project_home_url = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/model/index.html");
     wxString strlang = wxGetApp().current_language_code_safe();
     if (strlang != "")
-        m_project_home_url = wxString::Format("file://%s/web/model/index.html?lang=%s", from_u8(resources_dir()), strlang);
+        m_project_home_url += "?lang=" + strlang;
 
     wxBoxSizer* main_sizer = new wxBoxSizer(wxVERTICAL);
 

src/slic3r/GUI/ReleaseNote.cpp

@@ -510,7 +510,7 @@ void UpdateVersionDialog::update_version_info(wxString release_note, wxString ve
         out_buf->append(text, size);
     }, (void*) &html_source, MD_DIALECT_GITHUB | MD_FLAG_STRIKETHROUGH | MD_FLAG_WIKILINKS, 0);
     html_source.append("</body></html>");
-    m_vebview_release_note->LoadURL("file://" + (boost::filesystem::path (resources_dir()) / "web/guide/0/index.html").string());
+    m_vebview_release_note->LoadURL(file_url_from_path(boost::filesystem::path(resources_dir()) / "web/guide/0/index.html"));
 
     SetMinSize(GetSize());
     SetMaxSize(GetSize());

src/slic3r/GUI/WebDownPluginDlg.cpp

@@ -38,9 +38,7 @@ DownPluginFrame::DownPluginFrame(GUI_App *pGUI) : wxDialog((wxWindow *) (pGUI->m
 
     // set the frame icon
     wxBoxSizer *topsizer = new wxBoxSizer(wxVERTICAL);
-    wxString TargetUrl    = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/6/index.html").make_preferred().string());
+    wxString TargetUrl    = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/guide/6/index.html");
-
-    TargetUrl = "file://" + TargetUrl;
 
     // Create the webview
     m_browser = WebView::CreateWebView(this, TargetUrl);

src/slic3r/GUI/WebGuideDialog.cpp

@@ -218,37 +218,38 @@ wxString GuideFrame::SetStartPage(GuidePage startpage, bool load)
     m_page = startpage;
     BOOST_LOG_TRIVIAL(info) << __FUNCTION__<< boost::format(" enter, load=%1%, start_page=%2%")%load%int(startpage);
     //wxLogMessage("GUIDE: webpage_1  %s", (boost::filesystem::path(resources_dir()) / "web\\guide\\1\\index.html").make_preferred().string().c_str() );
-    wxString TargetUrl = from_u8( (boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=1").make_preferred().string() );
+    const wxString guide_url = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/guide/0/index.html");
+    wxString TargetUrl = guide_url + "?target=1";
     //wxLogMessage("GUIDE: webpage_2  %s", TargetUrl.mb_str());
 
     if (startpage == BBL_WELCOME){
         SetTitle(_L("Setup Wizard"));
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=1").make_preferred().string());
+        TargetUrl = guide_url + "?target=1";
     } else if (startpage == BBL_REGION) {
         SetTitle(_L("Setup Wizard"));
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=11").make_preferred().string());
+        TargetUrl = guide_url + "?target=11";
     } else if (startpage == BBL_MODELS) {
         SetTitle(_L("Setup Wizard"));
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=21").make_preferred().string());
+        TargetUrl = guide_url + "?target=21";
     } else if (startpage == BBL_FILAMENTS) {
         SetTitle(_L("Setup Wizard"));
 
         int nSize = m_ProfileJson["model"].size();
 
         if (nSize>0)
-            TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=22").make_preferred().string());
+            TargetUrl = guide_url + "?target=22";
         else
-            TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=21").make_preferred().string());
+            TargetUrl = guide_url + "?target=21";
     } else if (startpage == BBL_FILAMENT_ONLY) {
         SetTitle("");
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=23").make_preferred().string());
+        TargetUrl = guide_url + "?target=23";
     } else if (startpage == BBL_MODELS_ONLY) {
         SetTitle("");
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=24").make_preferred().string());
+        TargetUrl = guide_url + "?target=24";
     }
     else {
         SetTitle(_L("Setup Wizard"));
-        TargetUrl = from_u8((boost::filesystem::path(resources_dir()) / "web/guide/0/index.html?target=21").make_preferred().string());
+        TargetUrl = guide_url + "?target=21";
     }
 
     wxString strlang = wxGetApp().current_language_code_safe();
@@ -256,7 +257,6 @@ wxString GuideFrame::SetStartPage(GuidePage startpage, bool load)
     if (strlang != "")
         TargetUrl = wxString::Format("%s&lang=%s", w2s(TargetUrl), strlang);
 
-    TargetUrl = "file://" + TargetUrl;
     if (load)
         load_url(TargetUrl);
 
@@ -1284,7 +1284,7 @@ bool GuideFrame::BuildProfileJson(const PresetBundle& bundle, bool require_all_r
                 entry["vendor"]          = vp.id;
                 entry["nozzle_diameter"] = nozzle_str;
                 entry["materials"]       = materials_str;
-                entry["cover"]           = cover_path.string();
+                entry["cover"]           = into_u8(file_url_from_path(cover_path));
                 entry["nozzle_selected"] = "";
                 entry["sub_path"]        = "";
                 m_ProfileJson["model"].push_back(entry);
@@ -1405,26 +1405,23 @@ bool GuideFrame::BuildProfileDataFromVendors()
         // is served from the shipped profiles. Each is stamped by name and
         // version alone: a profile change requires a version bump, so those two
         // determine content wherever the vendor's copy sits.
-        std::vector<PresetBundle::VendorSource> ordered;
+        struct VendorSource { std::string name; boost::filesystem::path dir; std::string version; };
-        json stamps = json::array();
+        std::vector<VendorSource> ordered;
-        auto add_vendor = [&ordered, &stamps](const std::string& name, const boost::filesystem::path& dir) {
+        auto add_vendor = [&ordered](const std::string& name, const boost::filesystem::path& dir) {
             // The version a load from `dir` would serve: the profile's where one
             // exists (a cache is only served while it covers the profile beside
             // it), the cache's own stamp where the cache is the whole vendor.
             // A profile without a version (blacklist.json) carries no presets
             // and is passed over.
             const boost::filesystem::path profile = dir / (name + ".json");
-            std::string version;
             if (boost::filesystem::exists(profile)) {
                 const Semver v = get_version_from_json(profile.string());
-                if (! v.valid())
+                if (v.valid())
-                    return;
+                    ordered.push_back({name, dir, v.to_string()});
-                version = v.to_string();
             } else {
-                version = VendorCacheFile::peek_version((dir / (name + ".opc")).string(), name);
+                ordered.push_back({name, dir,
+                    VendorCacheFile::peek_version((dir / (name + ".opc")).string(), name)});
             }
-            ordered.push_back({name, dir});
-            stamps.push_back({name, version});
         };
         const std::string filament_library(PresetBundle::ORCA_FILAMENT_LIBRARY);
         if (auto it = vendor_sources.find(filament_library); it != vendor_sources.end())
@@ -1434,6 +1431,9 @@ bool GuideFrame::BuildProfileDataFromVendors()
                 add_vendor(name, dir);
         if (ordered.empty())
             return false;
+        json stamps = json::array();
+        for (const VendorSource& v : ordered)
+            stamps.push_back({v.name, v.version});
 
         // What this function derives is a pure function of that stamped set, so
         // the derived JSON is cached whole: a fresh cache makes an open one
@@ -1461,24 +1461,34 @@ bool GuideFrame::BuildProfileDataFromVendors()
         }
 
         // Each vendor comes from its preset cache where one covers it, which is
-        // what makes this worth doing instead of the scan below.
+        // what makes this worth doing instead of the scan below; loading into a
+        // bundle per vendor keeps the install order the startup path has.
         PresetBundle bundle;
-        std::vector<std::string> failed;
+        auto load_vendor = [](PresetBundle& into, const std::string& vendor,
-        const std::string errors = bundle.load_vendors(ordered, ForwardCompatibilitySubstitutionRule::EnableSilent,
+                              const boost::filesystem::path& dir, const PresetBundle* base) {
-                                                       /*allow_cache=*/true, m_cancel_token.get(), &failed).second;
+            into.load_vendor_configs_from_json(dir.string(), vendor, PresetBundle::LoadSystem,
-        if (*m_cancel_token || bundle.vendors.empty())
+                                               ForwardCompatibilitySubstitutionRule::EnableSilent, base);
-            return false;
+        };
-        if (! errors.empty())
+        for (const VendorSource& v : ordered) {
-            BOOST_LOG_TRIVIAL(warning) << "GuideFrame: loading the vendors reported: " << errors;
+            if (*m_cancel_token)
-        // A vendor that failed to load sends this open to the scan below, which lists
+                return false;   // as in the scan below: a vendor without a cache is parsed, and that takes time
-        // what it can read of every vendor.
+            if (v.name == filament_library) {
-        if (! failed.empty())
+                load_vendor(bundle, v.name, v.dir, nullptr);
+            } else {
+                PresetBundle tmp;
+                load_vendor(tmp, v.name, v.dir, &bundle);
+                bundle.merge_presets(std::move(tmp));
+            }
+        }
+        if (bundle.vendors.empty())
             return false;
         if (! BuildProfileJson(bundle, /*require_all_resource_vendors=*/false))
             return false;
 
         // Written through a temp file and moved into place, as the preset caches
-        // are: half a cache must never be readable.
+        // are: half a cache must never be readable, and the PID suffix keeps two
+        // instances from interleaving on one temp file.
+        const std::string tmp_path = cache_file.string() + "." + std::to_string(get_current_pid()) + ".tmp";
         try {
             json out;
             out["format"]  = 1;
@@ -1487,9 +1497,18 @@ bool GuideFrame::BuildProfileDataFromVendors()
             for (const char* key : { "model", "machine", "filament", "process" })
                 profile[key] = m_ProfileJson[key];
             boost::filesystem::create_directories(cache_file.parent_path());
-            if (const std::error_code ec = write_file_atomically(cache_file.string(), out.dump(-1, ' ', false, json::error_handler_t::ignore), /*binary=*/true))
+            {
+                boost::nowide::ofstream ofs(tmp_path, std::ios::binary | std::ios::trunc);
+                ofs << out.dump(-1, ' ', false, json::error_handler_t::ignore);
+                ofs.close();
+                if (! ofs.good())
+                    throw std::runtime_error("write failed");
+            }
+            if (const std::error_code ec = rename_file(tmp_path, cache_file.string()))
                 throw std::runtime_error(ec.message());
         } catch (const std::exception& e) {
+            boost::system::error_code rm;
+            boost::filesystem::remove(tmp_path, rm);
             BOOST_LOG_TRIVIAL(warning) << "GuideFrame: could not write the profile data cache: " << e.what();
         }
         return true;
@@ -1713,7 +1732,7 @@ int GuideFrame::LoadProfileFamily(std::string strVendor, std::string strFilePath
                     cover_path = (boost::filesystem::absolute(boost::filesystem::path(resources_dir()) / "/web/image/printer/") / cover_file)
                                      .make_preferred();
             }
-            OneModel["cover"]                  = cover_path.string();
+            OneModel["cover"]                  = into_u8(file_url_from_path(cover_path));
 
             OneModel["nozzle_selected"] = "";
 

src/slic3r/GUI/WebUserLoginDialog.cpp

@@ -599,7 +599,7 @@ void ZUserLogin::OnScriptResponseMessage(wxCommandEvent &WXUNUSED(evt))
 
 bool  ZUserLogin::ShowErrorPage()
 {
-    wxString ErrortUrl = from_u8((boost::filesystem::path(resources_dir()) / "web\\login\\error.html").make_preferred().string());
+    wxString ErrortUrl = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/login/error.html");
     load_url(ErrortUrl);
 
     return true;

src/slic3r/GUI/WebViewDialog.cpp

@@ -36,10 +36,10 @@ namespace GUI {
 WebViewPanel::WebViewPanel(wxWindow *parent)
         : wxPanel(parent, wxID_ANY, wxDefaultPosition, wxDefaultSize)
  {
-    wxString url = wxString::Format("file://%s/web/homepage/index.html", from_u8(resources_dir()));
+    wxString url = file_url_from_path(boost::filesystem::path(resources_dir()) / "web/homepage/index.html");
     wxString strlang = wxGetApp().current_language_code_safe();
     if (strlang != "")
-        url = wxString::Format("file://%s/web/homepage/index.html?lang=%s", from_u8(resources_dir()), strlang);
+        url += "?lang=" + strlang;
 
     wxBoxSizer* topsizer = new wxBoxSizer(wxVERTICAL);
     

src/slic3r/GUI/Widgets/MultiNozzleSync.cpp

@@ -1,5 +1,6 @@
 #include "MultiNozzleSync.hpp"
 
+#include "../GUI.hpp"
 #include "../GUI_App.hpp"
 #include "../I18N.hpp"
 #include "../Plater.hpp"
@@ -21,8 +22,6 @@
 #include <set>
 
 #include <wx/choice.h>
-#include <wx/filename.h>
-#include <wx/filesys.h>
 #include <wx/sizer.h>
 #include <wx/stattext.h>
 
@@ -641,8 +640,7 @@ NozzleListTable::NozzleListTable(wxWindow* parent) : wxPanel(parent,wxID_ANY,wxD
     m_web_view->AddScriptMessageHandler("nozzleListTable");
     m_web_view->EnableContextMenu(false);
     fs::path filepath = fs::path(resources_dir()) / "web/flush/NozzleListTable.html";
-    wxFileName fn(wxString::FromUTF8(filepath.string()));
+    wxString url = file_url_from_path(filepath);
-    wxString url = wxFileSystem::FileNameToURL(fn);
     m_web_view->LoadURL(url);
 
     auto sizer = new wxBoxSizer(wxVERTICAL);

src/slic3r/GUI/Widgets/WebHosting.cpp

@@ -48,20 +48,19 @@ constexpr char ORCA_BRIDGE_JS[] = R"JS(
 
 wxString bootstrap_url()
 {
-    return wxString("file://") + from_u8((boost::filesystem::path(resources_dir()) / BOOTSTRAP_PAGE).make_preferred().string());
+    return file_url_from_path(boost::filesystem::path(resources_dir()) / BOOTSTRAP_PAGE);
 }
 
 wxString content_base_url()
 {
-    const std::string dir = (boost::filesystem::path(resources_dir()) / "web").make_preferred().string();
+    return file_url_from_path(boost::filesystem::path(resources_dir()) / "web") + "/";
-    return wxString("file://") + from_u8(dir) + "/";
 }
 
 bool is_content_url(const wxString& url)
 {
-    // The web view reports the URL it parsed, which escapes anything the resources path holds
+    // The web view reports the URL it parsed, which may escape the resources path differently
-    // (a space, a non-ASCII character), while content_base_url() is the raw path.
+    // from content_base_url().
-    return wxURI::Unescape(url.BeforeFirst('#')) == content_base_url();
+    return wxURI::Unescape(url.BeforeFirst('#')) == wxURI::Unescape(content_base_url());
 }
 
 const char* orca_bridge_script() { return ORCA_BRIDGE_JS; }

src/slic3r/GUI/Widgets/WebViewHostDialog.cpp

@@ -192,7 +192,7 @@ bool WebViewHostDialog::create_webview(const std::string& resource_path,
 
 wxString WebViewHostDialog::build_resource_url(const std::string& resource_path) const
 {
-    wxString target_url = from_u8((boost::filesystem::path(resources_dir()) / resource_path).make_preferred().string());
+    wxString target_url = file_url_from_path(boost::filesystem::path(resources_dir()) / resource_path);
 
     if (append_language_to_url()) {
         const wxString lang = wxGetApp().current_language_code_safe();
@@ -202,7 +202,7 @@ wxString WebViewHostDialog::build_resource_url(const std::string& resource_path)
         }
     }
 
-    return wxString("file://") + target_url;
+    return target_url;
 }
 
 void WebViewHostDialog::load_url(const wxString& url)

src/slic3r/GUI/WipeTowerDialog.cpp

@@ -464,7 +464,7 @@ WipingDialog::WipingDialog(wxWindow* parent, const int max_flush_volume) :
     wxString filepath_str = from_path(filepath);
     wxFileName fn(filepath_str);
     if(fn.FileExists()) {
-        wxString url = wxFileSystem::FileNameToURL(fn);
+        wxString url = file_url_from_path(filepath);
         BOOST_LOG_TRIVIAL(debug) << __FUNCTION__<< "File exists and load url " << url.ToStdString();
         m_webview->LoadURL(url);
         BOOST_LOG_TRIVIAL(debug) << __FUNCTION__<< "Successfully loaded url: " << url.ToStdString();

src/slic3r/Utils/3DPrinterOS.cpp

@@ -2,7 +2,6 @@
 
 #include <algorithm>
 #include <sstream>
-#include <system_error>
 #include <exception>
 #include <boost/format.hpp>
 #include <boost/log/trivial.hpp>
@@ -581,10 +580,9 @@ bool C3DPrinterOS::save_api_session(const std::string &session, const std::strin
     j.put("session", session);
     j.put("email", email);
     try {
-        std::ostringstream json;
+        auto temp_path = m_api_session_file_path + ".tmp";
-        pt::write_json(json, j);
+        pt::write_json(temp_path, j);
-        if (const std::error_code ec = write_file_atomically(m_api_session_file_path, json.str()))
+        boost::filesystem::rename(temp_path, m_api_session_file_path);
-            throw std::system_error(ec);
     } catch (const std::exception &err) {
         BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to write json to file. Path = "
                                  << m_api_session_file_path

src/slic3r/Utils/ElegooLink.cpp

@@ -340,9 +340,7 @@ namespace Slic3r {
         if (classify_printer_model(config->opt_string("printer_model")) != ElegooPrinterType::CC2)
             return fallback_webui;
 
-        std::string web_path = resources_dir() + "/web/elegoolink/lan_service_web/index.html";
+        std::string web_path = GUI::into_u8(GUI::file_url_from_path(boost::filesystem::path(resources_dir()) / "web/elegoolink/lan_service_web/index.html"));
-        std::replace(web_path.begin(), web_path.end(), '\\', '/');
-        web_path = "file://" + web_path;
         const std::string token   = get_cc2_token(config->opt_string("printhost_apikey"));
         const std::string host_ip = Http::get_host_header_value(host);
 

src/slic3r/Utils/OrcaCloudServiceAgent.cpp

@@ -1475,8 +1475,15 @@ void OrcaCloudServiceAgent::save_sync_state()
     if (sync_state_path.empty())
         return;
 
-    if (const std::error_code ec = write_file_atomically(sync_state_path, std::to_string(sync_state.last_sync_timestamp)))
+    try {
-        BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: failed to save the sync state: " << ec.message();
+        std::string tmp_path = sync_state_path + ".tmp";
+        std::ofstream ofs(tmp_path, std::ios::out | std::ios::trunc);
+        if (ofs.good()) {
+            ofs << std::to_string(sync_state.last_sync_timestamp);
+            ofs.close();
+            boost::filesystem::rename(tmp_path, sync_state_path);
+        }
+    } catch (...) {}
 }
 
 void OrcaCloudServiceAgent::clear_sync_state()
@@ -1565,10 +1572,22 @@ void OrcaCloudServiceAgent::persist_user_secret(const std::string& secret)
             wxFileName::Mkdir(path.GetPath(), wxS_DIR_DEFAULT, wxPATH_MKDIR_FULL);
         }
 
-        if (const std::error_code ec = write_file_atomically(secret_fallback_path, signed_payload, /*binary=*/true))
+        const std::string tmp_path = secret_fallback_path + ".tmp";
-            BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: cannot write user secret file " << secret_fallback_path << ": " << ec.message();
+        std::ofstream ofs(tmp_path, std::ios::out | std::ios::trunc | std::ios::binary);
-        else
+        if (ofs.good()) {
+            ofs << signed_payload;
+            ofs.flush();
+            ofs.close();
+
+            if (wxRenameFile(wxString::FromUTF8(tmp_path.c_str()), wxString::FromUTF8(secret_fallback_path.c_str()), true)) {
                 stored = true;
+            } else {
+                wxRemoveFile(wxString::FromUTF8(tmp_path.c_str()));
+                BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: failed to atomically replace user secret file";
+            }
+        } else {
+            BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: cannot open user secret file for write - " << secret_fallback_path;
+        }
     } else {
         // Use wxSecretStore only
         wxSecretStore store = wxSecretStore::GetDefault();

src/slic3r/Utils/PresetUpdater.cpp

@@ -339,62 +339,8 @@ bool PresetUpdater::priv::get_file(const std::string &url, const fs::path &targe
 //BBS: refine preset update logic
 bool PresetUpdater::priv::extract_file(const fs::path &source_path, const fs::path &dest_path)
 {
-    bool res = true;
+    const std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string();
-    std::string file_path = source_path.string();
+    return extract_archive_confined(source_path.string(), parent_path);
-    std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string();
-    mz_zip_archive archive;
-    mz_zip_zero_struct(&archive);
-
-    if (!open_zip_reader(&archive, file_path))
-    {
-        BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for "<<file_path;
-        return false;
-    }
-
-    mz_uint num_entries = mz_zip_reader_get_num_files(&archive);
-
-    mz_zip_archive_file_stat stat;
-    // we first loop the entries to read from the archive the .amf file only, in order to extract the version from it
-    for (mz_uint i = 0; i < num_entries; ++i)
-    {
-        if (mz_zip_reader_file_stat(&archive, i, &stat))
-        {
-            std::string dest_file = parent_path+"/"+stat.m_filename;
-            if (stat.m_is_directory) {
-                fs::path dest_path(dest_file);
-                if (!fs::exists(dest_path))
-                    fs::create_directories(dest_path);
-				continue;
-            }
-            else if (stat.m_uncomp_size == 0) {
-                BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: invalid size for file "<<stat.m_filename;
-                continue;
-            }
-            try
-            {
-                res = mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0);
-                if (!res) {
-                    BOOST_LOG_TRIVIAL(error) << "[Orca Updater]extract file "<<stat.m_filename<<" to dest "<<dest_file<<" failed";
-                    close_zip_reader(&archive);
-                    return res;
-                }
-                BOOST_LOG_TRIVIAL(info) << "[Orca Updater]successfully extract file " << stat.m_file_index << " to "<<dest_file;
-            }
-            catch (const std::exception& e)
-            {
-                // ensure the zip archive is closed and rethrow the exception
-                close_zip_reader(&archive);
-                BOOST_LOG_TRIVIAL(error) << "[Orca Updater]Archive read exception:"<<e.what();
-                return false;
-            }
-        }
-        else {
-            BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: read file stat failed";
-        }
-    }
-    close_zip_reader(&archive);
-
-	return true;
 }
 
 // Remove a leftover partial archive for the vendor about to be synchronized.

src/slic3r/plugin/PluginConfig.cpp

@@ -249,10 +249,21 @@ bool PluginConfig::save()
         return false;
     }
 
-    // Written beside the target and moved into place, so a crash mid-write cannot truncate an
+    // Write to a PID-suffixed file and rename it into place, so a crash mid-write cannot truncate an
-    // existing config.
+    // existing config. Same approach as AppConfig::save().
-    if (const std::error_code ec = write_file_atomically(path, root.dump(1, '\t') + "\n")) {
+    const std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
-        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to write " << path << ": " << ec.message() << "; keeping the existing config";
+
+    boost::nowide::ofstream file;
+    file.open(path_pid, std::ios::out | std::ios::trunc);
+    file << root.dump(1, '\t') << std::endl;
+    file.close();
+    if (file.fail()) {
+        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to write " << path_pid << "; keeping the existing config";
+        return false;
+    }
+
+    if (const std::error_code rename_ec = rename_file(path_pid, path)) {
+        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to move " << path_pid << " onto " << path << ": " << rename_ec.message();
         return false;
     }
 

tests/libslic3r/CMakeLists.txt

@@ -16,10 +16,8 @@ add_executable(${_TEST_NAME}_tests
     test_clipper_utils.cpp
     test_config.cpp
     test_config_variant_expansion.cpp
-    test_locales_utils.cpp
     test_drc.cpp
     test_toolordering_nozzle_group.cpp
-    test_parallel_resolve.cpp
     test_preset_bundle_loading.cpp
     test_preset_setting_id.cpp
     test_preset_diff.cpp
@@ -46,6 +44,7 @@ add_executable(${_TEST_NAME}_tests
     test_lay_on_face.cpp
     test_model.cpp
     test_utils.cpp
+    test_miniz_extension.cpp
     test_timeutils.cpp
     test_voronoi.cpp
     test_wipe_tower_estimate.cpp
@@ -55,7 +54,6 @@ add_executable(${_TEST_NAME}_tests
     test_ordering_strategies.cpp
     # test_png_io.cpp
     test_indexed_triangle_set.cpp
-    test_instance_lock.cpp
     ../libnest2d/printer_parts.cpp
     )
 

tests/libslic3r/test_instance_lock.cpp

@@ -1,228 +0,0 @@
-#include <catch2/catch_all.hpp>
-
-#include <atomic>
-#include <chrono>
-#include <thread>
-
-#include <boost/filesystem.hpp>
-
-#include "libslic3r/InstanceLock.hpp"
-#include "test_utils.hpp"
-
-#ifdef _WIN32
-#include <boost/interprocess/sync/file_lock.hpp>
-#include <boost/nowide/convert.hpp>
-#include <boost/nowide/fstream.hpp>
-#else
-#include <fcntl.h>
-#include <sys/file.h>
-#include <unistd.h>
-#endif
-
-using namespace Slic3r;
-using namespace std::chrono_literals;
-
-// Sets a process-wide knob for one test and restores it however the test ends.
-template<typename T> struct ScopedStaticValue
-{
-    T &ref;
-    T  saved;
-    ScopedStaticValue(T &ref, T value) : ref(ref), saved(ref) { ref = value; }
-    ~ScopedStaticValue() { ref = saved; }
-};
-
-TEST_CASE("InstanceLock creates its lock file and holds it for the guard's scope", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    {
-        InstanceLock lock(path);
-        REQUIRE(lock.locked());
-        REQUIRE(boost::filesystem::exists(path));
-    }
-    // Released: a fresh guard gets the lock at once instead of waiting out a timeout.
-    const auto   started = std::chrono::steady_clock::now();
-    InstanceLock again(path, 5000ms);
-    REQUIRE(again.locked());
-    // Well inside the timeout it would otherwise have waited out; loose enough for a loaded runner.
-    REQUIRE(std::chrono::steady_clock::now() - started < 4000ms);
-}
-
-TEST_CASE("InstanceLock nests within one thread", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    InstanceLock outer(path);
-    {
-        InstanceLock inner(path, 100ms);
-        REQUIRE(inner.locked());
-    }
-    // The inner guard leaving does not release the outer one.
-    REQUIRE(outer.locked());
-}
-
-TEST_CASE("InstanceLock is a no-op for an empty path and survives an unwritable one", "[InstanceLock]")
-{
-    ScopedTemporaryDir dir;
-
-    InstanceLock none("");
-    REQUIRE_FALSE(none.locked());
-
-    // The directory does not exist, so the lock file cannot be created; the
-    // guard still constructs and the write it guards can go ahead.
-    InstanceLock unwritable((dir.path() / "missing" / "shared.lock").string(), 100ms);
-    REQUIRE_FALSE(unwritable.locked());
-}
-
-TEST_CASE("InstanceLock retries a lock file it could not open once the cool-down passes", "[InstanceLock]")
-{
-    ScopedTemporaryDir dir;
-    const std::string  path = (dir.path() / "later" / "shared.lock").string();
-    ScopedStaticValue  cooldown(InstanceLock::cooldown, 300ms);
-
-    bool before_dir, during_cooldown, after_cooldown;
-    const auto started = std::chrono::steady_clock::now();
-    {
-        InstanceLock lock(path, 100ms);
-        before_dir = lock.locked();
-    }
-    boost::filesystem::create_directories(dir.path() / "later");
-    {
-        InstanceLock lock(path, 100ms);
-        during_cooldown = lock.locked();
-    }
-    const bool second_guard_inside_cooldown = std::chrono::steady_clock::now() - started < InstanceLock::cooldown;
-    std::this_thread::sleep_for(400ms);
-    {
-        InstanceLock lock(path, 100ms);
-        after_cooldown = lock.locked();
-    }
-
-    REQUIRE_FALSE(before_dir);
-    // A loaded runner may take longer than the cool-down to get here; then the
-    // second guard legitimately retried, so only assert when the timing held.
-    if (second_guard_inside_cooldown)
-        REQUIRE_FALSE(during_cooldown);
-    REQUIRE(after_cooldown);
-}
-
-TEST_CASE("InstanceLock reopens a lock file that was replaced on disk", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-    {
-        InstanceLock lock(path);
-        REQUIRE(lock.locked());
-    }
-
-    boost::filesystem::remove(path);
-    InstanceLock lock(path);
-    REQUIRE(lock.locked());
-    // Each outermost guard opens the file afresh, so the deleted path is back.
-    REQUIRE(boost::filesystem::exists(path));
-}
-
-TEST_CASE("InstanceLock serialises the threads of one process", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    std::atomic<bool> holder_ready{false};
-    std::atomic<bool> holder_released{false};
-    std::thread holder([&] {
-        InstanceLock lock(path);
-        holder_ready = true;
-        std::this_thread::sleep_for(150ms);
-        holder_released = true;
-    });
-    while (! holder_ready)
-        std::this_thread::yield();
-
-    bool released_before_acquire = false;
-    {
-        InstanceLock lock(path);
-        released_before_acquire = holder_released;
-    }
-    holder.join();
-    REQUIRE(released_before_acquire);
-}
-
-// Holds the OS lock on a file through a handle of its own, as another instance would. The lock
-// belongs to the handle on Windows and to the open file description elsewhere, so the guard's
-// handle is refused while this one holds it.
-class OtherHolder
-{
-public:
-    explicit OtherHolder(const std::string &path)
-    {
-#ifdef _WIN32
-        boost::nowide::ofstream(path, std::ios::app).close();
-        m_lock = boost::interprocess::file_lock(boost::nowide::widen(path).c_str());
-        m_held = m_lock.try_lock();
-#else
-        m_fd   = ::open(path.c_str(), O_RDWR | O_CREAT, 0644);
-        m_held = m_fd >= 0 && ::flock(m_fd, LOCK_EX | LOCK_NB) == 0;
-#endif
-    }
-    ~OtherHolder() { release(); }
-    OtherHolder(const OtherHolder &) = delete;
-    OtherHolder &operator=(const OtherHolder &) = delete;
-
-    bool held() const { return m_held; }
-    void release()
-    {
-#ifdef _WIN32
-        if (m_held)
-            m_lock.unlock();
-        m_lock = boost::interprocess::file_lock();
-#else
-        if (m_fd >= 0)
-            ::close(m_fd);
-        m_fd = -1;
-#endif
-        m_held = false;
-    }
-
-private:
-#ifdef _WIN32
-    boost::interprocess::file_lock m_lock;
-#else
-    int m_fd{-1};
-#endif
-    bool m_held{false};
-};
-
-TEST_CASE("InstanceLock yields to a lock held through another handle and reports it", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-    ScopedStaticValue   cooldown(InstanceLock::cooldown, 300ms);
-
-    OtherHolder other(path);
-    REQUIRE(other.held());
-
-    bool locked_while_other_holds;
-    {
-        InstanceLock lock(path, 100ms);
-        locked_while_other_holds = lock.locked();
-    }
-    // The timed-out wait starts a cool-down: the next guard does not touch the file.
-    const auto started = std::chrono::steady_clock::now();
-    bool       locked_during_cooldown;
-    {
-        InstanceLock lock(path, 5000ms);
-        locked_during_cooldown = lock.locked();
-    }
-    const auto cooldown_wait = std::chrono::steady_clock::now() - started;
-    other.release();
-
-    REQUIRE_FALSE(locked_while_other_holds);
-    REQUIRE_FALSE(locked_during_cooldown);
-    REQUIRE(cooldown_wait < 4000ms);
-    // Once the cool-down passes, the lock the other holder released is taken again.
-    std::this_thread::sleep_for(400ms);
-    InstanceLock lock(path);
-    REQUIRE(lock.locked());
-}

tests/libslic3r/test_locales_utils.cpp

@@ -1,69 +0,0 @@
-#include <catch2/catch_all.hpp>
-
-#include <clocale>
-
-#include "libslic3r/LocalesUtils.hpp"
-
-using namespace Slic3r;
-
-namespace {
-
-// Switches this thread's numeric locale to one whose decimal separator is a comma,
-// when the system has one installed.
-struct CommaNumericLocale
-{
-#ifdef _WIN32
-    bool apply()
-    {
-        for (const char* name : { "de-DE", "German_Germany.1252" })
-            if (std::setlocale(LC_NUMERIC, name) != nullptr)
-                return true;
-        return false;
-    }
-#else
-    locale_t locale { (locale_t) 0 };
-    bool apply()
-    {
-        for (const char* name : { "de_DE.UTF-8", "de_DE.utf8", "de_DE" })
-            if ((locale = newlocale(LC_NUMERIC_MASK, name, (locale_t) 0)) != (locale_t) 0) {
-                uselocale(locale);
-                return true;
-            }
-        return false;
-    }
-    // Freed once the setters around apply() have put the thread's own locale back.
-    ~CommaNumericLocale()
-    {
-        if (locale != (locale_t) 0)
-            freelocale(locale);
-    }
-#endif
-};
-
-} // namespace
-
-TEST_CASE("a setter nested in another leaves the C locale in place for the outer one", "[LocalesUtils]")
-{
-    CNumericLocalesSetter outer;
-    {
-        CNumericLocalesSetter inner;
-        CHECK(is_decimal_separator_point());
-    }
-    CHECK(is_decimal_separator_point());
-}
-
-TEST_CASE("a setter nested in another sets C again when the locale changed between them", "[LocalesUtils]")
-{
-    CommaNumericLocale comma;
-    {
-        CNumericLocalesSetter outer;
-        if (! comma.apply())
-            SKIP("no locale with a comma decimal separator is installed");
-        REQUIRE_FALSE(is_decimal_separator_point());
-        {
-            CNumericLocalesSetter inner;
-            CHECK(is_decimal_separator_point());
-        }
-        CHECK_FALSE(is_decimal_separator_point());
-    }
-}

tests/libslic3r/test_miniz_extension.cpp

@@ -0,0 +1,194 @@
+#include <catch2/catch_all.hpp>
+
+#include "libslic3r/miniz_extension.hpp"
+
+#include "test_utils.hpp"
+
+#include <boost/filesystem.hpp>
+
+#include <algorithm>
+#include <fstream>
+#include <iterator>
+#include <string>
+#include <utility>
+#include <vector>
+
+using namespace Slic3r;
+namespace fs = boost::filesystem;
+
+namespace {
+
+void write_zip(const fs::path &zip_file, const std::vector<std::pair<std::string, std::string>> &entries)
+{
+    mz_zip_archive zip;
+    mz_zip_zero_struct(&zip);
+    REQUIRE(open_zip_writer(&zip, zip_file.string()));
+    for (const auto &[name, content] : entries)
+        REQUIRE(mz_zip_writer_add_mem(&zip, name.c_str(), content.data(), content.size(), MZ_DEFAULT_COMPRESSION));
+    REQUIRE(mz_zip_writer_finalize_archive(&zip));
+    REQUIRE(close_zip_writer(&zip));
+}
+
+// miniz refuses to write a name starting with '/', so write a placeholder of the same length and patch it in place.
+void rename_entry(const fs::path &zip_file, const std::string &from, const std::string &to)
+{
+    REQUIRE(from.size() == to.size());
+    std::string bytes;
+    {
+        std::ifstream in(zip_file.string(), std::ios::binary);
+        bytes.assign(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
+    }
+    size_t count = 0;
+    for (size_t pos = bytes.find(from); pos != std::string::npos; pos = bytes.find(from, pos + to.size()), ++count)
+        bytes.replace(pos, from.size(), to);
+    // Once in the local header and once in the central directory.
+    REQUIRE(count == 2);
+    std::ofstream out(zip_file.string(), std::ios::binary | std::ios::trunc);
+    out << bytes;
+}
+
+std::vector<std::string> list_dir(const fs::path &dir)
+{
+    std::vector<std::string> names;
+    for (const fs::directory_entry &entry : fs::directory_iterator(dir))
+        names.push_back(entry.path().filename().string());
+    std::sort(names.begin(), names.end());
+    return names;
+}
+
+std::string read_file(const fs::path &file)
+{
+    std::ifstream in(file.string(), std::ios::binary);
+    return std::string(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
+}
+
+} // namespace
+
+TEST_CASE("Confined extraction writes a well-formed archive under the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+    write_zip(zip_file, {{"vendor/", ""}, {"vendor/machine/", ""}, {"vendor.json", "{\"a\":1}"}, {"vendor/machine/printer.json", "{\"b\":2}"}});
+
+    REQUIRE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK(fs::is_directory(target / "vendor"));
+    CHECK(read_file(target / "vendor.json") == "{\"a\":1}");
+    CHECK(read_file(target / "vendor" / "machine" / "printer.json") == "{\"b\":2}");
+}
+
+TEST_CASE("Confined extraction rejects an archive with an entry outside the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    const std::string escaping_entry = GENERATE(std::string("../escape.txt"), std::string("..\\escape.txt"),
+                                                std::string("sub/../../escape.txt"), std::string("C:/escape.txt"),
+                                                std::string("C:escape.txt"), std::string("\\escape.txt"));
+    // The normal entry comes first so a per-entry check would already have written it.
+    write_zip(zip_file, {{"normal.json", "{}"}, {escaping_entry, "escaped"}});
+
+    CAPTURE(escaping_entry);
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction rejects an archive with an absolute entry name", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    const std::string absolute    = (tmp.path() / "escape.txt").generic_string();
+    const std::string placeholder = "#" + absolute.substr(1);
+    write_zip(zip_file, {{"normal.json", "{}"}, {placeholder, "escaped"}});
+    rename_entry(zip_file, placeholder, absolute);
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction rejects a directory entry outside the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+    write_zip(zip_file, {{"vendor/", ""}, {"../outside/", ""}});
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "outside"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction validates zero-size entries like any other", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    SECTION("an empty file inside the target does not fail the archive") {
+        write_zip(zip_file, {{"empty.json", ""}, {"vendor.json", "{}"}});
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK(read_file(target / "vendor.json") == "{}");
+    }
+    SECTION("an empty file outside the target rejects the archive") {
+        write_zip(zip_file, {{"vendor.json", "{}"}, {"../escape.txt", ""}});
+        CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+        CHECK(fs::is_empty(target));
+    }
+}
+
+TEST_CASE("Confined extraction writes nothing outside the target for Windows-specific name forms", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    // Windows strips trailing dots and spaces and maps device names; whether these extract depends on the
+    // platform, but none of them may land beside the target.
+    const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("..."), std::string(".. "),
+                                      std::string(".. /escape.txt"), std::string(".../escape.txt"), std::string("CON"),
+                                      std::string("sub/NUL.txt"), std::string("C:escape.txt"));
+    write_zip(zip_file, {{name, "payload"}});
+
+    CAPTURE(name);
+    extract_archive_confined(zip_file.string(), target.string());
+    CHECK(list_dir(tmp.path()) == std::vector<std::string>{"bundle.zip", "cache"});
+}
+
+#ifndef _WIN32
+TEST_CASE("Confined extraction replaces a symlink at the destination instead of writing through it", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    const fs::path     outside  = tmp.path() / "outside";
+    fs::create_directories(target);
+    fs::create_directories(outside);
+    write_zip(zip_file, {{"vendor.json", "{\"a\":1}"}});
+
+    SECTION("a dangling symlink") {
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(outside / "vendor.json"));
+        CHECK_FALSE(fs::is_symlink(fs::symlink_status(target / "vendor.json")));
+        CHECK(read_file(target / "vendor.json") == "{\"a\":1}");
+    }
+    SECTION("a symlink to an existing file") {
+        { std::ofstream((outside / "vendor.json").string()) << "original"; }
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        extract_archive_confined(zip_file.string(), target.string());
+        CHECK(read_file(outside / "vendor.json") == "original");
+    }
+}
+#endif

tests/libslic3r/test_parallel_resolve.cpp

@@ -1,122 +0,0 @@
-#include <catch2/catch_all.hpp>
-
-#include <numeric>
-#include <stdexcept>
-#include <thread>
-#include <vector>
-
-#include <tbb/blocked_range.h>
-#include <tbb/parallel_for.h>
-#include <tbb/task_group.h>
-
-#include "libslic3r/ParallelResolve.hpp"
-
-using namespace Slic3r;
-
-namespace {
-
-thread_local int t_live_setups = 0;
-
-// Counts how many are alive on the constructing thread.
-struct CountingSetup
-{
-    CountingSetup() { ++ t_live_setups; }
-    ~CountingSetup() { -- t_live_setups; }
-};
-
-std::vector<size_t> first_indices(size_t count)
-{
-    std::vector<size_t> indices(count);
-    std::iota(indices.begin(), indices.end(), size_t(0));
-    return indices;
-}
-
-} // namespace
-
-TEST_CASE("every item resolves once and commits in index order on the calling thread", "[ParallelResolve]")
-{
-    const size_t        count = 200;
-    std::vector<int>    resolves(count, 0);
-    std::vector<size_t> committed, values;
-    std::vector<bool>   on_caller;
-    const std::thread::id caller = std::this_thread::get_id();
-    resolve_then_commit(count,
-        [&](size_t i) { ++ resolves[i]; return i * 3; },
-        [&](size_t i, size_t resolved) {
-            committed.push_back(i);
-            values.push_back(resolved);
-            on_caller.push_back(std::this_thread::get_id() == caller);
-        });
-
-    CHECK(committed == first_indices(count));
-    for (size_t i = 0; i < count; ++ i) {
-        CHECK(resolves[i] == 1);
-        CHECK(values[i] == i * 3);
-        CHECK(on_caller[i]);
-    }
-}
-
-TEST_CASE("every item resolves inside one chunk setup", "[ParallelResolve]")
-{
-    const size_t     count = 200;
-    std::vector<int> live(count, 0);
-    resolve_then_commit<CountingSetup>(count,
-        [&](size_t i) { live[i] = t_live_setups; return 0; },
-        [](size_t, int) {});
-
-    for (size_t i = 0; i < count; ++ i)
-        CHECK(live[i] == 1);
-}
-
-TEST_CASE("an exception from resolve leaves the batches before it committed", "[ParallelResolve]")
-{
-    const size_t        count = 200, fails_at = 150;
-    REQUIRE(fails_at >= resolve_batch_size);
-    std::vector<size_t> committed;
-    CHECK_THROWS_AS(resolve_then_commit(count,
-                        [&](size_t i) {
-                            if (i == fails_at)
-                                throw std::runtime_error("resolve failed");
-                            return i;
-                        },
-                        [&](size_t i, size_t) { committed.push_back(i); }),
-                    std::runtime_error);
-
-    CHECK(committed == first_indices(fails_at / resolve_batch_size * resolve_batch_size));
-}
-
-TEST_CASE("an exception from commit stops at the item that threw", "[ParallelResolve]")
-{
-    const size_t        count = 200, fails_at = 90;
-    std::vector<size_t> committed;
-    CHECK_THROWS_AS(resolve_then_commit(count,
-                        [](size_t i) { return i; },
-                        [&](size_t i, size_t) {
-                            if (i == fails_at)
-                                throw std::runtime_error("commit failed");
-                            committed.push_back(i);
-                        }),
-                    std::runtime_error);
-
-    CHECK(committed == first_indices(fails_at));
-}
-
-TEST_CASE("a canceled task group stops before committing an item it did not resolve", "[ParallelResolve]")
-{
-    std::vector<size_t>     committed;
-    bool                    threw = false;
-    tbb::task_group_context context;
-    tbb::parallel_for(tbb::blocked_range<size_t>(0, 1), [&](const tbb::blocked_range<size_t>&) {
-        context.cancel_group_execution();
-        try {
-            resolve_then_commit(200,
-                [](size_t i) { return i + 1; },
-                [&](size_t, size_t resolved) { committed.push_back(resolved); });
-        } catch (const std::runtime_error&) {
-            threw = true;
-        }
-    }, context);
-
-    CHECK(threw);
-    CHECK(committed.empty());
-}

tests/libslic3r/test_preset_bundle_loading.cpp

@@ -5,7 +5,6 @@
 #include <fstream>
 
 #include "libslic3r/PresetBundle.hpp"
-#include "libslic3r/ParallelResolve.hpp"
 #include "libslic3r/AppConfig.hpp"
 #include "libslic3r/Model.hpp"
 #include "libslic3r/TriangleMesh.hpp"
@@ -18,10 +17,6 @@
 #include <iostream>
 #include <initializer_list>
 
-#ifndef _WIN32
-#include <unistd.h> // geteuid
-#endif
-
 using namespace Slic3r;
 
 namespace {
@@ -64,16 +59,6 @@ void write_preset_with_inherits(const DynamicPrintConfig &default_config, const
     config.save_to_json(file.string(), name, "User", "1.0.0");
 }
 
-// A user preset file stating nothing but the preset it inherits, so every value it
-// ends up with came from resolving that parent.
-void write_minimal_child(const fs::path &file, const std::string &name, const std::string &inherits)
-{
-    fs::create_directories(file.parent_path());
-    std::ofstream(file.string())
-        << R"({"type":"process","name":")" << name << R"(","from":"User","version":"1.0.0","inherits":")"
-        << inherits << R"("})";
-}
-
 // Add an in-memory preset (no file) with the given inherits value (empty => root preset).
 Preset &add_inmemory_preset(PresetCollection &coll, const std::string &name, const std::string &inherits = {})
 {
@@ -276,162 +261,6 @@ TEST_CASE("Selected printer uses its default or saved bed type", "[Preset][Bundl
     CHECK(app_config.get_printer_setting("Test Printer", "curr_bed_type") == std::to_string(static_cast<int>(expected_bed_type)));
 }
 
-TEST_CASE("A directory of user presets loads with each one resolved against its parent", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir   temp_dir;
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.config.option<ConfigOptionFloat>("layer_height", true)->value = 0.24;
-    parent.is_system = true;
-
-    constexpr int children = 400;
-    for (int i = 0; i < children; ++ i)
-        write_minimal_child(temp_dir.path() / PRESET_PRINT_NAME / ("Child " + std::to_string(i) + ".json"),
-                            "Child " + std::to_string(i), "Parent Process");
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable);
-
-    CHECK(coll.size() == size_t(children) + 2); // the children, the default preset and the parent
-    CHECK(coll.error_count() == 0);
-    for (int i = 0; i < children; ++ i) {
-        const Preset *child = coll.find_preset("Child " + std::to_string(i));
-        REQUIRE(child != nullptr);
-        CHECK(child->inherits() == "Parent Process");
-        CHECK(child->alias == "Child " + std::to_string(i));
-        CHECK(child->loaded);
-        REQUIRE(child->config.option<ConfigOptionFloat>("layer_height") != nullptr);
-        CHECK_THAT(child->config.opt_float("layer_height"), Catch::Matchers::WithinAbs(0.24, 1e-9));
-    }
-}
-
-TEST_CASE("Repeated loads of a user preset directory produce the same presets", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir temp_dir;
-
-    auto seed_directory = [&]() {
-        for (int i = 0; i < 200; ++ i)
-            write_minimal_child(temp_dir.path() / PRESET_PRINT_NAME / ("Child " + std::to_string(i) + ".json"),
-                                "Child " + std::to_string(i), "Parent Process");
-    };
-
-    std::vector<std::vector<std::string>> names_per_run;
-    for (int run = 0; run < 3; ++ run) {
-        RenameTestCollection coll;
-        Preset &parent = add_inmemory_preset(coll, "Parent Process");
-        parent.is_system = true;
-        if (run == 0)
-            seed_directory();
-
-        PresetsConfigSubstitutions substitutions;
-        coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                          ForwardCompatibilitySubstitutionRule::Disable);
-
-        std::vector<std::string> names;
-        for (auto it = coll.begin(); it != coll.end(); ++ it)
-            names.push_back(it->name + "|" + it->alias + "|" + it->inherits());
-        names_per_run.push_back(std::move(names));
-    }
-
-    REQUIRE(names_per_run[0].size() > 200);
-    CHECK(names_per_run[1] == names_per_run[0]);
-    CHECK(names_per_run[2] == names_per_run[0]);
-}
-
-TEST_CASE("An unreadable user preset is counted and removed while the rest still load", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir   temp_dir;
-    RenameTestCollection coll;
-    const fs::path       dir = temp_dir.path() / PRESET_PRINT_NAME;
-
-    for (int i = 0; i < 20; ++ i)
-        write_preset_with_inherits(coll.default_preset().config, dir / ("Good " + std::to_string(i) + ".json"),
-                                   "Good " + std::to_string(i), std::string());
-    fs::create_directories(dir);
-    std::ofstream((dir / "Broken.json").string()) << "{not-json";
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Broken") == nullptr);
-    CHECK_FALSE(fs::exists(dir / "Broken.json"));
-    for (int i = 0; i < 20; ++ i)
-        CHECK(coll.find_preset("Good " + std::to_string(i)) != nullptr);
-}
-
-TEST_CASE("A user filament naming no compatible printer gets the one after its @, in memory and on disk", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir temp_dir;
-    PresetBundle       bundle;
-    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
-    REQUIRE(bundle.filaments.default_preset().config.option<ConfigOptionStrings>("compatible_printers")->values.empty());
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    const std::vector<std::string> expected { "Test Printer" };
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == expected);
-
-    DynamicPrintConfig                 saved;
-    std::map<std::string, std::string> key_values;
-    std::string                        reason;
-    saved.load_from_json(file.string(), ForwardCompatibilitySubstitutionRule::EnableSilent, key_values, reason);
-    REQUIRE(reason.empty());
-    REQUIRE(saved.option<ConfigOptionStrings>("compatible_printers") != nullptr);
-    CHECK(saved.option<ConfigOptionStrings>("compatible_printers")->values == expected);
-}
-
-TEST_CASE("A user preset's setting id equal to its base id is dropped in memory, not in the .info written back", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir temp_dir;
-    PresetBundle       bundle;
-    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-    fs::path info = file;
-    info.replace_extension(".info");
-    std::ofstream(info.string()) << "sync_info = \nuser_id = \nsetting_id = PFUS1\nbase_id = PFUS1\nupdated_time = 0\n";
-
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->setting_id.empty());
-    CHECK(preset->base_id == "PFUS1");
-    Preset reloaded(Preset::TYPE_FILAMENT, "My PLA @Test Printer");
-    reloaded.load_info(info.string());
-    CHECK(reloaded.setting_id == "PFUS1");
-}
-
-TEST_CASE("A user preset that is not loaded still reports its substituted values", "[Preset][Bundle]")
-{
-    ScopedTemporaryDir   temp_dir;
-    RenameTestCollection coll;
-    const fs::path       dir = temp_dir.path() / PRESET_PRINT_NAME;
-    fs::create_directories(dir);
-    std::ofstream((dir / "Orphan.json").string())
-        << R"({"type":"process","name":"Orphan","from":"User","version":"1.0.0","inherits":"No Such Parent",)"
-        << R"("wall_generator":"no_such_generator"})";
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Enable);
-
-    CHECK(coll.find_preset("Orphan") == nullptr);
-    CHECK(coll.error_count() == 1);
-    REQUIRE(substitutions.size() == 1);
-    CHECK(substitutions.front().preset_name == "Orphan");
-}
-
 TEST_CASE("find_preset resolves a system preset's renamed_from", "[Preset][Rename]")
 {
     RenameTestCollection coll;
@@ -5653,214 +5482,3 @@ TEST_CASE("Config import confines zip entries, preset names and bundle ids to th
         CHECK_FALSE(any_filename_contains(temp_dir.path(), "bundle-escape"));
     }
 }
-
-TEST_CASE("A user preset saved over by another instance while its directory loads is read again under the lock", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.config.option<ConfigOptionFloat>("layer_height", true)->value = 0.24;
-    parent.is_system = true;
-
-    // Fewer than one batch, so every file is read before the first one is committed.
-    constexpr int  children = 8;
-    static_assert(children <= int(resolve_batch_size));
-    const fs::path dir      = temp_dir.path() / PRESET_PRINT_NAME;
-    for (int i = 0; i < children; ++ i)
-        write_minimal_child(dir / ("Child " + std::to_string(i) + ".json"), "Child " + std::to_string(i), "Parent Process");
-
-    // The first commit stands in for another instance saving every other preset after
-    // they were read and before they are committed.
-    std::string first;
-    auto save_the_others = [&](Preset &preset) {
-        if (! first.empty())
-            return;
-        first = preset.name;
-        for (int i = 0; i < children; ++ i) {
-            const std::string name = "Child " + std::to_string(i);
-            if (name != first)
-                std::ofstream((dir / (name + ".json")).string())
-                    << R"({"type":"process","name":")" << name
-                    << R"(","from":"User","version":"1.0.0","inherits":"Parent Process","layer_height":"0.3"})";
-        }
-    };
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable, save_the_others);
-
-    REQUIRE_FALSE(first.empty());
-    CHECK(coll.error_count() == 0);
-    for (int i = 0; i < children; ++ i) {
-        const std::string name  = "Child " + std::to_string(i);
-        const Preset     *child = coll.find_preset(name);
-        REQUIRE(child != nullptr);
-        CHECK_THAT(child->config.opt_float("layer_height"), Catch::Matchers::WithinAbs(name == first ? 0.24 : 0.3, 1e-9));
-    }
-}
-
-TEST_CASE("A user preset removed by another instance while its directory loads is not installed", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-
-    // Fewer than one batch, so every file is read before the first one is committed.
-    constexpr int  children = 8;
-    static_assert(children <= int(resolve_batch_size));
-    const fs::path dir      = temp_dir.path() / PRESET_PRINT_NAME;
-    for (int i = 0; i < children; ++ i)
-        write_minimal_child(dir / ("Child " + std::to_string(i) + ".json"), "Child " + std::to_string(i), "Parent Process");
-
-    std::string first;
-    auto remove_the_others = [&](Preset &preset) {
-        if (! first.empty())
-            return;
-        first = preset.name;
-        for (int i = 0; i < children; ++ i)
-            if (const std::string name = "Child " + std::to_string(i); name != first)
-                fs::remove(dir / (name + ".json"));
-    };
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable, remove_the_others);
-
-    REQUIRE_FALSE(first.empty());
-    CHECK(coll.error_count() == 0);
-    CHECK(coll.size() == 3); // the default preset, the parent and the first child
-    CHECK(coll.find_preset(first) != nullptr);
-}
-
-TEST_CASE("Without the instance lock a user preset loads but its file is not written back", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir temp_dir;
-    ScopedDataDir      data_dir_scope(temp_dir.path());
-    // A directory where the lock file belongs, so the lock cannot be taken.
-    fs::create_directories(fs::path(user_presets_lock_path()));
-    PresetBundle       bundle;
-    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-    const std::string before = read_file(file);
-
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == std::vector<std::string>{ "Test Printer" });
-    CHECK(read_file(file) == before);
-}
-
-#ifndef _WIN32
-// File permissions stop reading only on POSIX.
-TEST_CASE("A user preset that cannot be read under the instance lock is counted and removed", "[Preset][Bundle][InstanceLock]")
-{
-    if (::geteuid() == 0)
-        SKIP("file permissions do not stop root");
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-    const fs::path file = temp_dir.path() / PRESET_PRINT_NAME / "Unreadable.json";
-    write_minimal_child(file, "Unreadable", "Parent Process");
-    fs::permissions(file, fs::no_perms);
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable);
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Unreadable") == nullptr);
-    CHECK_FALSE(fs::exists(file));
-}
-#endif
-
-#ifndef _WIN32
-// Creating a symlink needs no privilege only on POSIX.
-TEST_CASE("A user preset that is a symlink to itself is counted and removed while the rest still load", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-    const fs::path dir = temp_dir.path() / PRESET_PRINT_NAME;
-    write_minimal_child(dir / "Good A.json", "Good A", "Parent Process");
-    write_minimal_child(dir / "Good B.json", "Good B", "Parent Process");
-    const fs::path loop = dir / "Loop.json";
-    fs::create_symlink(loop.filename(), loop);
-
-    PresetsConfigSubstitutions substitutions;
-    REQUIRE_NOTHROW(coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                                      ForwardCompatibilitySubstitutionRule::Disable));
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Good A") != nullptr);
-    CHECK(coll.find_preset("Good B") != nullptr);
-    CHECK(coll.find_preset("Loop") == nullptr);
-    boost::system::error_code ec;
-    CHECK(fs::symlink_status(loop, ec).type() == fs::file_not_found);
-}
-#endif
-
-#ifndef _WIN32
-// Creating a symlink needs no privilege only on POSIX.
-TEST_CASE("A user preset symlinked to a missing target is counted but the link is kept", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    const fs::path dir  = temp_dir.path() / PRESET_PRINT_NAME;
-    const fs::path link = dir / "Linked.json";
-    fs::create_directories(dir);
-    fs::create_symlink(temp_dir.path() / "unmounted" / "Linked.json", link);
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable);
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Linked") == nullptr);
-    boost::system::error_code ec;
-    CHECK(fs::symlink_status(link, ec).type() == fs::symlink_file);
-}
-#endif
-
-#ifndef _WIN32
-// The read-only bit on a directory stops file creation only on POSIX.
-TEST_CASE("A user filament whose derived compatible printer cannot be written back still loads with the failure counted", "[Preset][Bundle]")
-{
-    if (::geteuid() == 0)
-        SKIP("a read-only directory does not stop root");
-    ScopedTemporaryDir temp_dir;
-    PresetBundle       bundle;
-    const fs::path     dir  = temp_dir.path() / PRESET_FILAMENT_NAME;
-    const fs::path     file = dir / "My PLA @Test Printer.json";
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-    const std::string before = read_file(file);
-
-    fs::permissions(file, fs::owner_read);
-    fs::permissions(dir, fs::owner_read | fs::owner_exe);
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-    // Restored before any assertion, so a failure never leaves an unremovable directory behind.
-    fs::permissions(dir, fs::owner_all);
-    fs::permissions(file, fs::owner_read | fs::owner_write);
-
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == std::vector<std::string>{ "Test Printer" });
-    CHECK(bundle.filaments.error_count() == 1);
-    CHECK(read_file(file) == before);
-}
-#endif

tests/libslic3r/test_utils.cpp

@@ -10,8 +10,6 @@
 #include <cctype>
 #include <fstream>
 #include <string>
-#include <thread>
-#include <system_error>
 
 #ifndef _WIN32
 #include <unistd.h>     // getuid
@@ -64,113 +62,6 @@ TEST_CASE("per-user temp root is unchanged on Windows, isolated elsewhere", "[ut
 #endif
 }
 
-TEST_CASE("write_file_atomically replaces the target and leaves no temporary file", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "preset.json";
-
-    REQUIRE_FALSE(write_file_atomically(target.string(), "first"));
-    REQUIRE_FALSE(write_file_atomically(target.string(), "second"));
-
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "second");
-    size_t entries = 0;
-    for (auto &entry : boost::filesystem::directory_iterator(dir.path())) {
-        (void) entry;
-        ++entries;
-    }
-    REQUIRE(entries == 1);
-}
-
-TEST_CASE("write_file_atomically reports a missing directory and writes nothing", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "missing" / "preset.json";
-
-    const std::error_code ec = write_file_atomically(target.string(), "x");
-    REQUIRE(ec == std::errc::no_such_file_or_directory);
-    REQUIRE_FALSE(boost::filesystem::exists(target));
-}
-
-#ifndef _WIN32
-// The read-only bit on a directory stops file creation only on POSIX.
-TEST_CASE("write_file_atomically writes in place when no temporary can be created beside an existing target", "[utils]") {
-    if (::geteuid() == 0)
-        SKIP("a read-only directory does not stop root");
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "preset.json";
-    REQUIRE_FALSE(write_file_atomically(target.string(), "first"));
-    boost::filesystem::permissions(dir.path(), boost::filesystem::owner_read | boost::filesystem::owner_exe);
-
-    const std::error_code replaced = write_file_atomically(target.string(), "second");
-    const std::error_code created  = write_file_atomically((dir.path() / "new.json").string(), "x");
-    // Restored before any assertion, so a failure never leaves an unremovable directory behind.
-    boost::filesystem::permissions(dir.path(), boost::filesystem::owner_all);
-
-    REQUIRE_FALSE(replaced);
-    REQUIRE(created == std::errc::permission_denied);
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "second");
-}
-#endif
-
-TEST_CASE("write_file_atomically keeps bytes intact in binary mode", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "blob.bin";
-    const std::string bytes("a\r\nb\0c", 6);
-
-    REQUIRE_FALSE(write_file_atomically(target.string(), bytes, /*binary=*/true));
-    REQUIRE(boost::filesystem::file_size(target) == bytes.size());
-}
-
-#ifndef _WIN32
-TEST_CASE("write_file_atomically writes through a symlink and keeps the target's permissions", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path real = dir.path() / "real.json";
-    const boost::filesystem::path link = dir.path() / "link.json";
-    REQUIRE_FALSE(write_file_atomically(real.string(), "first"));
-    boost::filesystem::permissions(real, boost::filesystem::owner_read | boost::filesystem::owner_write);
-    boost::filesystem::create_symlink(real, link);
-
-    REQUIRE_FALSE(write_file_atomically(link.string(), "second"));
-
-    REQUIRE(boost::filesystem::is_symlink(boost::filesystem::symlink_status(link)));
-    std::string content;
-    load_string_file(real, content);
-    REQUIRE(content == "second");
-
-    REQUIRE_FALSE(write_file_atomically(real.string(), "third"));
-    const auto perms = boost::filesystem::status(real).permissions() & boost::filesystem::all_all;
-    REQUIRE(perms == (boost::filesystem::owner_read | boost::filesystem::owner_write));
-}
-#endif
-
-TEST_CASE("write_file_atomically survives two threads writing one target", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "shared.json";
-    const std::string a(20000, 'a'), b(20000, 'b');
-
-    std::thread other([&] {
-        for (int i = 0; i < 50; ++i)
-            write_file_atomically(target.string(), a);
-    });
-    for (int i = 0; i < 50; ++i)
-        write_file_atomically(target.string(), b);
-    other.join();
-
-    std::string content;
-    load_string_file(target, content);
-    const bool whole = content == a || content == b;
-    REQUIRE(whole);
-    // No temporary may be left; a scanner on Windows may briefly hold the old
-    // file under another name, so only the temporaries are counted.
-    size_t temporaries = 0;
-    for (auto &entry : boost::filesystem::directory_iterator(dir.path()))
-        if (entry.path().extension() == ".tmp")
-            ++temporaries;
-    REQUIRE(temporaries == 0);
-}
-
 TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") {
     ScopedTemporaryFile source(".txt");
     {
@@ -352,3 +243,82 @@ TEST_CASE("find_unused_filename gives up after 999 versions", "[Utils]") {
     REQUIRE_FALSE(find_unused_filename(dir.path(), "model.3mf", {}, name));
     CHECK(name == "model(999).3mf");
 }
+
+TEST_CASE("is_path_within_root accepts a root given with a trailing separator", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string root = tmp.path().string();
+    const std::string with_separator = GENERATE_COPY(root + "/", root + std::string(1, static_cast<char>(boost::filesystem::path::preferred_separator)));
+
+    CAPTURE(with_separator);
+    CHECK(is_path_within_root("vendor.json", with_separator));
+    CHECK(is_path_within_root("vendor/machine/printer.json", with_separator));
+    CHECK_FALSE(is_path_within_root("../vendor.json", with_separator));
+}
+
+TEST_CASE("is_path_within_root treats Windows-specific name forms the same on every platform", "[utils]") {
+    ScopedTemporaryDir tmp;
+
+    SECTION("names ending in dots or spaces stay inside the root") {
+        const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("dir./file.json"), std::string("dir /file.json"));
+        CAPTURE(name);
+        CHECK(is_path_within_root(name, tmp.path()));
+    }
+    SECTION("drive-relative names are rejected") {
+        const std::string name = GENERATE(std::string("C:x"), std::string("c:x/y.json"), std::string("C:"));
+        CAPTURE(name);
+        CHECK_FALSE(is_path_within_root(name, tmp.path()));
+    }
+}
+
+TEST_CASE("is_path_within_root rejects a name with an embedded NUL", "[utils]") {
+    ScopedTemporaryDir tmp;
+    // The filesystem calls stop at the NUL, so they would act on a different path than the one checked.
+    const std::string name = GENERATE(std::string("..\0", 3), std::string("..\0x/file.json", 14), std::string("sub/..\0x", 8),
+                                      std::string("file.json\0", 10), std::string("\0file.json", 10));
+    CAPTURE(name.size());
+    CHECK_FALSE(is_path_within_root(name, tmp.path()));
+}
+
+TEST_CASE("is_symlink_target_within_root accepts relative targets that stay inside the root", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const auto [link, target] = GENERATE(std::make_pair(std::string("Versions/Current"), std::string("A")),
+                                         std::make_pair(std::string("Foo.framework/Foo"), std::string("Versions/Current/Foo")),
+                                         std::make_pair(std::string("libfoo.so"), std::string("libfoo.so.1")),
+                                         std::make_pair(std::string("a/b/link"), std::string("c/d")));
+    CAPTURE(link, target);
+    CHECK(is_symlink_target_within_root(link, target, tmp.path()));
+}
+
+TEST_CASE("is_symlink_target_within_root rejects absolute targets and targets that climb out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string outside = (tmp.path().parent_path() / "outside").generic_string();
+    const auto [link, target] = GENERATE_COPY(std::make_pair(std::string("sub/link"), outside),
+                                              std::make_pair(std::string("sub/link"), std::string("/etc/passwd")),
+                                              std::make_pair(std::string("sub/link"), std::string("\\outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:/outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("")),
+                                              std::make_pair(std::string("link"), std::string("..")),
+                                              std::make_pair(std::string("link"), std::string("../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("../../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("x/../../../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("..\\..\\outside")),
+                                              // symlink() stops at the NUL, so this target would be created as "..".
+                                              std::make_pair(std::string("link"), std::string("..\0", 3)));
+    CAPTURE(link, target);
+    CHECK_FALSE(is_symlink_target_within_root(link, target, tmp.path()));
+}
+
+#ifndef _WIN32
+TEST_CASE("is_symlink_target_within_root rejects a target that passes through a symlink leading out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const boost::filesystem::path root    = tmp.path() / "root";
+    const boost::filesystem::path outside = tmp.path() / "outside";
+    boost::filesystem::create_directories(root);
+    boost::filesystem::create_directories(outside);
+    boost::filesystem::create_symlink(outside, root / "out");
+
+    CHECK_FALSE(is_symlink_target_within_root("link", "out/lib.so", root));
+    CHECK(is_symlink_target_within_root("link", "in/lib.so", root));
+}
+#endif

tests/libslic3r/test_vendor_cache.cpp

@@ -3,21 +3,12 @@
 #include <boost/filesystem.hpp>
 #include <boost/crc.hpp>
 #include <cereal/archives/binary.hpp>
-#include <algorithm>
-#include <atomic>
 #include <cstring>
 #include <fstream>
 #include <functional>
-#include <memory>
-#include <numeric>
-#include <random>
 #include <set>
 #include <sstream>
 
-#include <tbb/blocked_range.h>
-#include <tbb/parallel_for.h>
-#include <tbb/task_group.h>
-
 #include "libslic3r/PresetBundle.hpp"
 #include "libslic3r/PresetCacheFormat.hpp"
 #include "libslic3r/Preset.hpp"
@@ -61,27 +52,6 @@ void write_vendor_tree(const fs::path& dir, const std::string& vendor, const std
         << R"(","from":"system","instantiation":"true","layer_height":"0.2"})";
 }
 
-// A vendor whose process list is `processes`, each a preset name and the text of
-// its sub-file, listed in that order.
-void write_process_vendor(const fs::path& dir, const std::string& vendor,
-                          const std::vector<std::pair<std::string, std::string>>& processes)
-{
-    fs::create_directories(dir / vendor / "process");
-    std::ofstream index((dir / (vendor + ".json")).string());
-    index << R"({"version":"1.0.0","name":")" << vendor << R"(","process_list":[)";
-    for (size_t i = 0; i < processes.size(); ++ i) {
-        const std::string sub_path = "process/p" + std::to_string(i) + ".json";
-        index << (i ? "," : "") << R"({"name":")" << processes[i].first << R"(","sub_path":")" << sub_path << R"("})";
-        std::ofstream((dir / vendor / sub_path).string()) << processes[i].second;
-    }
-    index << "]}";
-}
-
-std::string process_json(const std::string& name, const std::string& extra = std::string())
-{
-    return R"({"type":"process","name":")" + name + R"(","from":"system",)" + extra + R"("layer_height":"0.2"})";
-}
-
 // A small but complete vendor: one machine model, one process, a non-instantiated
 // base filament with an instantiated child that inherits it and includes a
 // dual-extruder template, a second standalone filament carrying explicit
@@ -158,80 +128,6 @@ void write_lib_tree(const fs::path& dir, const std::string& version, const std::
         << R"("filament_id":"GFL99","filament_cost":")" << cost << R"("})";
 }
 
-struct FixtureEntry {
-    std::string         name, sub_path;
-    std::vector<size_t> deps;   // indices of the entries this one inherits or includes
-};
-
-// Filaments with two levels of inheritance, a template to include, and `leaves`
-// presets that inherit either level, some including the template. Only the
-// subfiles are written; write_filament_list lists them.
-std::vector<FixtureEntry> write_layered_filaments(const fs::path& dir, const std::string& vendor, int leaves)
-{
-    fs::create_directories(dir / vendor / "filament");
-    std::vector<FixtureEntry> entries {
-        { vendor + " Base PLA", "filament/base.json", {} },
-        { vendor + " Mid PLA", "filament/mid.json", {0} },
-        { vendor + " dual template", "filament/template.json", {} },
-    };
-    std::ofstream((dir / vendor / "filament" / "base.json").string())
-        << R"({"type":"filament","name":")" << entries[0].name << R"(","from":"system","instantiation":"false",)"
-        << R"("filament_id":"GFA_base","filament_cost":"42","filament_max_volumetric_speed":["12"]})";
-    std::ofstream((dir / vendor / "filament" / "mid.json").string())
-        << R"({"type":"filament","name":")" << entries[1].name << R"(","from":"system","instantiation":"false",)"
-        << R"("inherits":")" << entries[0].name << R"(","filament_flow_ratio":"0.95"})";
-    std::ofstream((dir / vendor / "filament" / "template.json").string())
-        << R"({"type":"filament","name":")" << entries[2].name << R"(","from":"system","instantiation":"false",)"
-        << R"("filament_extruder_variant":["Direct Drive Standard","Direct Drive High Flow"],)"
-        << R"("filament_max_volumetric_speed":["20","22"]})";
-    for (int i = 0; i < leaves; ++ i) {
-        const size_t parent  = i % 2;
-        const bool   include = i % 3 == 0;
-        entries.push_back({ vendor + " PLA " + std::to_string(i) + " @0.4", "filament/leaf" + std::to_string(i) + ".json",
-                            include ? std::vector<size_t>{parent, 2} : std::vector<size_t>{parent} });
-        std::ofstream((dir / vendor / entries.back().sub_path).string())
-            << R"({"type":"filament","name":")" << entries.back().name << R"(","from":"system","instantiation":"true",)"
-            << R"("inherits":")" << entries[parent].name << R"(",)"
-            << (include ? R"("include":[")" + entries[2].name + R"("],)" : std::string())
-            << R"("nozzle_temperature":[")" << (200 + i % 40) << R"("]})";
-    }
-    return entries;
-}
-
-// The vendor profile for write_layered_filaments, listing its entries in `order`,
-// or as they were written when `order` is empty.
-void write_filament_list(const fs::path& dir, const std::string& vendor, const std::vector<FixtureEntry>& entries,
-                         std::vector<size_t> order = {})
-{
-    if (order.empty()) {
-        order.resize(entries.size());
-        std::iota(order.begin(), order.end(), 0);
-    }
-    std::ofstream f((dir / (vendor + ".json")).string());
-    f << R"({"version":"1.0.0","name":")" << vendor << R"(","filament_list":[)";
-    for (size_t k = 0; k < order.size(); ++ k)
-        f << (k ? "," : "") << R"({"name":")" << entries[order[k]].name << R"(","sub_path":")"
-          << entries[order[k]].sub_path << R"("})";
-    f << "]}";
-}
-
-// A random order of `entries` that lists each one after everything it depends on.
-std::vector<size_t> shuffled_after_dependencies(const std::vector<FixtureEntry>& entries, std::mt19937& rng)
-{
-    std::vector<size_t> order;
-    std::vector<bool>   listed(entries.size(), false);
-    while (order.size() < entries.size()) {
-        std::vector<size_t> ready;
-        for (size_t i = 0; i < entries.size(); ++ i)
-            if (! listed[i] && std::all_of(entries[i].deps.begin(), entries[i].deps.end(), [&](size_t d) { return listed[d]; }))
-                ready.push_back(i);
-        const size_t next = ready[std::uniform_int_distribution<size_t>(0, ready.size() - 1)(rng)];
-        listed[next] = true;
-        order.push_back(next);
-    }
-    return order;
-}
-
 // A vendor whose one filament inherits the library's base and states nothing of
 // its own — everything it shows comes from the library it is resolved against.
 void write_vendor_with_lib_filament(const fs::path& dir, const std::string& vendor, const std::string& version)
@@ -728,299 +624,6 @@ TEST_CASE("a cache-loaded vendor is indistinguishable from a JSON-loaded one", "
     CHECK(pr->config.opt_string("machine_start_gcode") == "G28 ; template");   // through the include
 }
 
-TEST_CASE("a wide vendor loads from its cache exactly as it loads from JSON", "[VendorCache]")
-{
-    InstallDirs dirs;
-    constexpr int leaves = 400;
-    write_filament_list(dirs.system, "Acme", write_layered_filaments(dirs.system, "Acme", leaves));
-
-    PresetBundle from_json;
-    from_json.set_generate_vendor_caches(true);
-    from_json.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                            ForwardCompatibilitySubstitutionRule::EnableSilent);
-    REQUIRE(fs::exists(dirs.system / "Acme.opc"));
-
-    fs::remove_all(dirs.system / "Acme");
-    PresetBundle from_cache;
-    from_cache.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                             ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    auto a = presets_for(from_json.filaments, "Acme");
-    auto b = presets_for(from_cache.filaments, "Acme");
-    REQUIRE(a.size() == size_t(leaves));
-    REQUIRE(b.size() == a.size());
-    for (size_t i = 0; i < a.size(); ++ i) {
-        CHECK(a[i]->name == b[i]->name);
-        CHECK(preset_deep_equal(*a[i], *b[i]));
-    }
-    CHECK(from_cache.error_count() == from_json.error_count());
-    // The two-level chain resolved, so the leaf has the middle profile's flow ratio
-    // and the base profile's filament id, neither of which it states.
-    const Preset* leaf = from_cache.filaments.find_preset("Acme PLA 7 @0.4", false);
-    REQUIRE(leaf != nullptr);
-    CHECK(leaf->filament_id == "GFA_base");
-    const auto* flow = leaf->config.option<ConfigOptionFloats>("filament_flow_ratio");
-    REQUIRE(flow != nullptr);
-    CHECK_THAT(flow->values.front(), WithinAbs(0.95, 1e-9));
-}
-
-TEST_CASE("presets install the same in any order that lists each after what it depends on", "[VendorCache]")
-{
-    InstallDirs dirs;
-    const std::vector<FixtureEntry> entries = write_layered_filaments(dirs.system, "Acme", 150);
-    auto load = [&](const std::vector<size_t>& order) {
-        write_filament_list(dirs.system, "Acme", entries, order);
-        auto bundle = std::make_unique<PresetBundle>();
-        bundle->load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                              ForwardCompatibilitySubstitutionRule::EnableSilent);
-        return bundle;
-    };
-    const auto reference = load({});
-    CHECK(reference->error_count() == 0);
-    const auto expected = presets_for(reference->filaments, "Acme");
-    REQUIRE(expected.size() == 150);
-
-    // Every seed checks against one reference load, which GENERATE would repeat per seed.
-    for (unsigned seed = 0; seed < 8; ++ seed) {
-        CAPTURE(seed);
-        std::mt19937 rng(seed);
-        const auto shuffled = load(shuffled_after_dependencies(entries, rng));
-        CHECK(shuffled->error_count() == 0);
-        const auto actual = presets_for(shuffled->filaments, "Acme");
-        REQUIRE(actual.size() == expected.size());
-        for (size_t i = 0; i < actual.size(); ++ i)
-            CHECK(preset_deep_equal(*actual[i], *expected[i]));
-    }
-}
-
-TEST_CASE("a key misplaced into a vendor preset is reported and removed", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_process_vendor(dirs.system, "Acme", {
-        { "Acme template", R"({"type":"process","name":"Acme template","from":"system","instantiation":"false","wall_loops":"5","filament_density":"1.2"})" },
-        { "Acme base", R"({"type":"process","name":"Acme base","from":"system","instantiation":"false","filament_cost":"5"})" },
-        { "0.20mm Standard @Acme", process_json("0.20mm Standard @Acme",
-            R"("instantiation":"true","inherits":"Acme base","include":["Acme template"],"nozzle_temperature":["210"],)") } });
-    PresetBundle bundle;
-    bundle.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                         ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    CHECK(bundle.error_count() == 3);
-    const Preset* preset = bundle.prints.find_preset("0.20mm Standard @Acme", false);
-    REQUIRE(preset != nullptr);
-    CHECK_FALSE(preset->config.has("filament_cost"));
-    CHECK_FALSE(preset->config.has("filament_density"));
-    CHECK_FALSE(preset->config.has("nozzle_temperature"));
-    CHECK(preset->config.opt_int("wall_loops") == 5);
-}
-
-TEST_CASE("each vendor loads from the directory it is listed with", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_vendor_tree(dirs.system, "Acme", "1.0.0");
-    write_vendor_tree(dirs.profiles, "Zeta", "2.0.0");
-    PresetBundle bundle;
-    bundle.load_vendors({ { "Acme", dirs.system }, { "Zeta", dirs.profiles } },
-                        ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/false);
-
-    CHECK(bundle.vendors.count("Acme") == 1);
-    CHECK(bundle.vendors.count("Zeta") == 1);
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Acme", false) != nullptr);
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Zeta", false) != nullptr);
-}
-
-TEST_CASE("a vendor that fails to load is left out, reported, and the others still load", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_vendor_tree(dirs.system, "Acme", "1.0.0");
-    write_process_vendor(dirs.system, "Broken", { { "Broken A", "{not-json" } });
-    PresetBundle             bundle;
-    std::vector<std::string> failed;
-    const std::string errors = bundle.load_vendors({ { "Acme", dirs.system }, { "Broken", dirs.system } },
-                                                   ForwardCompatibilitySubstitutionRule::EnableSilent,
-                                                   /*allow_cache=*/false, nullptr, &failed).second;
-
-    CHECK(bundle.vendors.count("Acme") == 1);
-    CHECK(bundle.vendors.count("Broken") == 0);
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Acme", false) != nullptr);
-    CHECK(errors.find("Broken") != std::string::npos);
-    CHECK(failed == std::vector<std::string>{ "Broken" });
-}
-
-TEST_CASE("a filament library that fails partway is reported, and vendors inheriting from it are left out", "[VendorCache]")
-{
-    InstallDirs dirs;
-    const std::string lib(PresetBundle::ORCA_FILAMENT_LIBRARY);
-    write_lib_tree(dirs.system, "1.0.0", "20");
-    std::ofstream((dirs.system / (lib + ".json")).string())
-        << R"({"version":"1.0.0","name":")" << lib << R"(","filament_list":[)"
-        << R"({"name":"Generic PLA","sub_path":"filament/generic_pla.json"},)"
-        << R"({"name":"Generic PETG","sub_path":"filament/generic_petg.json"}]})";
-    std::ofstream((dirs.system / lib / "filament" / "generic_petg.json").string()) << "{not-json";
-    write_vendor_with_lib_filament(dirs.system, "Acme", "1.0.0");
-    write_vendor_tree(dirs.system, "Zeta", "1.0.0");
-
-    PresetBundle             bundle;
-    std::vector<std::string> failed;
-    const std::string errors = bundle.load_vendors({ { "Acme", dirs.system }, { lib, dirs.system }, { "Zeta", dirs.system } },
-                                                   ForwardCompatibilitySubstitutionRule::EnableSilent,
-                                                   /*allow_cache=*/false, nullptr, &failed).second;
-
-    CHECK(failed == std::vector<std::string>{ lib, "Acme" });
-    CHECK(errors.find("generic_petg.json") != std::string::npos);
-    CHECK(bundle.vendors.count(lib) == 1);
-    CHECK(bundle.vendors.count("Acme") == 0);
-    CHECK(bundle.filaments.find_preset("Acme PLA @0.4", false) == nullptr);
-    CHECK(bundle.vendors.count("Zeta") == 1);
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Zeta", false) != nullptr);
-}
-
-TEST_CASE("a vendor whose profile cannot be read is left out, reported, and the others still load", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_vendor_tree(dirs.system, "Acme", "1.0.0");
-    std::ofstream((dirs.system / "Broken.json").string()) << "{not-json";
-    PresetBundle bundle;
-    const std::string errors = bundle.load_vendors({ { "Acme", dirs.system }, { "Broken", dirs.system } },
-                                                   ForwardCompatibilitySubstitutionRule::EnableSilent,
-                                                   /*allow_cache=*/false).second;
-
-    CHECK(bundle.vendors.count("Acme") == 1);
-    CHECK(bundle.vendors.count("Broken") == 0);
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Acme", false) != nullptr);
-    CHECK(errors.find("Broken.json") != std::string::npos);
-}
-
-TEST_CASE("a canceled vendor load starts no vendor", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_vendor_tree(dirs.system, "Acme", "1.0.0");
-    write_vendor_tree(dirs.system, "Zeta", "1.0.0");
-    const std::atomic<bool> cancel { true };
-    PresetBundle bundle;
-    bundle.load_vendors({ { "Acme", dirs.system }, { "Zeta", dirs.system } },
-                        ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/false, &cancel);
-
-    CHECK(bundle.vendors.empty());
-    CHECK(bundle.prints.find_preset("0.20mm Standard @Acme", false) == nullptr);
-}
-
-TEST_CASE("a preset two vendors both define is kept from the first listed and reported under the others", "[VendorCache]")
-{
-    InstallDirs dirs;
-    for (const std::string vendor : { "Acme", "Mira", "Zeta" })
-        write_process_vendor(dirs.system, vendor, {
-            { "Shared", process_json("Shared", R"("instantiation":"true",)") },
-            { "Own @" + vendor, process_json("Own @" + vendor, R"("instantiation":"true",)") } });
-    PresetBundle bundle;
-    const std::string errors = bundle.load_vendors({ { "Mira", dirs.system }, { "Acme", dirs.system }, { "Zeta", dirs.system } },
-                                                   ForwardCompatibilitySubstitutionRule::EnableSilent,
-                                                   /*allow_cache=*/false).second;
-
-    const Preset* shared = bundle.prints.find_preset("Shared", false);
-    REQUIRE(shared != nullptr);
-    REQUIRE(shared->vendor != nullptr);
-    CHECK(shared->vendor->id == "Mira");
-    CHECK(errors.find("vendor Mira") == std::string::npos);
-    CHECK(errors.find("Found duplicated settings in vendor Acme's json file lists: Shared") != std::string::npos);
-    CHECK(errors.find("Found duplicated settings in vendor Zeta's json file lists: Shared") != std::string::npos);
-    CHECK(bundle.error_count() == 2);
-    for (const std::string vendor : { "Acme", "Mira", "Zeta" }) {
-        const Preset* own = bundle.prints.find_preset("Own @" + vendor, false);
-        REQUIRE(own != nullptr);
-        CHECK(own->vendor == &bundle.vendors.at(vendor));
-    }
-}
-
-TEST_CASE("filaments merged from several vendors come out generic first, then by name", "[VendorCache]")
-{
-    InstallDirs dirs;
-    auto write_filaments = [&](const std::string& vendor, const std::vector<std::string>& names) {
-        fs::create_directories(dirs.system / vendor / "filament");
-        std::ofstream index((dirs.system / (vendor + ".json")).string());
-        index << R"({"version":"1.0.0","name":")" << vendor << R"(","filament_list":[)";
-        for (size_t i = 0; i < names.size(); ++ i) {
-            const std::string sub_path = "filament/f" + std::to_string(i) + ".json";
-            index << (i ? "," : "") << R"({"name":")" << names[i] << R"(","sub_path":")" << sub_path << R"("})";
-            std::ofstream((dirs.system / vendor / sub_path).string())
-                << R"({"type":"filament","name":")" << names[i] << R"(","from":"system","instantiation":"true",)"
-                << R"("filament_id":"GF)" << vendor << i << R"("})";
-        }
-        index << "]}";
-    };
-    write_filaments("Zeta", { "Zeta PLA @0.4", "Generic PETG @Zeta" });
-    write_filaments("Acme", { "Acme PLA @0.4", "Generic PLA @Acme" });
-    PresetBundle bundle;
-    bundle.load_vendors({ { "Zeta", dirs.system }, { "Acme", dirs.system } },
-                        ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/false);
-
-    std::vector<std::string> names;
-    for (const Preset& preset : bundle.filaments.get_presets())
-        if (! preset.is_default)
-            names.push_back(preset.name);
-    CHECK(names == std::vector<std::string>{ "Generic PETG @Zeta", "Generic PLA @Acme", "Acme PLA @0.4", "Zeta PLA @0.4" });
-    for (const std::string& name : names)
-        CHECK(bundle.filaments.find_preset(name, false) != nullptr);
-}
-
-TEST_CASE("a vendor read while the filament library loads resolves against it, from JSON and from its cache", "[VendorCache]")
-{
-    InstallDirs dirs;
-    const std::string lib(PresetBundle::ORCA_FILAMENT_LIBRARY);
-    write_lib_tree(dirs.system, "1.0.0", "20");
-    write_vendor_with_lib_filament(dirs.system, "Acme", "1.0.0");
-    auto load = [&] {
-        auto bundle = std::make_unique<PresetBundle>();
-        bundle->set_generate_vendor_caches(true);
-        bundle->load_vendors({ { "Acme", dirs.system }, { lib, dirs.system } },
-                             ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/true);
-        return bundle;
-    };
-
-    const auto from_json = load();
-    REQUIRE(fs::exists(dirs.system / "Acme.opc"));
-    fs::remove_all(dirs.system / "Acme");
-    const auto from_cache = load();
-
-    for (const PresetBundle* bundle : { from_json.get(), from_cache.get() }) {
-        const Preset* pla = bundle->filaments.find_preset("Acme PLA @0.4", false);
-        REQUIRE(pla != nullptr);
-        CHECK(pla->filament_id == "GFL99");
-        CHECK_THAT(pla->config.option<ConfigOptionFloats>("filament_cost")->values.front(), WithinAbs(20., 1e-9));
-        CHECK(bundle->error_count() == 0);
-    }
-}
-
-TEST_CASE("repeated cache loads of one vendor produce the same presets", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_filament_list(dirs.system, "Acme", write_layered_filaments(dirs.system, "Acme", 400));
-
-    PresetBundle seed;
-    seed.set_generate_vendor_caches(true);
-    seed.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                       ForwardCompatibilitySubstitutionRule::EnableSilent);
-    REQUIRE(fs::exists(dirs.system / "Acme.opc"));
-    fs::remove_all(dirs.system / "Acme");
-
-    std::vector<PresetBundle> loads(3);
-    for (PresetBundle& bundle : loads)
-        bundle.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                             ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    auto first = presets_for(loads[0].filaments, "Acme");
-    REQUIRE(!first.empty());
-    for (size_t run = 1; run < loads.size(); ++ run) {
-        auto again = presets_for(loads[run].filaments, "Acme");
-        REQUIRE(again.size() == first.size());
-        for (size_t i = 0; i < first.size(); ++ i) {
-            CHECK(first[i]->name == again[i]->name);
-            CHECK(preset_deep_equal(*first[i], *again[i]));
-        }
-        CHECK(loads[run].error_count() == loads[0].error_count());
-    }
-}
-
 TEST_CASE("a cache-served vendor reports the errors its parse counted", "[VendorCache]")
 {
     TempDir tmp;
@@ -1263,27 +866,6 @@ TEST_CASE("a vendor installed as its cache alone still loads after a library upd
     CHECK_THAT(cost->values.front(), WithinAbs(30., 1e-9));
 }
 
-TEST_CASE("a cache install stopped by a canceled task group throws instead of rejecting the cache", "[VendorCache]")
-{
-    TempDir tmp;
-    const std::string cache = (tmp.path / "Acme.opc").string();
-    REQUIRE(save_one_vendor(cache, one_vendor("Acme"), "Acme", "1.0.0", {filament_entry("Acme PLA @0.4")}));
-
-    PresetBundle            bundle;
-    bool                    threw = false;
-    tbb::task_group_context context;
-    tbb::parallel_for(tbb::blocked_range<size_t>(0, 1), [&](const tbb::blocked_range<size_t>&) {
-        context.cancel_group_execution();
-        try {
-            bundle.load_vendor_cache(cache, "Acme", Semver(1, 0, 0));
-        } catch (const std::runtime_error&) {
-            threw = true;
-        }
-    }, context);
-
-    CHECK(threw);
-}
-
 TEST_CASE("a cache entry whose parent is missing falls back to the vendor's JSONs", "[VendorCache]")
 {
     TempDir tmp;
@@ -1311,12 +893,6 @@ TEST_CASE("a cache entry whose parent is missing falls back to the vendor's JSON
         user.string(), "Acme", PresetBundle::LoadSystem, ForwardCompatibilitySubstitutionRule::EnableSilent);
     CHECK(presets_loaded == 1);
     CHECK(out.vendors.at("Acme").name == "Acme");   // the profile's name, not the cache's
-
-    // Through load_vendors, which reads the cache before installing it.
-    PresetBundle several;
-    several.load_vendors({ { "Acme", user } }, ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/true);
-    CHECK(several.vendors.at("Acme").name == "Acme");
-    CHECK(several.prints.find_preset("0.20mm Standard @Acme", false) != nullptr);
 }
 
 TEST_CASE("a profile with no usable version is never served from cache", "[VendorCache]")
@@ -1804,6 +1380,24 @@ TEST_CASE("a header claiming more body than the file holds is rejected", "[Vendo
     REQUIRE_FALSE(bundle.load_vendor_cache(cache, "Bounded", Semver(1, 0, 0)));
 }
 
+TEST_CASE("a failed write leaves the previous cache in place", "[VendorCache]")
+{
+    TempDir tmp;
+    const std::string cache = (tmp.path / "Durable.opc").string();
+    REQUIRE(save_one_vendor(cache, one_vendor("Durable"), "Durable", "1.0.0"));
+    const std::string before = slurp(cache);
+
+    // A directory where the temp file wants to go: the write cannot complete,
+    // and must not have destroyed what was already there to find that out.
+    const fs::path blocker = fs::path(cache + "." + std::to_string(get_current_pid()) + ".tmp");
+    fs::create_directories(blocker);
+
+    REQUIRE_FALSE(save_one_vendor(cache, one_vendor("Durable"), "Durable", "2.0.0"));
+    CHECK(slurp(cache) == before);
+
+    fs::remove_all(blocker);
+}
+
 TEST_CASE("a cache written by another build's option ordering still loads", "[VendorCache]")
 {
     // The regression the fingerprint used to prevent by refusing the file
@@ -2097,31 +1691,6 @@ TEST_CASE("an include listed after the preset that names it is an error, and the
     CHECK(silk->config.option<ConfigOptionFloats>("filament_max_volumetric_speed")->values == std::vector<double>{12.});
 }
 
-TEST_CASE("an include listed after the preset that names it is missing from the cache load too", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_full_vendor_tree(dirs.system, "Acme", "1.0.0", /*templates_last=*/true);
-    PresetBundle from_json;
-    from_json.set_generate_vendor_caches(true);
-    from_json.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                            ForwardCompatibilitySubstitutionRule::EnableSilent);
-    REQUIRE(fs::exists(dirs.system / "Acme.opc"));
-
-    fs::remove_all(dirs.system / "Acme");
-    PresetBundle from_cache;
-    from_cache.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                             ForwardCompatibilitySubstitutionRule::EnableSilent);
-    CHECK(from_cache.error_count() == 2);
-    const Preset* pr = from_cache.printers.find_preset("Acme 0.4 nozzle", false);
-    REQUIRE(pr != nullptr);
-    CHECK(pr->config.opt_string("machine_start_gcode") != "G28 ; template");
-    const Preset* silk = from_cache.filaments.find_preset("Acme Silk PLA @0.4", false);
-    REQUIRE(silk != nullptr);
-    const auto& speed = silk->config.option<ConfigOptionFloats>("filament_max_volumetric_speed")->values;
-    REQUIRE(speed.size() == 1);
-    CHECK_THAT(speed.front(), WithinAbs(12., 1e-9));
-}
-
 TEST_CASE("a G-code template that states no instantiation is included, not loaded as a preset", "[VendorCache]")
 {
     InstallDirs dirs;
@@ -2143,35 +1712,3 @@ TEST_CASE("a G-code template that states no instantiation is included, not loade
     CHECK(pr->config.opt_string("machine_start_gcode") == "G28 ; template");
     CHECK(presets_for(bundle.printers, "Acme").size() == 1);
 }
-
-TEST_CASE("a sub-file that fails to parse leaves the ones listed before it installed", "[VendorCache]")
-{
-    InstallDirs dirs;
-    write_process_vendor(dirs.system, "Acme", {
-        { "Acme A", process_json("Acme A", R"("instantiation":"true",)") },
-        { "Acme B", "{not-json" },
-        { "Acme C", process_json("Acme C", R"("instantiation":"true",)") } });
-    PresetBundle bundle;
-    CHECK_THROWS_AS(bundle.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                                         ForwardCompatibilitySubstitutionRule::EnableSilent),
-                    ConfigurationError);
-    CHECK(bundle.prints.find_preset("Acme A", false) != nullptr);
-    CHECK(bundle.prints.find_preset("Acme C", false) == nullptr);
-}
-
-TEST_CASE("a load that fails to install partway counts no errors of the sub-files after it", "[VendorCache]")
-{
-    InstallDirs dirs;
-    // B names a parent nobody defines. C states no instantiation, an error of its own.
-    write_process_vendor(dirs.system, "Acme", {
-        { "Acme A", process_json("Acme A", R"("instantiation":"true",)") },
-        { "Acme B", process_json("Acme B", R"("instantiation":"true","inherits":"Nobody",)") },
-        { "Acme C", process_json("Acme C") } });
-    PresetBundle bundle;
-    CHECK_THROWS_AS(bundle.load_vendor_configs_from_json(dirs.system.string(), "Acme", PresetBundle::LoadSystem,
-                                                         ForwardCompatibilitySubstitutionRule::EnableSilent),
-                    ConfigurationError);
-    // B's missing parent and B's failed install.
-    CHECK(bundle.error_count() == 2);
-    CHECK(bundle.prints.find_preset("Acme A", false) != nullptr);
-}

tests/slic3rutils/CMakeLists.txt

@@ -27,6 +27,7 @@ add_executable(${_TEST_NAME}_tests
     test_plugin_cloud_metadata.cpp
     test_plugin_audit.cpp
     test_shortcuts.cpp
+    test_file_url.cpp
     ../fff_print/test_helpers.cpp
     )
 

tests/slic3rutils/test_file_url.cpp

@@ -0,0 +1,74 @@
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/generators/catch_generators.hpp>
+
+#include "slic3r/GUI/GUI.hpp"
+
+#include <boost/filesystem/path.hpp>
+
+#include <wx/uri.h>
+
+using namespace Slic3r::GUI;
+
+#ifndef _WIN32
+TEST_CASE("A file URL keeps characters special to URLs in its path", "[FileUrl]")
+{
+    const std::string path = GENERATE(as<std::string>{},
+        "/opt/test#dir/resources/web/homepage/index.html",
+        "/opt/test%20x/resources/web/homepage/index.html",
+        "/opt/Orca Slicer/resources/web/homepage/index.html",
+        "/opt/what?/resources/web/homepage/index.html",
+        "/home/Jos\xC3\xA9/\xE8\xB5\x84\xE6\xBA\x90/resources/web/homepage/index.html");
+
+    const wxString url = file_url_from_path(boost::filesystem::path(path));
+    CAPTURE(path, url.utf8_string());
+
+    // WebView::CreateWebView() and WebView::LoadUrl() re-parse the URL before loading it.
+    const wxURI uri(wxURI(url).BuildURI());
+    CHECK(uri.GetScheme() == "file");
+    CHECK(!uri.HasQuery());
+    CHECK(!uri.HasFragment());
+    CHECK(wxURI::Unescape(uri.GetPath()).utf8_string() == path);
+}
+
+TEST_CASE("A file URL of a plain path is the path behind file://", "[FileUrl]")
+{
+    const std::string path = "/opt/OrcaSlicer/resources/web/homepage/index.html";
+    CHECK(file_url_from_path(boost::filesystem::path(path)) == "file://" + path);
+}
+
+TEST_CASE("A query appended to a file URL stays separate from its path", "[FileUrl]")
+{
+    const std::string path = "/opt/test#dir%20x/resources/web/guide/0/index.html";
+    const wxURI uri(file_url_from_path(boost::filesystem::path(path)) + "?target=21&lang=de");
+    CHECK(wxURI::Unescape(uri.GetPath()).utf8_string() == path);
+    CHECK(uri.GetQuery() == "target=21&lang=de");
+    CHECK(!uri.HasFragment());
+}
+#else
+TEST_CASE("A file URL of a Windows path has a drive letter and forward slashes", "[FileUrl]")
+{
+    const auto [path, url] = GENERATE(table<std::wstring, std::string>({
+        { L"C:\\Program Files\\OrcaSlicer\\resources\\web\\homepage\\index.html",
+          "file:///C:/Program%20Files/OrcaSlicer/resources/web/homepage/index.html" },
+        { L"D:\\#OneDrive\\OrcaSlicer\\resources\\web\\guide\\0\\index.html",
+          "file:///D:/%23OneDrive/OrcaSlicer/resources/web/guide/0/index.html" },
+        { L"D:\\100%\\OrcaSlicer\\resources\\web\\homepage\\index.html",
+          "file:///D:/100%25/OrcaSlicer/resources/web/homepage/index.html" },
+        // Callers join the resources directory with a forward-slash relative path.
+        { L"D:\\#OneDrive\\OrcaSlicer\\resources/web/homepage/index.html",
+          "file:///D:/%23OneDrive/OrcaSlicer/resources/web/homepage/index.html" },
+        { L"\\\\server\\share\\OrcaSlicer\\resources\\web\\homepage\\index.html",
+          "file://server/share/OrcaSlicer/resources/web/homepage/index.html" },
+    }));
+    CHECK(file_url_from_path(boost::filesystem::path(path)).utf8_string() == url);
+}
+
+TEST_CASE("A query appended to a Windows file URL stays separate from its path", "[FileUrl]")
+{
+    const wxURI uri(file_url_from_path(boost::filesystem::path(L"D:\\#OneDrive\\OrcaSlicer\\resources\\web\\guide\\0\\index.html")) +
+                    "?target=21&lang=de");
+    CHECK(wxURI::Unescape(uri.GetPath()) == "/D:/#OneDrive/OrcaSlicer/resources/web/guide/0/index.html");
+    CHECK(uri.GetQuery() == "target=21&lang=de");
+    CHECK(!uri.HasFragment());
+}
+#endif

tests/slic3rutils/test_plugin_host_api.cpp

@@ -16,6 +16,8 @@
 
 #include <string>
 
+#include <wx/uri.h>
+
 namespace py = pybind11;
 
 namespace {
@@ -210,23 +212,33 @@ TEST_CASE("A reloaded plugin page is recognised by its base URL, fragment aside"
 {
     using namespace Slic3r::GUI::web_hosting;
 
-    // A resources path holding a space, which the web view reports escaped.
+    // A resources path holding a space, which the web view may report escaped differently.
     const Slic3r::ScopedResourcesDir resources("web content check");
 
     // The swapped-in page, then after an in-page anchor and a reload.
     CHECK(is_content_url(content_base_url()));
     CHECK(is_content_url(content_base_url() + "#tab2"));
-    wxString escaped = content_base_url();
+    const wxString unescaped = wxURI::Unescape(content_base_url());
-    escaped.Replace(" ", "%20");
+    REQUIRE(unescaped != content_base_url());
-    REQUIRE(escaped != content_base_url());
+    CHECK(is_content_url(unescaped));
-    CHECK(is_content_url(escaped));
+    CHECK(is_content_url(unescaped + "#tab2"));
-    CHECK(is_content_url(escaped + "#tab2"));
     // A page the plugin linked to keeps its own URL and must be left alone.
     CHECK_FALSE(is_content_url(content_base_url() + "guide.html"));
     CHECK_FALSE(is_content_url("https://example.com/"));
     CHECK_FALSE(is_content_url(""));
 }
 
+TEST_CASE("A reloaded plugin page is recognised when the resources path holds a '#'", "[PluginHost]")
+{
+    using namespace Slic3r::GUI::web_hosting;
+
+    const Slic3r::ScopedResourcesDir resources("web#content check");
+
+    CHECK(is_content_url(content_base_url()));
+    CHECK(is_content_url(content_base_url() + "#tab2"));
+    CHECK_FALSE(is_content_url(content_base_url() + "guide.html"));
+}
+
 TEST_CASE("Plugin host API exposes model geometry and structure to Python", "[PluginHost][Python]")
 {
     using Catch::Matchers::WithinAbs;