From f3acc52ceb8de2306440dc7c3999d47cba01efd1 Mon Sep 17 00:00:00 2001 From: Jason Millard Date: Thu, 28 Aug 2025 18:38:34 -0400 Subject: [PATCH] misc: fix ShapeDefinition XML parsing segfault on Linux --- .../bitmapshapes/ShapeDefinitions.cpp | 88 +++++++++++++++++-- src/fx/matrixfx/bitmapshapes/ShapeList.cpp | 84 ++++++++++++------ 2 files changed, 140 insertions(+), 32 deletions(-) diff --git a/src/fx/matrixfx/bitmapshapes/ShapeDefinitions.cpp b/src/fx/matrixfx/bitmapshapes/ShapeDefinitions.cpp index a1743b2..9299ad8 100644 --- a/src/fx/matrixfx/bitmapshapes/ShapeDefinitions.cpp +++ b/src/fx/matrixfx/bitmapshapes/ShapeDefinitions.cpp @@ -73,14 +73,27 @@ ShapeDefinitions* ShapeDefinitions::GetShapeDefinitionsFromShapeDefinitionsXmlFi try { xml = FileReader::ReadFileToString(fileName); + if (xml.empty()) + { + Log::Warning(StringExtensions::Build("ShapeDefinitions file {0} is empty.", fileName)); + return nullptr; + } } catch (const std::exception& e) { - Log::Exception(StringExtensions::Build("Could not load ShapeDefinitions from {0}.", fileName)); + Log::Exception(StringExtensions::Build("Could not load ShapeDefinitions from {0}. Error: {1}", fileName, e.what())); throw std::runtime_error(StringExtensions::Build("Could not read ShapeDefinitions file {0}.", fileName)); } - return GetShapeDefinitionsFromShapeDefinitionsXml(xml); + try + { + return GetShapeDefinitionsFromShapeDefinitionsXml(xml); + } + catch (const std::exception& e) + { + Log::Exception(StringExtensions::Build("Could not parse ShapeDefinitions XML from {0}. Error: {1}", fileName, e.what())); + return nullptr; + } } bool ShapeDefinitions::TestShapeDefinitionsShapeDefinitionsXmlFile(const std::string& fileName) @@ -105,13 +118,14 @@ ShapeDefinitions* ShapeDefinitions::GetShapeDefinitionsFromShapeDefinitionsXml(c tinyxml2::XMLError result = doc.Parse(shapeDefinitionsXml.c_str()); if (result != tinyxml2::XML_SUCCESS) { - Log::Exception("Could not deserialize the ShapeDefinitions from XML data."); + Log::Exception(StringExtensions::Build("Could not deserialize the ShapeDefinitions from XML data. Error: {0}", doc.ErrorStr() ? doc.ErrorStr() : "unknown error")); throw std::runtime_error("Could not deserialize the ShapeDefinitions from XML data."); } tinyxml2::XMLElement* root = doc.FirstChildElement("ShapeDefinitions"); if (!root) { + Log::Exception("Could not find ShapeDefinitions root element in XML"); throw std::runtime_error("Could not find ShapeDefinitions root element."); } @@ -120,10 +134,70 @@ ShapeDefinitions* ShapeDefinitions::GetShapeDefinitionsFromShapeDefinitionsXml(c tinyxml2::XMLElement* shapesElement = root->FirstChildElement("Shapes"); if (shapesElement) { - tinyxml2::XMLPrinter printer; - shapesElement->Accept(&printer); - std::string shapesXml = printer.CStr(); - shapeDefinitions->m_shapes.ReadXml(shapesXml); + try + { + for (tinyxml2::XMLElement* shapeElement = shapesElement->FirstChildElement(); shapeElement != nullptr; shapeElement = shapeElement->NextSiblingElement()) + { + std::string elementName = shapeElement->Name() ? shapeElement->Name() : ""; + + if (elementName == "Shape") + { + Shape* shape = new Shape(); + bool shapeValid = true; + + try + { + tinyxml2::XMLElement* nameElement = shapeElement->FirstChildElement("Name"); + if (nameElement && nameElement->GetText()) + shape->SetName(nameElement->GetText()); + + tinyxml2::XMLElement* frameElement = shapeElement->FirstChildElement("BitmapFrameNumber"); + if (frameElement) + shape->SetBitmapFrameNumber(frameElement->IntText(0)); + + tinyxml2::XMLElement* topElement = shapeElement->FirstChildElement("BitmapTop"); + if (topElement) + shape->SetBitmapTop(topElement->IntText(0)); + + tinyxml2::XMLElement* leftElement = shapeElement->FirstChildElement("BitmapLeft"); + if (leftElement) + shape->SetBitmapLeft(leftElement->IntText(0)); + + tinyxml2::XMLElement* widthElement = shapeElement->FirstChildElement("BitmapWidth"); + if (widthElement) + shape->SetBitmapWidth(widthElement->IntText(0)); + + tinyxml2::XMLElement* heightElement = shapeElement->FirstChildElement("BitmapHeight"); + if (heightElement) + shape->SetBitmapHeight(heightElement->IntText(0)); + + if (!shape->GetName().empty() && !shapeDefinitions->m_shapes.Contains(shape->GetName())) + { + shapeDefinitions->m_shapes.Add(shape); + } + else + { + shapeValid = false; + } + } + catch (const std::exception& e) + { + Log::Warning(StringExtensions::Build("Error processing shape: {0}", e.what())); + shapeValid = false; + } + + if (!shapeValid) + { + delete shape; + } + } + } + } + catch (const std::exception& e) + { + Log::Exception(StringExtensions::Build("Exception during shape processing: {0}", e.what())); + return shapeDefinitions; + } } return shapeDefinitions; diff --git a/src/fx/matrixfx/bitmapshapes/ShapeList.cpp b/src/fx/matrixfx/bitmapshapes/ShapeList.cpp index 5f2d881..b7db9a0 100644 --- a/src/fx/matrixfx/bitmapshapes/ShapeList.cpp +++ b/src/fx/matrixfx/bitmapshapes/ShapeList.cpp @@ -31,14 +31,26 @@ void ShapeList::WriteXml(std::string& writer) void ShapeList::ReadXml(const std::string& reader) { + if (reader.empty()) + { + Log::Warning("ShapeList::ReadXml - Empty XML string provided"); + return; + } + tinyxml2::XMLDocument doc; tinyxml2::XMLError result = doc.Parse(reader.c_str()); if (result != tinyxml2::XML_SUCCESS) + { + Log::Warning(StringExtensions::Build("ShapeList::ReadXml - XML parse error: {0}", doc.ErrorStr() ? doc.ErrorStr() : "unknown error")); return; + } tinyxml2::XMLElement* root = doc.FirstChildElement(); if (!root) + { + Log::Warning("ShapeList::ReadXml - No root element found"); return; + } for (tinyxml2::XMLElement* element = root->FirstChildElement(); element != nullptr; element = element->NextSiblingElement()) { @@ -47,34 +59,56 @@ void ShapeList::ReadXml(const std::string& reader) if (t == "Shape") { Shape* shape = new Shape(); + bool shapeValid = true; - tinyxml2::XMLElement* nameElement = element->FirstChildElement("Name"); - if (nameElement) - shape->SetName(nameElement->GetText() ? nameElement->GetText() : ""); - - tinyxml2::XMLElement* frameElement = element->FirstChildElement("BitmapFrameNumber"); - if (frameElement) - shape->SetBitmapFrameNumber(frameElement->IntText(0)); - - tinyxml2::XMLElement* topElement = element->FirstChildElement("BitmapTop"); - if (topElement) - shape->SetBitmapTop(topElement->IntText(0)); - - tinyxml2::XMLElement* leftElement = element->FirstChildElement("BitmapLeft"); - if (leftElement) - shape->SetBitmapLeft(leftElement->IntText(0)); - - tinyxml2::XMLElement* widthElement = element->FirstChildElement("BitmapWidth"); - if (widthElement) - shape->SetBitmapWidth(widthElement->IntText(0)); - - tinyxml2::XMLElement* heightElement = element->FirstChildElement("BitmapHeight"); - if (heightElement) - shape->SetBitmapHeight(heightElement->IntText(0)); + try + { + tinyxml2::XMLElement* nameElement = element->FirstChildElement("Name"); + if (nameElement) + shape->SetName(nameElement->GetText() ? nameElement->GetText() : ""); + + tinyxml2::XMLElement* frameElement = element->FirstChildElement("BitmapFrameNumber"); + if (frameElement) + shape->SetBitmapFrameNumber(frameElement->IntText(0)); + + tinyxml2::XMLElement* topElement = element->FirstChildElement("BitmapTop"); + if (topElement) + shape->SetBitmapTop(topElement->IntText(0)); + + tinyxml2::XMLElement* leftElement = element->FirstChildElement("BitmapLeft"); + if (leftElement) + shape->SetBitmapLeft(leftElement->IntText(0)); + + tinyxml2::XMLElement* widthElement = element->FirstChildElement("BitmapWidth"); + if (widthElement) + shape->SetBitmapWidth(widthElement->IntText(0)); + + tinyxml2::XMLElement* heightElement = element->FirstChildElement("BitmapHeight"); + if (heightElement) + shape->SetBitmapHeight(heightElement->IntText(0)); + + if (shape->GetName().empty()) + { + shapeValid = false; + } + else if (!this->Contains(shape->GetName())) + { + this->Add(shape); + } + else + { + shapeValid = false; + } + } + catch (const std::exception& e) + { + Log::Warning(StringExtensions::Build("Error processing shape: {0}", e.what())); + shapeValid = false; + } - if (!this->Contains(shape->GetName())) + if (!shapeValid) { - this->Add(shape); + delete shape; } } else