tdf#98646 Fixed freeze by flattening loops

DrawViewShell::MakeVisible was using loops for finding the area
to change the visible part of the EditView to. That could (and did)
potentially loop for a long time for very large objects, deep zoom or
numerical problems. That loops were flattened, the results checked
to be the same.
Also added a test for numerical overflow for getting values from the
Rectangle describing the object size. Despite these values being
the result of erraneous import, I opt for checking and avoiding using
these values.

Change-Id: I783dc1f2ad9b6a60a47e660b0d576ea3f22a4e42
Reviewed-on: https://gerrit.libreoffice.org/23278
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Björn Michaelsen <bjoern.michaelsen@canonical.com>
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>

1 files changed, 59 insertions, 9 deletions

diff --git a/sd/source/ui/view/drviewsh.cxx b/sd/source/ui/view/drviewsh.cxx
index a4a1e434cc6a..c2ea1094186a 100644
--- a/sd/source/ui/view/drviewsh.cxx
+++ b/sd/source/ui/view/drviewsh.cxx
@@ -59,12 +59,24 @@ void DrawViewShell::GotoBookmark(const OUString& rBookmark)
 
 void DrawViewShell::MakeVisible(const Rectangle& rRect, vcl::Window& rWin)
 {
+    // tdf#98646 check if Rectangle which contains the bounds of the region to
+    // be shown eventually contains values that cause overflows when processing
+    // e.g. when calling GetWidth()
+    const bool bOverflowInX(!rtl::math::approxEqual((double)rRect.getWidth(), (double)rRect.Right() - (double)rRect.Left()));
+    const bool bOverflowInY(!rtl::math::approxEqual((double)rRect.getHeight(), (double)rRect.Bottom() - (double)rRect.Top()));
+
+    if(bOverflowInX || bOverflowInY)
+    {
+        SAL_WARN("sd", "The given Rectangle contains values that lead to numerical overflows (!)");
+        return;
+    }
+
     // In older versions, if in X or Y the size of the object was
     // smaller than the visible area, the user-defined zoom was
     // changed. This was decided to be a bug for 6.x, thus I developed a
     // version which instead handles X/Y bigger/smaller and visibility
     // questions separately
-    Size aLogicSize(rRect.GetSize());
+    const Size aLogicSize(rRect.GetSize());
 
     // visible area
     Size aVisSizePixel(rWin.GetOutputSizePixel());
@@ -104,13 +116,32 @@ void DrawViewShell::MakeVisible(const Rectangle& rRect, vcl::Window& rWin)
         else
         {
             if(nFreeSpaceX > rRect.GetWidth())
+            {
                 nFreeSpaceX = rRect.GetWidth();
+            }
+
+            if(nFreeSpaceX < 0)
+            {
+                SAL_WARN("sd", "The given Rectangle contains values that lead to numerical overflows (!)");
+            }
+            else
+            {
+                const long distRight(rRect.Right() - aNewPos.X() + aVisAreaSize.Width());
 
-            while(rRect.Right() > aNewPos.X() + aVisAreaSize.Width())
-                aNewPos.X() += nFreeSpaceX;
+                if(distRight > 0)
+                {
+                    long mult = (distRight / nFreeSpaceX) + 1;
+                    aNewPos.X() += mult * nFreeSpaceX;
+                }
 
-            while(rRect.Left() < aNewPos.X())
-                aNewPos.X() -= nFreeSpaceX;
+                const long distLeft(aNewPos.X() - rRect.Left());
+
+                if(distLeft > 0)
+                {
+                    long mult = (distLeft / nFreeSpaceX) + 1;
+                    aNewPos.X() -= mult * nFreeSpaceX;
+                }
+            }
         }
 
         if(nFreeSpaceY < 0)
@@ -130,13 +161,32 @@ void DrawViewShell::MakeVisible(const Rectangle& rRect, vcl::Window& rWin)
         else
         {
             if(nFreeSpaceY > rRect.GetHeight())
+            {
                 nFreeSpaceY = rRect.GetHeight();
+            }
+
+            if(nFreeSpaceY < 0)
+            {
+                SAL_WARN("sd", "The given Rectangle contains values that lead to numerical overflows (!)");
+            }
+            else
+            {
+                const long distBottom(rRect.Bottom() - aNewPos.Y() + aVisAreaSize.Height());
 
-            while(rRect.Bottom() > aNewPos.Y() + aVisAreaSize.Height())
-                aNewPos.Y() += nFreeSpaceY;
+                if(distBottom > 0)
+                {
+                    long mult = (distBottom / nFreeSpaceY) + 1;
+                    aNewPos.Y() += mult * nFreeSpaceY;
+                }
 
-            while(rRect.Top() < aNewPos.Y())
-                aNewPos.Y() -= nFreeSpaceY;
+                const long distTop(aNewPos.Y() - rRect.Top());
+
+                if(distTop > 0)
+                {
+                    long mult = (distTop / nFreeSpaceY) + 1;
+                    aNewPos.Y() -= mult * nFreeSpaceY;
+                }
+            }
         }
 
         // did position change? Does it need to be set?