diff --git a/src/Base/XMLTools.cpp b/src/Base/XMLTools.cpp index 0d0e03497f..ae2b94b268 100644 --- a/src/Base/XMLTools.cpp +++ b/src/Base/XMLTools.cpp @@ -117,6 +117,44 @@ std::basic_string XMLTools::toXMLString(const char* const fromTranscode) return str; } +/*! + * \brief Escape special XML characters in a string. + * + * Replaces XML special characters (&, <, >, ", ') with their entity equivalents + * (&, <, >, ", '). + * + * \param input The string to escape + * \return The escaped string safe for use in XML content or attributes + */ +std::string XMLTools::escapeXml(const std::string& input) +{ + std::string output; + output.reserve(input.size()); + for (char ch : input) { + switch (ch) { + case '&': + output.append("&"); + break; + case '<': + output.append("<"); + break; + case '>': + output.append(">"); + break; + case '"': + output.append("""); + break; + case '\'': + output.append("'"); + break; + default: + output.push_back(ch); + break; + } + } + return output; +} + void XMLTools::terminate() { transcoder.reset(); diff --git a/src/Base/XMLTools.h b/src/Base/XMLTools.h index 14fadabbac..5cb77f8a4c 100644 --- a/src/Base/XMLTools.h +++ b/src/Base/XMLTools.h @@ -47,6 +47,7 @@ class BaseExport XMLTools public: static std::string toStdString(const XMLCh* const toTranscode); static std::basic_string toXMLString(const char* const fromTranscode); + static std::string escapeXml(const std::string& input); static void initialize(); static void terminate(); diff --git a/src/Mod/Mesh/App/Core/IO/Writer3MF.cpp b/src/Mod/Mesh/App/Core/IO/Writer3MF.cpp index 4e62fb3eab..e4218e201f 100644 --- a/src/Mod/Mesh/App/Core/IO/Writer3MF.cpp +++ b/src/Mod/Mesh/App/Core/IO/Writer3MF.cpp @@ -29,6 +29,7 @@ #include "Core/Evaluation.h" #include "Core/MeshKernel.h" #include +#include #include "Writer3MF.h" @@ -74,11 +75,11 @@ void Writer3MF::Finish(std::ostream& str) str << "\n"; } -bool Writer3MF::AddMesh(const MeshKernel& mesh, const Base::Matrix4D& mat) +bool Writer3MF::AddMesh(const MeshKernel& mesh, const Base::Matrix4D& mat, const std::string& name) { int id = ++objectIndex; SaveBuildItem(id, mat); - return SaveObject(zip, id, mesh); + return SaveObject(zip, id, mesh, name); } void Writer3MF::AddResource(const Resource3MF& res) @@ -111,7 +112,7 @@ bool Writer3MF::Save() return true; } -bool Writer3MF::SaveObject(std::ostream& str, int id, const MeshKernel& mesh) const +bool Writer3MF::SaveObject(std::ostream& str, int id, const MeshKernel& mesh, const std::string& name) const { // NOLINTBEGIN(readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers) const MeshPointArray& rPoints = mesh.GetPoints(); @@ -121,7 +122,11 @@ bool Writer3MF::SaveObject(std::ostream& str, int id, const MeshKernel& mesh) co return false; } - str << Base::blanks(2) << "\n"; + str << Base::blanks(2) << "\n"; str << Base::blanks(3) << "\n"; // vertices diff --git a/src/Mod/Mesh/App/Core/IO/Writer3MF.h b/src/Mod/Mesh/App/Core/IO/Writer3MF.h index d8d90c1743..18ecc1c23a 100644 --- a/src/Mod/Mesh/App/Core/IO/Writer3MF.h +++ b/src/Mod/Mesh/App/Core/IO/Writer3MF.h @@ -76,9 +76,10 @@ public: * \brief Add a mesh object resource to the 3MF file. * \param mesh The mesh object to be written * \param mat The placement of the mesh object + * \param name The name of the mesh object which can later be displayed by downstream applications * \return true if the added mesh could be written successfully, false otherwise. */ - bool AddMesh(const MeshKernel& mesh, const Base::Matrix4D& mat); + bool AddMesh(const MeshKernel& mesh, const Base::Matrix4D& mat, const std::string& name = ""); /*! * \brief AddResource * Add an additional resource to the 3MF file. @@ -97,7 +98,7 @@ private: std::string GetType(const MeshKernel& mesh) const; void SaveBuildItem(int id, const Base::Matrix4D& mat); static std::string DumpMatrix(const Base::Matrix4D& mat); - bool SaveObject(std::ostream& str, int id, const MeshKernel& mesh) const; + bool SaveObject(std::ostream& str, int id, const MeshKernel& mesh, const std::string& name) const; bool SaveRels(std::ostream& str) const; bool SaveContent(std::ostream& str) const; diff --git a/src/Mod/Mesh/App/Core/MeshIO.cpp b/src/Mod/Mesh/App/Core/MeshIO.cpp index 5b02cdaeae..ef6f2e0a6d 100644 --- a/src/Mod/Mesh/App/Core/MeshIO.cpp +++ b/src/Mod/Mesh/App/Core/MeshIO.cpp @@ -2193,7 +2193,7 @@ void MeshOutput::SaveXML(Base::Writer& writer) const bool MeshOutput::Save3MF(std::ostream& output) const { Writer3MF writer(output); - writer.AddMesh(_rclMesh, _transform); + writer.AddMesh(_rclMesh, _transform, objectName); return writer.Save(); } diff --git a/src/Mod/Mesh/App/Exporter.cpp b/src/Mod/Mesh/App/Exporter.cpp index e4e90613bb..c510d4151b 100644 --- a/src/Mod/Mesh/App/Exporter.cpp +++ b/src/Mod/Mesh/App/Exporter.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include "Core/Iterator.h" #include "Core/IO/Writer3MF.h" #include @@ -91,17 +92,6 @@ static std::vector expandSubObjectNames( Exporter::Exporter() = default; // static -std::string Exporter::xmlEscape(const std::string& input) -{ - std::string out(input); - boost::replace_all(out, "&", "&"); - boost::replace_all(out, "\"", """); - boost::replace_all(out, "'", "'"); - boost::replace_all(out, "<", "<"); - boost::replace_all(out, ">", ">"); - return out; -} - int Exporter::addObject(App::DocumentObject* obj, float tol) { int count = 0; @@ -304,8 +294,7 @@ Exporter3MF::~Exporter3MF() bool Exporter3MF::addMesh(const char* name, const MeshObject& mesh) { - boost::ignore_unused(name); - bool ok = d->writer3mf.AddMesh(mesh.getKernel(), mesh.getTransform()); + bool ok = d->writer3mf.AddMesh(mesh.getKernel(), mesh.getTransform(), name); if (ok) { for (const auto& it : d->ext) { d->writer3mf.AddResource(it->addMesh(mesh)); @@ -421,7 +410,8 @@ bool ExporterAMF::addMesh(const char* name, const MeshObject& mesh) Base::SequencerLauncher seq("Saving...", 2 * numFacets + 1); *outputStreamPtr << "\t\n"; - *outputStreamPtr << "\t\t" << xmlEscape(name) << "\n"; + *outputStreamPtr << "\t\t" << XMLTools::escapeXml(name) + << "\n"; *outputStreamPtr << "\t\t\n" << "\t\t\t\n"; diff --git a/src/Mod/Mesh/App/Exporter.h b/src/Mod/Mesh/App/Exporter.h index 17dbc1ac33..5228c6c140 100644 --- a/src/Mod/Mesh/App/Exporter.h +++ b/src/Mod/Mesh/App/Exporter.h @@ -69,8 +69,6 @@ public: Exporter& operator=(Exporter&&) = delete; protected: - /// Does some simple escaping of characters for XML-type exports - static std::string xmlEscape(const std::string& input); void throwIfNoPermission(const std::string&); std::map> subObjectNameCache; diff --git a/tests/src/Base/CMakeLists.txt b/tests/src/Base/CMakeLists.txt index b3f31b71ab..3bcce0c585 100644 --- a/tests/src/Base/CMakeLists.txt +++ b/tests/src/Base/CMakeLists.txt @@ -29,6 +29,7 @@ add_executable(Base_tests_run Vector3D.cpp ViewProj.cpp Writer.cpp + XMLTools.cpp ) setup_qt_test(InventorBuilder) diff --git a/tests/src/Base/XMLTools.cpp b/tests/src/Base/XMLTools.cpp new file mode 100644 index 0000000000..38b4efb5a6 --- /dev/null +++ b/tests/src/Base/XMLTools.cpp @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include +#include + +class TestXMLTools: public testing::Test +{ +}; + + +TEST_F(TestXMLTools, EscapeXmlEmptyString) +{ + EXPECT_EQ(XMLTools::escapeXml(""), ""); +} + +TEST_F(TestXMLTools, EscapeXmlNoChangesForSafeText) +{ + EXPECT_EQ(XMLTools::escapeXml("abcXYZ_123"), "abcXYZ_123"); + EXPECT_EQ(XMLTools::escapeXml("hello world"), "hello world"); + EXPECT_EQ(XMLTools::escapeXml("line1\nline2\tend"), "line1\nline2\tend"); +} + +TEST_F(TestXMLTools, EscapeXmlEscapesAmpersand) +{ + EXPECT_EQ(XMLTools::escapeXml("&"), "&"); + EXPECT_EQ(XMLTools::escapeXml("a&b"), "a&b"); +} + +TEST_F(TestXMLTools, EscapeXmlEscapesLessThanAndGreaterThan) +{ + EXPECT_EQ(XMLTools::escapeXml("<"), "<"); + EXPECT_EQ(XMLTools::escapeXml(">"), ">"); + EXPECT_EQ(XMLTools::escapeXml("ab"), "a>b"); + EXPECT_EQ(XMLTools::escapeXml("ac"), "a<b>c"); +} + +TEST_F(TestXMLTools, EscapeXmlEscapesQuotes) +{ + EXPECT_EQ(XMLTools::escapeXml("\""), """); + EXPECT_EQ(XMLTools::escapeXml("'"), "'"); + EXPECT_EQ(XMLTools::escapeXml("a\"b"), "a"b"); + EXPECT_EQ(XMLTools::escapeXml("a'b"), "a'b"); +} + +TEST_F(TestXMLTools, EscapeXmlEscapesAllFiveInOneString) +{ + // input: & < > " ' + EXPECT_EQ(XMLTools::escapeXml("&<>\"'"), "&<>"'"); +} + +TEST_F(TestXMLTools, EscapeXmlDoesNotDoubleEscapeNewlyInsertedEntities) +{ + // This test specifically catches the classic bug where you replace + // '<' with "<" and then later replace '&' with "&" and end up with "&lt;". + EXPECT_EQ(XMLTools::escapeXml("<"), "<"); + EXPECT_EQ(XMLTools::escapeXml(">"), ">"); + EXPECT_EQ(XMLTools::escapeXml("\""), """); + EXPECT_EQ(XMLTools::escapeXml("'"), "'"); +} + +TEST_F(TestXMLTools, EscapeXmlComplexMixedContent) +{ + const std::string in = "Tom & Jerry <\"fun\"> 'n' games"; + const std::string out = "Tom & Jerry <"fun"> 'n' games"; + EXPECT_EQ(XMLTools::escapeXml(in), out); +} + +TEST_F(TestXMLTools, EscapeXmlMultipleAdjacentCharacters) +{ + EXPECT_EQ(XMLTools::escapeXml("&&&&"), "&&&&"); + EXPECT_EQ(XMLTools::escapeXml("<<<<"), "<<<<"); + EXPECT_EQ(XMLTools::escapeXml("\"\"''"), """''"); +} + +TEST_F(TestXMLTools, EscapeXmlPreservesWhitespaceAndControlCommonInText) +{ + EXPECT_EQ(XMLTools::escapeXml(" a \r\n b \t c "), " a \r\n b \t c "); +}