[Libreoffice-commits] core.git: starmath/source

[Stephan Bergmann (via logerrit)]

 starmath/source/mathml/mathmlattr.cxx |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

New commits:
commit 1d4adcee1ba82cb8f02ac73be3e558d10dea8eda
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Thu Sep 16 11:23:03 2021 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Sep 16 13:00:40 2021 +0200

    Increase accuracy of ParseMathMLUnsignedNumber Fraction result
    
    The new test from d5e55d204b71710eb5eb5d2c683dd6698626df3c "tdf#144319:
    sw_odfexport: Add unittest" started to cause UBSan failure during
    CppunitTest_sw_odfexport (<https://ci.libreoffice.org/job/lo_ubsan/2136/>),
    
    > /workdir/UnpackedTarball/boost/boost/rational.hpp:605:22: runtime error: signed integer overflow: 1073741824 * 2 cannot be represented in type 'int'
    >     #0 0x2b450983594f in boost::rational<int>::operator/=(boost::rational<int> const&) /workdir/UnpackedTarball/boost/boost/rational.hpp:605:22
    >     #1 0x2b450982e216 in Fraction::operator/=(Fraction const&) /tools/source/generic/fract.cxx:224:7
    >     #2 0x2b45098312d3 in operator/(Fraction const&, Fraction const&) /tools/source/generic/fract.cxx:320:10
    >     #3 0x2b46066963d5 in (anonymous namespace)::lcl_CountBlanks(MathMLAttributeLengthValue const&, int*, int*) /starmath/source/mathml/mathmlimport.cxx:1497:30
    >     #4 0x2b46066943eb in (anonymous namespace)::SmXMLSpaceContext_Impl::startFastElement(int, com::sun::star::uno::Reference<com::sun::star::xml::sax::XFastAttributeList> const&) /starmath/source/mathml/mathmlimport.cxx:1526:25
    [...]
    
    Formula-14/content.xml in sw/qa/extras/odfexport/data/tdf144319.odt contains
    width="0.444em", which resulted in the inaccurate Fraction 476741369/1073741824
    since 67d83e40e2c4f3862c50e6abeabfc24a75119fc8 "speedup rational_FromDouble"
    (which computes dVal=4.76741e+08 and nDen=1073741824; where before that speedup
    it computed dVal=4.44e+08 and nDen=1000000000, which would have resulted in the
    accurate Fraction 111/250).  The excessively large denominator (1073741824 =
    std::numeric_limits<sal_Int32>::max()/2 + 1) then caused overflow later on, as
    shown above.
    
    Change-Id: I221549528e4fa155e10697d218ec76bf678e8b2f
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122186
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/starmath/source/mathml/mathmlattr.cxx b/starmath/source/mathml/mathmlattr.cxx
index 008f8e2a81f6..7da7f947a8c6 100644
--- a/starmath/source/mathml/mathmlattr.cxx
+++ b/starmath/source/mathml/mathmlattr.cxx
@@ -9,6 +9,8 @@
 
 #include <mathmlattr.hxx>
 
+#include <o3tl/safeint.hxx>
+
 #include <unordered_map>
 
 static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN)
@@ -16,6 +18,9 @@ static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN)
     auto nLen = rStr.getLength();
     sal_Int32 nDecimalPoint = -1;
     sal_Int32 nIdx;
+    sal_Int64 nom = 0;
+    sal_Int64 den = 1;
+    bool validNomDen = true;
     for (nIdx = 0; nIdx < nLen; nIdx++)
     {
         auto cD = rStr[nIdx];
@@ -28,11 +33,29 @@ static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN)
         }
         if (cD < u'0' || u'9' < cD)
             break;
+        if (validNomDen
+            && (o3tl::checked_multiply(nom, sal_Int64(10), nom)
+                || o3tl::checked_add(nom, sal_Int64(cD - u'0'), nom)
+                || (nDecimalPoint >= 0 && o3tl::checked_multiply(den, sal_Int64(10), den))))
+        {
+            validNomDen = false;
+        }
     }
     if (nIdx == 0 || (nIdx == 1 && nDecimalPoint == 0))
         return -1;
 
-    rUN = Fraction(rStr.copy(0, nIdx).toDouble());
+    // If the input "xx.yyy" can be represented with nom = xx*10^n + yyy and den = 10^n in sal_Int64
+    // (where n is the length of "yyy"), then use that to create an accurate Fraction (and TODO: we
+    // could even ignore trailing "0" characters in "yyy", for a smaller n and thus a greater chance
+    // of validNomDen); if not, use the less accurate approach of creating a Fraction from double:
+    if (validNomDen)
+    {
+        rUN = Fraction(nom, den);
+    }
+    else
+    {
+        rUN = Fraction(rStr.copy(0, nIdx).toDouble());
+    }
 
     return nIdx;
 }