juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-10-10 17:21:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Escape the IMEX plate attributes written into the 3MF

Browse Source 
Closes review comment 1.

imex_parallel_mode and imex_head_filament_map were streamed raw into XML attribute
values, while every other free-text attribute in the same writer goes through
xml_escape. Mode names are free text, so "PLA & ABS", a quote or a "<" made the
document malformed. The failure is not a bad value on reload: both load paths for
model_settings.config return false on an expat error, and m_is_bbl_3mf is set
before the second entry loop runs, so the whole project fails to open with
"Archive does not contain a valid model config".

Both attributes now use xml_escape_double_quotes_attribute_value(), which also
emits tab, CR and LF as numeric character references. That matters and plain
xml_escape would not do: XML normalises literal whitespace in attribute values on
read, so a tab in a mode name would come back as a space and silently rename the
mode. The read side needs no change -- it takes expat's already-decoded value with
no second unescape -- so this is a lossless round trip and a file written by the
new code still loads in an older build.

The round-trip test used "copy_mode", which exercised none of this; it now carries
&, <, a quote and a tab, and also pins that ' and > come back unmodified, since
both are legal raw inside a double-quoted value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford Garwood
2026-09-03 00:55:58 -04:00
co-authored by Claude Opus 5
parent 5629dd29e9
commit a5ba39393e
2 changed files with 31 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libslic3r/Format/bbs_3mf.cpp
+8 -2
View File
@@ -8105,14 +8105,20 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
stream << " <" << METADATA_TAG << " " << KEY_ATTR << "=\"" << SPIRAL_VASE_MODE << "\" " << VALUE_ATTR << "=\"" << spiral_mode_opt->getBool() << "\"/>\n";
{
// Mode names are user supplied free text, so the value has to be escaped for an
// attribute. xml_escape_double_quotes_attribute_value() is used rather than
// xml_escape() because it also emits tab/CR/LF as numeric character references:
// XML normalizes literal whitespace in attribute values on read, which would
// silently rename the mode. The reader takes the value straight from expat,
// which resolves both entities and character references, so this round-trips.
auto* imex_mode_opt = plate_data->config.option<ConfigOptionString>("imex_parallel_mode");
if (imex_mode_opt && !imex_mode_opt->value.empty() && imex_mode_opt->value != kImexPrimaryMode)
stream << " <" << METADATA_TAG << " " << KEY_ATTR << "=\"" << IMEX_PARALLEL_MODE_ATTR << "\" " << VALUE_ATTR << "=\"" << imex_mode_opt->value << "\"/>\n";
stream << " <" << METADATA_TAG << " " << KEY_ATTR << "=\"" << IMEX_PARALLEL_MODE_ATTR << "\" " << VALUE_ATTR << "=\"" << xml_escape_double_quotes_attribute_value(imex_mode_opt->value) << "\"/>\n";
}
{
auto* imex_hfm_opt = plate_data->config.option<ConfigOptionString>("imex_head_filament_map");
if (imex_hfm_opt && !imex_hfm_opt->value.empty())
stream << " <" << METADATA_TAG << " " << KEY_ATTR << "=\"" << IMEX_HEAD_FILAMENT_MAP_ATTR << "\" " << VALUE_ATTR << "=\"" << imex_hfm_opt->value << "\"/>\n";
stream << " <" << METADATA_TAG << " " << KEY_ATTR << "=\"" << IMEX_HEAD_FILAMENT_MAP_ATTR << "\" " << VALUE_ATTR << "=\"" << xml_escape_double_quotes_attribute_value(imex_hfm_opt->value) << "\"/>\n";
}
//filament map related
tests/libslic3r/test_3mf.cpp
+23 -6
View File
@@ -504,10 +504,27 @@ SCENARIO("BBS 3MF round-trips per-plate IMEX state (parallel mode + head filamen
// Regression guard for the class of bug where per-plate state silently drops through
// save/load (the variant-truncation bug was the precipitating example; IMEX plate state
// rides the same XML metadata path and is equally vulnerable).
//
// The mode name is user-typed free text, so the values below deliberately carry every
// character class that is special inside an XML attribute value:
// & and < must be escaped or the document is not well formed and the project will
// not load at all,
// " must be escaped or it terminates the attribute early,
// tab must be written as a numeric character reference, because XML normalizes
// literal whitespace in attribute values on read and the mode would be
// silently renamed,
// ' and > are legal raw inside a double-quoted value and must survive untouched.
// The head filament map value is machine generated in practice; it is given hostile
// content here only to pin the escaping of the sibling attribute, so this asserts the
// XML transport, not the map grammar (the loader stores the string verbatim).
//
// BBS exporter scaffolds a backup dir under temporary_dir() for the project config file;
// point it at a writable location for the test process.
set_temporary_dir(boost::filesystem::temp_directory_path().string());
const std::string hostile_mode = "PLA & ABS <hot> \"2x\"\tcopy's mode";
const std::string hostile_hfm = "1:2,2:3 & <\"\tx>";
GIVEN("A Model with a single object on plate 0 and IMEX plate state set") {
Model src_model;
std::string src_file = std::string(TEST_DATA_DIR) + "/test_3mf/Prusa.stl";
@@ -519,8 +536,8 @@ SCENARIO("BBS 3MF round-trips per-plate IMEX state (parallel mode + head filamen
PlateDataPtrs src_plates;
auto *plate0 = new PlateData();
plate0->plate_index = 0;
plate0->config.set_key_value("imex_parallel_mode", new ConfigOptionString("copy_mode"));
plate0->config.set_key_value("imex_head_filament_map", new ConfigOptionString("1:2,2:3"));
plate0->config.set_key_value("imex_parallel_mode", new ConfigOptionString(hostile_mode));
plate0->config.set_key_value("imex_head_filament_map", new ConfigOptionString(hostile_hfm));
src_plates.push_back(plate0);
WHEN("the model is saved to BBS 3MF and loaded back") {
@@ -551,17 +568,17 @@ SCENARIO("BBS 3MF round-trips per-plate IMEX state (parallel mode + head filamen
THEN("the loaded plate list has the same number of plates") {
REQUIRE(dst_plates.size() == src_plates.size());
}
THEN("imex_parallel_mode round-trips with its original value") {
THEN("imex_parallel_mode round-trips byte for byte, XML metacharacters included") {
REQUIRE(dst_plates.size() >= 1);
auto *mode_opt = dst_plates[0]->config.option<ConfigOptionString>("imex_parallel_mode");
REQUIRE(mode_opt != nullptr);
REQUIRE(mode_opt->value == "copy_mode");
REQUIRE(mode_opt->value == hostile_mode);
}
THEN("imex_head_filament_map round-trips with its original value") {
THEN("imex_head_filament_map round-trips byte for byte, XML metacharacters included") {
REQUIRE(dst_plates.size() >= 1);
auto *hfm_opt = dst_plates[0]->config.option<ConfigOptionString>("imex_head_filament_map");
REQUIRE(hfm_opt != nullptr);
REQUIRE(hfm_opt->value == "1:2,2:3");
REQUIRE(hfm_opt->value == hostile_hfm);
}
release_PlateData_list(dst_plates);