From 23391fdb5cffb62006415ad1f4c96b6ed5d50cf8 Mon Sep 17 00:00:00 2001 From: Armin Le Grand Date: Tue, 15 Mar 2016 17:24:11 +0100 Subject: tdf#98646 Fixed freeze by flattening loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Björn Michaelsen Reviewed-by: Armin Le Grand --- sd/source/ui/view/drviewsh.cxx | 68 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 9 deletions(-) (limited to 'sd') 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? -- cgit