oox/source/export/drawingml.cxx | 65 ++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 31 deletions(-)
New commits: commit eb0f75e3140952179edf3be53000e67a8a20a11e Author: [email protected] <[email protected]> AuthorDate: Thu Jan 15 09:42:48 2026 -0500 Commit: Justin Luth <[email protected]> CommitDate: Fri Jan 16 01:38:47 2026 +0100 follow-up tdf#166335: concern about 'correctness' and cleanup This function has been full of coverity bugs and MS Office reporting the output as corrupt, so it seems appropriate to warn the code-reader that absolutely nothing about the function might actually be correct. bIsOOXML could have been more easily gotten by just extending aCustomShape2d to return m_bOOXMLShape, if it actually would eventually be needed. Change-Id: I4d74291c51ad46127a3121cb63607bf683ab4f9f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197392 Tested-by: Jenkins Reviewed-by: Justin Luth <[email protected]> diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx index 9253dead3c15..78622750c84c 100644 --- a/oox/source/export/drawingml.cxx +++ b/oox/source/export/drawingml.cxx @@ -4938,6 +4938,9 @@ void prepareTextArea(const EnhancedCustomShape2d& rEnhancedCustomShape2d, bool IsValidOOXMLFormula(std::u16string_view sFormula) { + // NOTE: this list is not complete - just a first effort at a reasonable list, + // and most of these are not yet even generated by GetFormula()... + // Accepted Formulas // "val n1" // "abs n1" @@ -4946,35 +4949,38 @@ bool IsValidOOXMLFormula(std::u16string_view sFormula) // "max n1 n2" // "*/ n1 n2 n3" // "+- n1 n2 n3" - // "?: n1 n2 n3" + // "+/ n1 n2 n3" + // "?: n1 n2 n3" // ifelse // Below vector contains validTokens for the 1st token based on the number of tokens in the formula. The order is: 2, 3, 4 const std::vector<std::set<OUString>> validTokens - = { { "val", "abs", "sqrt" }, { "min", "max" }, { "*/", "+-", "?:" } }; + = { { "val", "abs", "sqrt" }, { "min", "max" }, { "*/", "+-", "+/", "?:" } }; const std::set<OUString> builtInVariables = { "w", "h", "t", "b", "l", "r" }; const std::vector<OUString> strTokens = comphelper::string::split(sFormula, ' '); sal_uInt16 nSize = strTokens.size(); - if (nSize > 1 && nSize < 5) + if (nSize < 2 || nSize > 4) + return false; + + auto aTokens = validTokens[nSize - 2]; + + // Check whether the 1st token is valid + if (aTokens.find(strTokens[0]) == aTokens.end()) + return false; + + // Check that the remaining tokens are either numbers or built-in variables + for (sal_Int16 i = 1; i < nSize; i++) { - auto aTokens = validTokens[nSize - 2]; + OUString sVal = strTokens[i]; + if (builtInVariables.find(sVal) != builtInVariables.end()) + continue; // valid built-in variable - // Check whether the 1st token is valid - if (aTokens.find(strTokens[0]) == aTokens.end()) - return false; + // TODO: recognize valid 'guide name' pointing to other equations or adjustments - // Check that the remaining tokens are either numbers or built-in variables - for (sal_Int16 i = 1; i < nSize; i++) - { - OUString sVal = strTokens[i]; - sal_Int64 nVal = sVal.toInt64(); - if (builtInVariables.find(sVal) == builtInVariables.end() - && OUString::number(nVal) != sVal) - return false; - } - return true; + if (OUString::number(sVal.toInt64()) != sVal) + return false; // not a simple number } - return false; + return true; } OUString GetFormula(const OUString& sEquation) @@ -4983,6 +4989,9 @@ OUString GetFormula(const OUString& sEquation) // TODO: This needs to be completely re-written. It is extremely simplistic/minimal. // What is needed here is the reverse of convertToOOEquation. + // WARNING: Almost all of the current logic here was created simply to avoid 'corrupt document' + // notices from MS Office. The 'correct position' of the actual glue-points might be very wrong. + // If the equation is numerical sal_Int64 nValue = sEquation.toInt64(); if (!sEquation.isEmpty() && OUString::number(nValue) == sEquation) @@ -5027,7 +5036,7 @@ OUString GetFormula(const OUString& sEquation) void prepareGluePoints(std::vector<Guide>& rGuideList, const css::uno::Sequence<OUString>& aEquations, const uno::Sequence<drawing::EnhancedCustomShapeParameterPair>& rGluePoints, - const bool /*bIsOOXML*/, const sal_Int32 nWidth, const sal_Int32 nHeight) + const sal_Int32 nWidth, const sal_Int32 nHeight) { if (rGluePoints.hasElements()) { @@ -5065,7 +5074,12 @@ void prepareGluePoints(std::vector<Guide>& rGuideList, continue; } else + { + // nice TODO: avoid divide by zero, and simplify when multiplying by zero. + + // confirmation needed: does a simple gluepoint number always mean nIdx1 * w / width aGuideX.sFormula = "*/ " + OString::number(nIdx1) + " w " + OString::number(nWidth); + } if (bValidIdx2) { aGuideY.sFormula = GetFormula(aEquations[nIdx2]).toUtf8(); @@ -5119,17 +5133,6 @@ bool DrawingML::WriteCustomGeometry( uno::Sequence<beans::PropertyValue> aPathProp; pPathProp->Value >>= aPathProp; - auto pShapeType = std::find_if(std::cbegin(*pGeometrySeq), std::cend(*pGeometrySeq), - [](const PropertyValue& rProp) { return rProp.Name == "Type"; }); - bool bOOXML = false; - if (pShapeType != std::cend(*pGeometrySeq)) - { - OUString sShapeType; - pShapeType->Value >>= sShapeType; - if (sShapeType.startsWith("ooxml")) - bOOXML = true; - } - auto pEquationsProp = std::find_if(std::cbegin(*pGeometrySeq), std::cend(*pGeometrySeq), [](const PropertyValue& rProp) { return rProp.Name == "Equations"; }); @@ -5262,7 +5265,7 @@ bool DrawingML::WriteCustomGeometry( TextAreaRect aTextAreaRect; std::vector<Guide> aGuideList; // for now only for <a:rect> prepareTextArea(aCustomShape2d, aGuideList, aTextAreaRect); - prepareGluePoints(aGuideList, aEquationSeq, aGluePoints, bOOXML, nViewBoxWidth, nViewBoxHeight); + prepareGluePoints(aGuideList, aEquationSeq, aGluePoints, nViewBoxWidth, nViewBoxHeight); mpFS->startElementNS(XML_a, XML_custGeom); mpFS->singleElementNS(XML_a, XML_avLst); if (aGuideList.empty())
