diff --git a/src/libslic3r/Format/AMF.cpp b/src/libslic3r/Format/AMF.cpp index fa28d93f09..42583419b4 100644 --- a/src/libslic3r/Format/AMF.cpp +++ b/src/libslic3r/Format/AMF.cpp @@ -314,8 +314,13 @@ void AMFParserContext::startElement(const char *name, const char **atts) case 2: if (strcmp(name, "metadata") == 0) { if (m_path[1] == NODE_TYPE_MATERIAL || m_path[1] == NODE_TYPE_OBJECT) { - m_value[0] = get_attribute(atts, "type"); - node_type_new = NODE_TYPE_METADATA; + const char *type = get_attribute(atts, "type"); + if (type == nullptr) + this->stop(); + else { + m_value[0] = type; + node_type_new = NODE_TYPE_METADATA; + } } }/* else if (strcmp(name, "layer_config_ranges") == 0 && m_path[1] == NODE_TYPE_OBJECT) node_type_new = NODE_TYPE_LAYER_CONFIG;*/ diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index 61d5c48007..4ae316a055 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -1395,7 +1395,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) bool _handle_start_relationship(const char** attributes, unsigned int num_attributes); - void _generate_current_object_list(std::vector &sub_objects, Id object_id, IdToCurrentObjectMap& current_objects); + bool _generate_current_object_list(std::vector &sub_objects, Id object_id, IdToCurrentObjectMap& current_objects); bool _generate_volumes_new(ModelObject& object, const std::vector &sub_objects, const ObjectMetadata::VolumeMetadataList& volumes, ConfigSubstitutionContext& config_substitutions); //bool _generate_volumes(ModelObject& object, const Geometry& geometry, const ObjectMetadata::VolumeMetadataList& volumes, ConfigSubstitutionContext& config_substitutions); @@ -2117,7 +2117,8 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) return false; } std::vector object_id_list; - _generate_current_object_list(object_id_list, object.first, m_current_objects); + if (!_generate_current_object_list(object_id_list, object.first, m_current_objects)) + return false; ObjectMetadata::VolumeMetadataList volumes; ObjectMetadata::VolumeMetadataList* volumes_ptr = nullptr; @@ -2216,7 +2217,8 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) }*/ std::vector object_id_list; - _generate_current_object_list(object_id_list, object.first, m_current_objects); + if (!_generate_current_object_list(object_id_list, object.first, m_current_objects)) + return false; ObjectMetadata::VolumeMetadataList volumes; ObjectMetadata::VolumeMetadataList* volumes_ptr = nullptr; @@ -5071,11 +5073,18 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) return true; } - void _BBS_3MF_Importer::_generate_current_object_list(std::vector &sub_objects, Id object_id, IdToCurrentObjectMap ¤t_objects) + bool _BBS_3MF_Importer::_generate_current_object_list(std::vector &sub_objects, Id object_id, IdToCurrentObjectMap ¤t_objects) { + // A cycle in the component graph would expand forever, and an acyclic graph can still expand + // exponentially, so bound the number of component references queued. Checking before they are + // queued bounds the work list itself, whatever the fan-out. A valid file over the budget is + // rejected too, but the budget is way above the component references of any real object. + static constexpr size_t max_components = 100000; + std::list> id_list; id_list.push_back(std::make_pair(Component(object_id, Transform3d::Identity()), Transform3d::Identity())); + size_t num_components = 0; while (!id_list.empty()) { auto current_item = id_list.front(); @@ -5085,6 +5094,12 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) if (current_object != current_objects.end()) { //found one if (!current_object->second.components.empty()) { + num_components += current_object->second.components.size(); + if (num_components > max_components) { + add_error("invalid 3mf: cyclic or too many component references"); + sub_objects.clear(); + return false; + } for (const Component &comp : current_object->second.components) { id_list.push_back(std::pair(comp, current_item.second * comp.transform)); } @@ -5096,6 +5111,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) } } } + return true; } bool _BBS_3MF_Importer::_generate_volumes_new(ModelObject& object, const std::vector &sub_objects, const ObjectMetadata::VolumeMetadataList& volumes, ConfigSubstitutionContext& config_substitutions) diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index 2e9577e9c8..bd357ef59e 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -2345,7 +2345,7 @@ bool PresetCollection::reset_project_embedded_presets() return re_select; } -void PresetCollection::set_sync_info_and_save(std::string name, std::string setting_id, std::string syncinfo, long long update_time) +void PresetCollection::set_sync_info_and_save(std::string name, std::string setting_id, std::string syncinfo, long long update_time, const std::string& user_id) { lock(); const std::string canonical_name = this->canonical_preset_name(name); @@ -2363,7 +2363,10 @@ void PresetCollection::set_sync_info_and_save(std::string name, std::string sett preset2.save_info(); } } - preset->setting_id = setting_id; + if (!setting_id.empty()) + preset->setting_id = setting_id; + if (!user_id.empty()) + preset->user_id = user_id; if (update_time > 0) preset->updated_time = update_time; if (preset->sync_info == "update") @@ -2653,6 +2656,9 @@ bool PresetCollection::load_user_preset(std::string name, std::mapbase_id = based_id; iter->filament_id = cloud_filament_id; update_alias(*iter); + // Persist the cloud-assigned identity to disk, mirroring the equal/newer branch + // above; otherwise the id stays only in memory and the next launch rewrites it. + iter->save_info(); //presets_loaded.emplace_back(*it->second); BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << boost::format(", update the user preset %1% from cloud, type %2%, setting_id %3%, base_id %4%, sync_info %5% inherits %6%, filament_id %7%") % iter->name %Preset::get_type_string(m_type) %iter->setting_id %iter->base_id %iter->sync_info %iter->inherits() % iter->filament_id; diff --git a/src/libslic3r/Preset.hpp b/src/libslic3r/Preset.hpp index 2b1fb372af..f06c8f4224 100644 --- a/src/libslic3r/Preset.hpp +++ b/src/libslic3r/Preset.hpp @@ -603,7 +603,7 @@ public: void update_after_user_presets_loaded(); //BBS: get user presets int get_user_presets(PresetBundle *preset_bundle, std::vector &result_presets); - void set_sync_info_and_save(std::string name, std::string setting_id, std::string syncinfo, long long update_time); + void set_sync_info_and_save(std::string name, std::string setting_id, std::string syncinfo, long long update_time, const std::string& user_id); bool need_sync(std::string name, std::string setting_id, long long update_time); //BBS: add function to generate differed preset for save diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 23ae29d8d7..756775a4d9 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -7204,11 +7204,11 @@ void GUI_App::sync_preset(Preset* preset, bool force) BOOST_LOG_TRIVIAL(trace) << "sync_preset: sync operation: " << preset->sync_info << " success! preset = " << preset->name; if (preset->type == Preset::Type::TYPE_FILAMENT) { - preset_bundle->filaments.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time); + preset_bundle->filaments.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time, m_agent->get_user_id()); } else if (preset->type == Preset::Type::TYPE_PRINT) { - preset_bundle->prints.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time); + preset_bundle->prints.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time, m_agent->get_user_id()); } else if (preset->type == Preset::Type::TYPE_PRINTER) { - preset_bundle->printers.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time); + preset_bundle->printers.set_sync_info_and_save(preset->name, setting_id, updated_info, update_time, m_agent->get_user_id()); } } } @@ -7907,7 +7907,7 @@ void GUI_App::force_push_conflicting_preset(const std::string& setting_id) ? OrcaCloudServiceAgent::generate_uuid_for_setting_id(preset.name, user_id) : preset.setting_id; if (preset_id == setting_id) { - coll->set_sync_info_and_save(preset.name, setting_id, "update", 0); + coll->set_sync_info_and_save(preset.name, setting_id, "update", 0, user_id); break; } } diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index b24bfe42b4..84a6f40c53 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -15032,7 +15032,8 @@ bool Plater::priv::undo_redo_blocked_by_job() return false; notification_manager->push_notification(NotificationType::CustomNotification, NotificationManager::NotificationLevel::RegularNotificationLevel, - _u8L("Cannot undo or redo while an operation is running. Stop it first.")); + _u8L("Cannot undo or redo while an operation is running. Stop the operation, or wait " + "for it to finish and then retry.")); return true; } diff --git a/src/slic3r/Utils/OrcaCloudServiceAgent.cpp b/src/slic3r/Utils/OrcaCloudServiceAgent.cpp index c3e67afef1..7aa2b1ab2e 100644 --- a/src/slic3r/Utils/OrcaCloudServiceAgent.cpp +++ b/src/slic3r/Utils/OrcaCloudServiceAgent.cpp @@ -1129,6 +1129,19 @@ std::string OrcaCloudServiceAgent::request_setting_id(std::string name, if (http_code) *http_code = result.http_code; + // 409 duplicate_profile_uuid in the create path means the deterministic id we + // just generated already exists in this account: the earlier create succeeded. + // Adopt it instead of failing, so sync_preset persists the id and stops retrying. + if (result.http_code == 409 && result.conflict_code == -2 + && !result.server_version.id.empty() && result.server_version.id == new_id) { + if (values_map && result.server_version.updated_time != 0) + (*values_map)[IOT_JSON_KEY_UPDATED_TIME] = std::to_string(result.server_version.updated_time); + if (http_code) + *http_code = 200; + BOOST_LOG_TRIVIAL(info) << "OrcaCloudServiceAgent: request_setting_id adopted existing profile id " << new_id << " (409 duplicate_profile_uuid)"; + return new_id; + } + if (result.success) { if (values_map && result.new_updated_time != 0) { (*values_map)[IOT_JSON_KEY_UPDATED_TIME] = std::to_string(result.new_updated_time); @@ -1394,6 +1407,7 @@ SyncPushResult OrcaCloudServiceAgent::sync_push(const std::string& profile_id, SyncPushResult result; result.success = false; result.http_code = 0; + result.conflict_code = 0; result.server_deleted = false; nlohmann::json body; @@ -1429,20 +1443,30 @@ SyncPushResult OrcaCloudServiceAgent::sync_push(const std::string& profile_id, err_body = json; if (json.is_null()) { result.server_deleted = true; - } else { - auto& profile_data = json["server_profile"]; - result.server_version.id = profile_data.value("id", ""); - result.server_version.name = profile_data.value("name", ""); - result.server_version.updated_time = profile_data.value(ORCA_JSON_KEY_UPDATE_TIME, 0); + } else if (json.is_object()) { + result.conflict_code = json.value("code", 0); + if (json.contains("server_profile") && !json["server_profile"].is_null()) { + auto& profile_data = json["server_profile"]; + result.server_version.id = profile_data.value("id", ""); + result.server_version.name = profile_data.value("name", ""); + result.server_version.updated_time = profile_data.value(ORCA_JSON_KEY_UPDATE_TIME, 0); + } } } catch (...) {} - // Surface the conflict via the http-error callback with the local preset name injected. - // The raw server body omits the name for tombstone (-3) conflicts (server_profile is null), - // but the GUI needs it to regenerate the deterministic setting_id for a force push. - if (!err_body.is_object()) - err_body = nlohmann::json::object(); - err_body["name"] = name; - invoke_http_error_callback(409, err_body.dump()); + // Create-path duplicate_profile_uuid (-2) is an idempotent success: the deterministic id + // already exists, so the caller adopts the returned id. Skip the conflict notification, + // otherwise every already-imported preset would raise a Pull/Force-push prompt on each launch. + const bool is_create = original_updated_time.empty(); + const bool auto_resolved_duplicate = (is_create && result.conflict_code == -2); + if (!auto_resolved_duplicate) { + // Surface the conflict via the http-error callback with the local preset name injected. + // The raw server body omits the name for tombstone (-3) conflicts (server_profile is null), + // but the GUI needs it to regenerate the deterministic setting_id for a force push. + if (!err_body.is_object()) + err_body = nlohmann::json::object(); + err_body["name"] = name; + invoke_http_error_callback(409, err_body.dump()); + } result.error_message = response; return result; } diff --git a/src/slic3r/Utils/OrcaCloudServiceAgent.hpp b/src/slic3r/Utils/OrcaCloudServiceAgent.hpp index 458aa55fcd..d4a182db12 100644 --- a/src/slic3r/Utils/OrcaCloudServiceAgent.hpp +++ b/src/slic3r/Utils/OrcaCloudServiceAgent.hpp @@ -94,6 +94,7 @@ struct SyncPullResponse { struct SyncPushResult { bool success; int http_code; + int conflict_code; long long new_updated_time; ProfileUpsert server_version; bool server_deleted; diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index cf9a69e5d6..ef96e45445 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -3,6 +3,7 @@ get_filename_component(_TEST_NAME ${CMAKE_CURRENT_LIST_DIR} NAME) add_executable(${_TEST_NAME}_tests ${_TEST_NAME}_tests.cpp test_3mf.cpp + test_amf.cpp # Round-trip seam metadata and active/dormant volume settings in both formats. test_precise_seam_3mf.cpp # Pure perimeter extraction is independent of Print/Layer fixtures. diff --git a/tests/libslic3r/test_3mf.cpp b/tests/libslic3r/test_3mf.cpp index 9570cb3ecf..8439e8f846 100644 --- a/tests/libslic3r/test_3mf.cpp +++ b/tests/libslic3r/test_3mf.cpp @@ -45,6 +45,7 @@ #include #include // for std::enable_if_t #include // for typeid +#include #include #include @@ -415,6 +416,68 @@ TEST_CASE("A project with a plate id below 1 fails to load", "[3mf][Regression]" REQUIRE_FALSE(loaded); } +TEST_CASE("A project whose components reference themselves fails to load", "[3mf][Regression]") +{ + // One self-reference keeps the expansion going without ever reaching a mesh. A thousand also make + // each expansion queue a thousand more, so the bound has to hold the work list, not just the loop. + const int references = GENERATE(1, 1000); + INFO("self-references " << references); + + ScopedTemporaryFile temp(".3mf"); + store_painted_cube(temp.string()); + + // Point the component back at the object that holds it, repeated `references` times. + REQUIRE(rewrite_3mf_entries(temp.string(), [references](std::string& name, std::string& data) { + if (!boost::algorithm::ends_with(name, "3dmodel.model")) + return false; + std::smatch match; + if (!std::regex_search(data, match, std::regex("]*>\\s*]*/>"))) + return false; + std::string repeated; + for (int i = 0; i < references; ++i) + repeated += component.str(); + data.replace(component.position(), component.length(), repeated); + return true; + })); + + ScopedTemporaryDir backup_dir("orca_cycle_dst"); + Model model; + bool loaded = true; + REQUIRE_NOTHROW(loaded = load_project(temp.string(), model, backup_dir)); + REQUIRE_FALSE(loaded); +} + +TEST_CASE("An object loads up to the component reference budget and fails past it", "[3mf][Regression]") +{ + // The importer queues at most 100000 component references per object. Every reference besides the + // cube's own points at an object the file does not define: it counts toward the budget, then expands + // to nothing, so the object stays a single part whatever the count. + const auto [references, loads] = GENERATE(table({ { 100000, true }, { 100001, false } })); + INFO("component references " << references); + + ScopedTemporaryFile temp(".3mf"); + store_painted_cube(temp.string()); + + std::string dangling; + for (int i = 1; i < references; ++i) + dangling += ""; + REQUIRE(replace_in_3mf_entry(temp.string(), "3dmodel.model", "", dangling + "")); + + ScopedTemporaryDir backup_dir("orca_budget_dst"); + Model model; + bool loaded = !loads; + REQUIRE_NOTHROW(loaded = load_project(temp.string(), model, backup_dir)); + REQUIRE(loaded == loads); + if (loads) { + REQUIRE(model.objects.size() == 1); + CHECK(model.objects.front()->volumes.size() == 1); + } +} + TEST_CASE("A project with malformed paint data loads without the damaged facet", "[3mf][Regression]") { ScopedTemporaryFile temp(".3mf"); diff --git a/tests/libslic3r/test_amf.cpp b/tests/libslic3r/test_amf.cpp new file mode 100644 index 0000000000..eea678fc4a --- /dev/null +++ b/tests/libslic3r/test_amf.cpp @@ -0,0 +1,80 @@ +#include + +#include "libslic3r/Config.hpp" +#include "libslic3r/Format/AMF.hpp" +#include "libslic3r/Model.hpp" +#include "libslic3r/PrintConfig.hpp" + +#include "test_utils.hpp" + +#include + +#include +#include + +using namespace Slic3r; + +namespace { + +// The smallest AMF the loader accepts: one object holding one volume, a tetrahedron. The metadata +// element of the object is dropped in verbatim, so a test can hand the parser a malformed one. +std::string amf_with_object_metadata(const std::string &object_metadata) +{ + return "\n" + "\n" + " \n" + " " + object_metadata + "\n" + " \n" + " \n" + " 000\n" + " 100\n" + " 010\n" + " 001\n" + " \n" + " \n" + " 021\n" + " 013\n" + " 032\n" + " 123\n" + " \n" + " \n" + " \n" + "\n"; +} + +void write_file(const std::string &path, const std::string &content) +{ + boost::nowide::ofstream f(path, std::ios::binary); + f << content; +} + +bool load(const std::string &path, Model &model) +{ + DynamicPrintConfig config; + ConfigSubstitutionContext substitutions(ForwardCompatibilitySubstitutionRule::Disable); + return load_amf(path.c_str(), &config, &substitutions, &model, nullptr); +} + +} // namespace + +TEST_CASE("An AMF object metadata element with no type attribute is rejected", "[AMF]") +{ + ScopedTemporaryFile tmp(".amf"); + + SECTION("with the attribute the file loads") + { + write_file(tmp.string(), amf_with_object_metadata("tetra")); + + Model model; + REQUIRE(load(tmp.string(), model)); + CHECK(model.objects.size() == 1); + } + + SECTION("without it the load fails instead of reading a null attribute") + { + write_file(tmp.string(), amf_with_object_metadata("tetra")); + + Model model; + CHECK_FALSE(load(tmp.string(), model)); + } +}