From 6ae00fc24786eac379e6e64ac3e6d83c6a057b24 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Mon, 17 Jan 2022 12:53:44 +0200 Subject: 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 (cherry picked from commit 0e4bcbb67dda204ba78f99df68a63458c29e7470) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128441 Reviewed-by: Xisco Fauli (cherry picked from commit 68fa037b8f1300ffb950cc3ba4be4347f976eb83) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128446 Reviewed-by: Michael Stahl Tested-by: Michael Stahl --- sw/source/core/inc/drawfont.hxx | 22 ++++++++++++++++++++++ sw/source/core/text/inftxt.cxx | 4 +++- sw/source/core/text/itrcrsr.cxx | 6 +++++- sw/source/core/txtnode/fntcap.cxx | 7 ++----- sw/source/core/txtnode/swfont.cxx | 16 ++++------------ 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(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(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); -- cgit