tdf#145321 Crash scrolling DOCX to bottom

this is a consequence of
    commit d4dc6b5cfd
    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>
This commit is contained in:
Noel Grandin
2022-01-17 12:53:44 +02:00
parent bbc4360b5b
commit 0e4bcbb67d
5 changed files with 36 additions and 19 deletions

View File

@@ -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;
}

View File

@@ -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();

View File

@@ -1667,12 +1667,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 )

View File

@@ -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 );

View File

@@ -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);