tdf#145321 Crash scrolling DOCX to bottom

this is a consequence of
commit d4dc6b5cfdb02ad00a06ad32650948648abe010d
use std::vector for fetching DX array data
which made pre-existing bugs easier to see.

This crash made apparent that we have bad data ending up in
SwDrawTextInfo. So I added some asserts there to catch that.

However, that simply made apparent that there are bug(s) at
a higher level that I have no idea how to to fix.

So at the primary problem site in
SwTextCursor::GetModelPositionForViewPoint I clamp
the values to sane ones.

Change-Id: Ic74f6944932bbfc22e8cf9addf9e7511755b18be
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128497
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
(cherry picked from commit 0e4bcbb67dda204ba78f99df68a63458c29e7470)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128441
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit 68fa037b8f1300ffb950cc3ba4be4347f976eb83)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128446
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
Tested-by: Michael Stahl <michael.stahl@allotropia.de>

5 files changed, 36 insertions, 19 deletions

diff --git a/sw/source/core/inc/drawfont.hxx b/sw/source/core/inc/drawfont.hxx
index 9f5a6e676291..c6d834fdb253 100644
--- a/sw/source/core/inc/drawfont.hxx
+++ b/sw/source/core/inc/drawfont.hxx
@@ -118,6 +118,7 @@ public:
                     vcl::text::TextLayoutCache const*const pCachedVclData = nullptr)
         : m_pCachedVclData(pCachedVclData)
     {
+        assert( (nLen == TextFrameIndex(COMPLETE_STRING)) ? (nIdx.get() < rText.getLength()) : (nIdx + nLen).get() <= rText.getLength() );
         m_pFrame = nullptr;
         m_pSh = pSh;
         m_pOut = &rOut;
@@ -414,10 +415,29 @@ public:
 
     void SetText( const OUString &rNew )
     {
+        assert( (m_nLen == TextFrameIndex(COMPLETE_STRING)) ? (m_nIdx.get() < rNew.getLength()) : (m_nIdx + m_nLen).get() <= rNew.getLength() );
         m_aText = rNew;
         m_pCachedVclData = nullptr; // would any case benefit from save/restore?
     }
 
+    // These methods are here so we can set all the related fields together to preserve the invariants that we assert
+    void SetTextIdxLen( const OUString &rNewStr, TextFrameIndex const nNewIdx, TextFrameIndex const nNewLen )
+    {
+        assert( (nNewLen == TextFrameIndex(COMPLETE_STRING)) ? (nNewIdx.get() < rNewStr.getLength()) : (nNewIdx + nNewLen).get() <= rNewStr.getLength() );
+        m_aText = rNewStr;
+        m_nIdx = nNewIdx;
+        m_nLen = nNewLen;
+        m_pCachedVclData = nullptr; // would any case benefit from save/restore?
+    }
+
+    // These methods are here so we can set all the related fields together to preserve the invariants that we assert
+    void SetIdxLen( TextFrameIndex const nNewIdx, TextFrameIndex const nNewLen )
+    {
+        assert( (nNewLen == TextFrameIndex(COMPLETE_STRING)) ? (nNewIdx.get() < m_aText.getLength()) : (nNewIdx + nNewLen).get() <= m_aText.getLength() );
+        m_nIdx = nNewIdx;
+        m_nLen = nNewLen;
+    }
+
     void SetWrong(sw::WrongListIterator *const pNew)
     {
         m_pWrong = pNew;
@@ -457,11 +477,13 @@ public:
 
     void SetIdx(TextFrameIndex const nNew)
     {
+        assert( (m_nLen == TextFrameIndex(COMPLETE_STRING)) ? (nNew.get() < m_aText.getLength()) : (nNew + m_nLen).get() <= m_aText.getLength() );
         m_nIdx = nNew;
     }
 
     void SetLen(TextFrameIndex const nNew)
     {
+        assert( (nNew == TextFrameIndex(COMPLETE_STRING)) ? (m_nIdx.get() < m_aText.getLength()) : (m_nIdx + nNew).get() <= m_aText.getLength() );
         m_nLen = nNew;
     }
 
diff --git a/sw/source/core/text/inftxt.cxx b/sw/source/core/text/inftxt.cxx
index a5e588ebad0b..16af09c8ccbf 100644
--- a/sw/source/core/text/inftxt.cxx
+++ b/sw/source/core/text/inftxt.cxx
@@ -1230,7 +1230,9 @@ void SwTextPaintInfo::DrawBackBrush( const SwLinePortion &rPor ) const
                     {
                         continue;
                     }
-                    if (i >= TextFrameIndex(GetText().getLength())
+                    if ((i + TextFrameIndex(1) ).get() > GetText().getLength())
+                        ; // prevent crash by not passing bad data down to GetTextSize->SwDrawTextInfo
+                    else if (i >= TextFrameIndex(GetText().getLength())
                         || GetText()[sal_Int32(i)] != CH_BLANK)
                     {
                         sal_uInt16 nOldWidth = rPor.Width();
diff --git a/sw/source/core/text/itrcrsr.cxx b/sw/source/core/text/itrcrsr.cxx
index cd915f14ea50..7d60210fba7b 100644
--- a/sw/source/core/text/itrcrsr.cxx
+++ b/sw/source/core/text/itrcrsr.cxx
@@ -1662,12 +1662,16 @@ TextFrameIndex SwTextCursor::GetModelPositionForViewPoint( SwPosition *pPos, con
                 SwParaPortion* pPara = const_cast<SwParaPortion*>(GetInfo().GetParaPortion());
                 OSL_ENSURE( pPara, "No paragraph!" );
 
+                // protect against bugs elsewhere
+                SAL_WARN_IF( aSizeInf.GetIdx().get() + pPor->GetLen().get() > aSizeInf.GetText().getLength(), "sw", "portion and text are out of sync" );
+                TextFrameIndex nSafeLen( std::min(pPor->GetLen().get(), aSizeInf.GetText().getLength() - aSizeInf.GetIdx().get()) );
+
                 SwDrawTextInfo aDrawInf( aSizeInf.GetVsh(),
                                          *aSizeInf.GetOut(),
                                          &pPara->GetScriptInfo(),
                                          aSizeInf.GetText(),
                                          aSizeInf.GetIdx(),
-                                         pPor->GetLen() );
+                                         nSafeLen );
 
                 // Drop portion works like a multi portion, just its parts are not portions
                 if( pPor->IsDropPortion() && static_cast<SwDropPortion*>(pPor)->GetLines() > 1 )
diff --git a/sw/source/core/txtnode/fntcap.cxx b/sw/source/core/txtnode/fntcap.cxx
index 47d91755e7d5..12dd8de0ccbf 100644
--- a/sw/source/core/txtnode/fntcap.cxx
+++ b/sw/source/core/txtnode/fntcap.cxx
@@ -720,14 +720,11 @@ void SwSubFont::DoOnCapitals( SwDoCapitals &rDo )
                             oldText.copy(sal_Int32(nOldPos), sal_Int32(nTmp-nOldPos)));
                         aCapInf.nIdx = nOldPos;
                         aCapInf.nLen = nTmp - nOldPos;
-                        rDo.GetInf().SetIdx(TextFrameIndex(0));
-                        rDo.GetInf().SetLen(TextFrameIndex(aNewText.getLength()));
-                        rDo.GetInf().SetText( aNewText );
+                        rDo.GetInf().SetTextIdxLen( aNewText, TextFrameIndex(0), TextFrameIndex(aNewText.getLength()));
                     }
                     else
                     {
-                        rDo.GetInf().SetIdx( nOldPos );
-                        rDo.GetInf().SetLen( nTmp - nOldPos );
+                        rDo.GetInf().SetIdxLen( nOldPos, nTmp - nOldPos );
                     }
 
                     rDo.GetInf().SetOut( *pOutSize );
diff --git a/sw/source/core/txtnode/swfont.cxx b/sw/source/core/txtnode/swfont.cxx
index b1451a77a45b..e0b64c24ef52 100644
--- a/sw/source/core/txtnode/swfont.cxx
+++ b/sw/source/core/txtnode/swfont.cxx
@@ -1230,9 +1230,7 @@ void SwSubFont::DrawText_( SwDrawTextInfo &rInf, const bool bGrey )
         }
 
         rInf.SetWidth( sal_uInt16(aFontSize.Width() + nSpace) );
-        rInf.SetText( "  " );
-        rInf.SetIdx( TextFrameIndex(0) );
-        rInf.SetLen( TextFrameIndex(2) );
+        rInf.SetTextIdxLen( "  ", TextFrameIndex(0), TextFrameIndex(2) );
         SetUnderline( nOldUnder );
         rInf.SetUnderFnt( nullptr );
 
@@ -1242,9 +1240,7 @@ void SwSubFont::DrawText_( SwDrawTextInfo &rInf, const bool bGrey )
         pUnderFnt->GetFont().DrawStretchText_( rInf );
 
         rInf.SetUnderFnt( pUnderFnt );
-        rInf.SetText(oldStr);
-        rInf.SetIdx( nOldIdx );
-        rInf.SetLen( nOldLen );
+        rInf.SetTextIdxLen(oldStr, nOldIdx, nOldLen);
     }
 
     rInf.SetPos(aOldPos);
@@ -1312,9 +1308,7 @@ void SwSubFont::DrawStretchText_( SwDrawTextInfo &rInf )
         const OUString oldStr = rInf.GetText();
         TextFrameIndex const nOldIdx = rInf.GetIdx();
         TextFrameIndex const nOldLen = rInf.GetLen();
-        rInf.SetText( "  " );
-        rInf.SetIdx( TextFrameIndex(0) );
-        rInf.SetLen( TextFrameIndex(2) );
+        rInf.SetTextIdxLen( "  ", TextFrameIndex(0), TextFrameIndex(2) );
         SetUnderline( nOldUnder );
         rInf.SetUnderFnt( nullptr );
 
@@ -1324,9 +1318,7 @@ void SwSubFont::DrawStretchText_( SwDrawTextInfo &rInf )
         pUnderFnt->GetFont().DrawStretchText_( rInf );
 
         rInf.SetUnderFnt( pUnderFnt );
-        rInf.SetText(oldStr);
-        rInf.SetIdx( nOldIdx );
-        rInf.SetLen( nOldLen );
+        rInf.SetTextIdxLen(oldStr, nOldIdx, nOldLen);
     }
 
     rInf.SetPos(aOldPos);