get rid of the string copy and make the deep clone have a recursive limit

This commit is contained in:
Aaron Kimbrell
2025-10-06 22:25:05 -05:00
parent f25f49d42c
commit 74766fe6f9
2 changed files with 94 additions and 12 deletions

View File

@@ -38,11 +38,9 @@ Lxfml::Result Lxfml::NormalizePosition(const std::string_view data, const NiPoin
return toReturn;
}
// Ensure null-terminated string for tinyxml2::Parse
std::string nullTerminatedData(data);
tinyxml2::XMLDocument doc;
const auto err = doc.Parse(nullTerminatedData.c_str());
// Use length-based parsing to avoid expensive string copy
const auto err = doc.Parse(data.data(), data.size());
if (err != tinyxml2::XML_SUCCESS) {
return toReturn;
}
@@ -170,8 +168,9 @@ Lxfml::Result Lxfml::NormalizePosition(const std::string_view data, const NiPoin
}
// Deep-clone an XMLElement (attributes, text, and child elements) into a target document
static tinyxml2::XMLElement* CloneElementDeep(const tinyxml2::XMLElement* src, tinyxml2::XMLDocument& dstDoc) {
if (!src) return nullptr;
// with maximum depth protection to prevent infinite loops
static tinyxml2::XMLElement* CloneElementDeep(const tinyxml2::XMLElement* src, tinyxml2::XMLDocument& dstDoc, int maxDepth = 100) {
if (!src || maxDepth <= 0) return nullptr;
auto* dst = dstDoc.NewElement(src->Name());
// copy attributes
@@ -182,7 +181,9 @@ static tinyxml2::XMLElement* CloneElementDeep(const tinyxml2::XMLElement* src, t
// copy children (elements and text)
for (const tinyxml2::XMLNode* child = src->FirstChild(); child; child = child->NextSibling()) {
if (const tinyxml2::XMLElement* childElem = child->ToElement()) {
dst->InsertEndChild(CloneElementDeep(childElem, dstDoc));
// Recursively clone child elements with decremented depth
auto* clonedChild = CloneElementDeep(childElem, dstDoc, maxDepth - 1);
if (clonedChild) dst->InsertEndChild(clonedChild);
} else if (const tinyxml2::XMLText* txt = child->ToText()) {
auto* n = dstDoc.NewText(txt->Value());
dst->InsertEndChild(n);
@@ -208,12 +209,9 @@ std::vector<Lxfml::Result> Lxfml::Split(const std::string_view data, const NiPoi
return results;
}
// Ensure null-terminated string for tinyxml2::Parse
// string_view::data() may not be null-terminated, causing undefined behavior
std::string nullTerminatedData(data);
tinyxml2::XMLDocument doc;
const auto err = doc.Parse(nullTerminatedData.c_str());
// Use length-based parsing to avoid expensive string copy
const auto err = doc.Parse(data.data(), data.size());
if (err != tinyxml2::XML_SUCCESS) {
return results;
}

View File

@@ -296,3 +296,87 @@ TEST(LxfmlTests, MixedValidInvalidTransformsHandling) {
}) << "Mixed valid/invalid transform processing should not cause fatal errors";
}
}
TEST(LxfmlTests, DeepCloneDepthProtection) {
// Test that deep cloning has protection against excessive nesting
// Create a deeply nested XML structure that would exceed reasonable limits
std::string deeplyNestedLxfml = R"(<?xml version="1.0" encoding="UTF-8"?>
<LXFML versionMajor="5" versionMinor="0">
<Meta>
<Application name="LEGO Universe" versionMajor="0" versionMinor="0"/>
<Brand name="LEGOUniverse"/>
<BrickSet version="457"/>
</Meta>
<Bricks>
<Brick refID="0" designID="3001">
<Part refID="0" designID="3001" materials="23">
<Bone refID="0" transformation="1,0,0,0,1,0,0,0,1,0,0,0"/>
</Part>
</Brick>
</Bricks>
<RigidSystems>
</RigidSystems>
<GroupSystems>
<GroupSystem>
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0">
<Group partRefs="0"/>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</Group>
</GroupSystem>
</GroupSystems>
</LXFML>)";
// The Split function should handle deeply nested structures without hanging
std::vector<Lxfml::Result> results;
EXPECT_NO_FATAL_FAILURE({
results = Lxfml::Split(deeplyNestedLxfml);
}) << "Split should not hang or crash on deeply nested XML structures";
// Should still produce valid output despite depth limitations
EXPECT_GT(results.size(), 0) << "Should produce at least one result even with deep nesting";
if (results.size() > 0) {
// Verify the result is still valid XML
tinyxml2::XMLDocument doc;
auto parseResult = doc.Parse(results[0].lxfml.c_str());
EXPECT_EQ(parseResult, tinyxml2::XML_SUCCESS) << "Result should still be valid XML";
if (parseResult == tinyxml2::XML_SUCCESS) {
auto* lxfml = doc.FirstChildElement("LXFML");
EXPECT_NE(lxfml, nullptr) << "Result should have LXFML root element";
// Verify that bricks are still included despite group nesting issues
auto* bricks = lxfml->FirstChildElement("Bricks");
EXPECT_NE(bricks, nullptr) << "Bricks element should be present";
if (bricks) {
auto* brick = bricks->FirstChildElement("Brick");
EXPECT_NE(brick, nullptr) << "At least one brick should be present";
}
}
}
}