ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

follow-up tdf#166335: concern about 'correctness' and cleanup

Browse Source 
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 <jluth@mail.com>
This commit is contained in:
justin.luth@collabora.com
2026-01-16 01:38:47 +01:00
committed by Justin Luth
parent 777503f2a0
commit eb0f75e314
1 changed files with 34 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
oox/source/export/drawingml.cxx
+34 -31
View File
@@ -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())