Hi,
I have submitted a patch for review:
https://gerrit.libreoffice.org/2180
To pull it, you can do:
git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/80/2180/1
fdo#60732: make callers of SwTxtNode::InsertText more robust:
Return the actually inserted string from InsertText(), so callers can
check if the insertion was actually successful.
Especially InsertHint() will likely cause problems if it cannot insert
its CH_TXTATR; also Undo objects should not Undo more than was actually
inserted.
Change-Id: I87c9ea8b226ae4a2a6c20c112da76db07051a1bf
(cherry picked from commit d47218d79a2440e71efb66b2224063801ba6623b)
---
M sw/inc/ndtxt.hxx
M sw/source/core/doc/doc.cxx
M sw/source/core/doc/docedt.cxx
M sw/source/core/txtnode/ndtxt.cxx
M sw/source/core/txtnode/thints.cxx
M sw/source/core/undo/undel.cxx
M sw/source/core/undo/unins.cxx
M sw/source/core/undo/unovwr.cxx
8 files changed, 83 insertions(+), 39 deletions(-)
diff --git a/sw/inc/ndtxt.hxx b/sw/inc/ndtxt.hxx
index 0865fa3..57bca6b 100644
--- a/sw/inc/ndtxt.hxx
+++ b/sw/inc/ndtxt.hxx
@@ -244,7 +244,10 @@
virtual sal_uInt16 ResetAllAttr();
/// insert text content
- void InsertText( const XubString & rStr, const SwIndex & rIdx,
+ /// @param rStr text to insert; in case it does not fit into the limit of
+ /// TXTNODE_MAX, the longest prefix that fits is inserted
+ /// @return the prefix of rStr that was actually inserted
+ OUString InsertText( const XubString & rStr, const SwIndex & rIdx,
const enum IDocumentContentOperations::InsertFlags nMode
= IDocumentContentOperations::INS_DEFAULT );
diff --git a/sw/source/core/doc/doc.cxx b/sw/source/core/doc/doc.cxx
index 7a27c23..b71288c 100644
--- a/sw/source/core/doc/doc.cxx
+++ b/sw/source/core/doc/doc.cxx
@@ -945,12 +945,11 @@
if (!bDoesUndo || !GetIDocumentUndoRedo().DoesGroupUndo())
{
- pNode->InsertText( rStr, rPos.nContent, nInsertMode );
-
+ OUString const ins(pNode->InsertText(rStr, rPos.nContent, nInsertMode));
if (bDoesUndo)
{
- SwUndoInsert * const pUndo( new SwUndoInsert(
- rPos.nNode, rPos.nContent.GetIndex(), rStr.Len(), nInsertMode));
+ SwUndoInsert * const pUndo( new SwUndoInsert(rPos.nNode,
+ rPos.nContent.GetIndex(), ins.getLength(), nInsertMode));
GetIDocumentUndoRedo().AppendUndo(pUndo);
}
}
@@ -980,16 +979,16 @@
GetIDocumentUndoRedo().AppendUndo( pUndo );
}
- pNode->InsertText( rStr, rPos.nContent, nInsertMode );
+ OUString const ins(pNode->InsertText(rStr, rPos.nContent, nInsertMode));
- for( xub_StrLen i = 0; i < rStr.Len(); ++i )
+ for (sal_Int32 i = 0; i < ins.getLength(); ++i)
{
nInsPos++;
- // if CanGrouping() returns sal_True, everything has already been done
- if( !pUndo->CanGrouping( rStr.GetChar( i ) ))
+ // if CanGrouping() returns true, everything has already been done
+ if (!pUndo->CanGrouping(ins[i]))
{
- pUndo = new SwUndoInsert( rPos.nNode, nInsPos, 1, nInsertMode,
- !rCC.isLetterNumeric( rStr, i ) );
+ pUndo = new SwUndoInsert(rPos.nNode, nInsPos, 1, nInsertMode,
+ !rCC.isLetterNumeric(ins, i));
GetIDocumentUndoRedo().AppendUndo( pUndo );
}
}
diff --git a/sw/source/core/doc/docedt.cxx b/sw/source/core/doc/docedt.cxx
index 693e2bf..e45f525 100644
--- a/sw/source/core/doc/docedt.cxx
+++ b/sw/source/core/doc/docedt.cxx
@@ -734,8 +734,11 @@
}
SwTxtNode *pNode = rPt.nNode.GetNode().GetTxtNode();
- if(!pNode)
+ if (!pNode || ( static_cast<size_t>(rStr.Len()) // worst case: no erase
+ + static_cast<size_t>(pNode->GetTxt().Len()) > TXTNODE_MAX))
+ {
return sal_False;
+ }
if (GetIDocumentUndoRedo().DoesUndo())
{
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index f634571..db87a6a 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -1704,23 +1704,27 @@
}
-void SwTxtNode::InsertText( const XubString & rStr, const SwIndex & rIdx,
+OUString SwTxtNode::InsertText( const XubString & rStr, const SwIndex & rIdx,
const IDocumentContentOperations::InsertFlags nMode )
{
OSL_ENSURE( rIdx <= m_Text.Len(), "SwTxtNode::InsertText: invalid index." );
- OSL_ENSURE( (sal_uLong)m_Text.Len() + (sal_uLong)rStr.Len() <= STRING_LEN,
- "SwTxtNode::InsertText: node text with insertion > STRING_LEN." );
xub_StrLen aPos = rIdx.GetIndex();
xub_StrLen nLen = m_Text.Len() - aPos;
ssize_t const nOverflow(static_cast<ssize_t>(m_Text.Len())
+ static_cast<ssize_t>(rStr.Len()) - TXTNODE_MAX);
- m_Text.Insert((nOverflow > 0) ? rStr.Copy(0, rStr.Len() - nOverflow) : rStr,
- aPos);
+ SAL_WARN_IF(nOverflow > 0, "sw.core",
+ "SwTxtNode::InsertText: node text with insertion > TXTNODE_MAX.");
+ OUString const sInserted(
+ (nOverflow > 0) ? rStr.Copy(0, rStr.Len() - nOverflow) : rStr);
+ if (sInserted.isEmpty())
+ {
+ return sInserted;
+ }
+ m_Text.Insert(sInserted, aPos);
assert(m_Text.Len() <= TXTNODE_MAX);
nLen = m_Text.Len() - aPos - nLen;
-
- if ( !nLen ) return;
+ assert(nLen != 0);
bool bOldExpFlg = IsIgnoreDontExpand();
if (nMode & IDocumentContentOperations::INS_FORCEHINTEXPAND)
@@ -1805,6 +1809,7 @@
SetCalcHiddenCharFlags();
CHECK_SWPHINTS(this);
+ return sInserted;
}
/*************************************************************************
@@ -3010,9 +3015,12 @@
if( aExpand.Len() )
{
++aDestIdx; // dahinter einfuegen;
- rDestNd.InsertText( aExpand, aDestIdx );
+ OUString const ins(
+ rDestNd.InsertText( aExpand, aDestIdx));
+ SAL_INFO_IF(ins.getLength() != aExpand.Len(),
+ "sw.core", "GetExpandTxt lossage");
aDestIdx = nInsPos + nAttrStartIdx;
- nInsPos = nInsPos + aExpand.Len();
+ nInsPos = nInsPos + ins.getLength();
}
rDestNd.EraseText( aDestIdx, 1 );
--nInsPos;
@@ -3041,10 +3049,13 @@
rDestNd.InsertItem(aItem,
aDestIdx.GetIndex(),
aDestIdx.GetIndex() );
- rDestNd.InsertText( sExpand, aDestIdx,
- IDocumentContentOperations::INS_EMPTYEXPAND);
+ OUString const ins( rDestNd.InsertText(sExpand,
+ aDestIdx,
+ IDocumentContentOperations::INS_EMPTYEXPAND));
+ SAL_INFO_IF(ins.getLength() != sExpand.Len(),
+ "sw.core", "GetExpandTxt lossage");
aDestIdx = nInsPos + nAttrStartIdx;
- nInsPos = nInsPos + sExpand.Len();
+ nInsPos = nInsPos + ins.getLength();
}
}
rDestNd.EraseText( aDestIdx, 1 );
diff --git a/sw/source/core/txtnode/thints.cxx b/sw/source/core/txtnode/thints.cxx
index 1e65ae6..cf549e3 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -1254,7 +1254,15 @@
SwIndex aIdx( this, *pAttr->GetStart() );
const rtl::OUString c(GetCharOfTxtAttr(*pAttr));
- InsertText( c, aIdx, nInsertFlags );
+ OUString const ins( InsertText(c, aIdx, nInsertFlags) );
+ if (ins.isEmpty())
+ {
+ // do not record deletion of Format!
+ ::sw::UndoGuard const ug(
+ pFmt->GetDoc()->GetIDocumentUndoRedo());
+ DestroyAttr(pAttr);
+ return false; // text node full :(
+ }
nInsMode |= nsSetAttrMode::SETATTR_NOTXTATRCHR;
if (pAnchor &&
@@ -1371,7 +1379,12 @@
// Dokument nicht eingetrage wird.
SwIndex aNdIdx( this, *pAttr->GetStart() );
const rtl::OUString c(GetCharOfTxtAttr(*pAttr));
- InsertText( c, aNdIdx, nInsertFlags );
+ OUString const ins( InsertText(c, aNdIdx, nInsertFlags) );
+ if (ins.isEmpty())
+ {
+ DestroyAttr(pAttr);
+ return false; // text node full :(
+ }
nInsMode |= nsSetAttrMode::SETATTR_NOTXTATRCHR;
}
@@ -1430,7 +1443,13 @@
if( !(nsSetAttrMode::SETATTR_NOTXTATRCHR & nInsMode) )
{
SwIndex aIdx( this, *pAttr->GetStart() );
- InsertText( rtl::OUString(GetCharOfTxtAttr(*pAttr)), aIdx, nInsertFlags );
+ OUString const ins( InsertText(OUString(GetCharOfTxtAttr(*pAttr)),
+ aIdx, nInsertFlags) );
+ if (ins.isEmpty())
+ {
+ DestroyAttr(pAttr);
+ return false; // text node full :(
+ }
// adjust end of hint to account for inserted CH_TXTATR
xub_StrLen * const pEnd(pAttr->GetEnd());
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index dc92233..06d4529 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -789,8 +789,9 @@
}
if( pTxtNd )
{
- pTxtNd->InsertText( *pEndStr, aPos.nContent,
- IDocumentContentOperations::INS_NOHINTEXPAND );
+ OUString const ins( pTxtNd->InsertText(*pEndStr, aPos.nContent,
+ IDocumentContentOperations::INS_NOHINTEXPAND) );
+ assert(ins.getLength() == pEndStr->Len()); // must succeed
// METADATA: restore
pTxtNd->RestoreMetadata(m_pMetadataUndoEnd);
}
@@ -882,8 +883,9 @@
// SectionNode mode and selection from top to bottom:
// -> in StartNode is still the rest of the Join => delete
aPos.nContent.Assign( pTxtNd, nSttCntnt );
- pTxtNd->InsertText( *pSttStr, aPos.nContent,
- IDocumentContentOperations::INS_NOHINTEXPAND );
+ OUString const ins( pTxtNd->InsertText(*pSttStr, aPos.nContent,
+ IDocumentContentOperations::INS_NOHINTEXPAND) );
+ assert(ins.getLength() == pSttStr->Len()); // must succeed
// METADATA: restore
pTxtNd->RestoreMetadata(m_pMetadataUndoStart);
}
diff --git a/sw/source/core/undo/unins.cxx b/sw/source/core/undo/unins.cxx
index 03cc454..7018dd1 100644
--- a/sw/source/core/undo/unins.cxx
+++ b/sw/source/core/undo/unins.cxx
@@ -347,8 +347,10 @@
{
SwTxtNode *const pTxtNode = pCNd->GetTxtNode();
OSL_ENSURE( pTxtNode, "where is my textnode ?" );
- pTxtNode->InsertText( *pTxt, pPam->GetMark()->nContent,
- m_nInsertFlags );
+ OUString const ins(
+ pTxtNode->InsertText( *pTxt, pPam->GetMark()->nContent,
+ m_nInsertFlags) );
+ assert(ins.getLength() == pTxt->Len()); // must succeed
DELETEZ( pTxt );
}
else
@@ -712,7 +714,8 @@
if (!m_sOld.isEmpty())
{
- pNd->InsertText( m_sOld, aIdx );
+ OUString const ins( pNd->InsertText( m_sOld, aIdx ) );
+ assert(ins.getLength() == m_sOld.getLength()); // must succeed
}
if( pHistory )
diff --git a/sw/source/core/undo/unovwr.cxx b/sw/source/core/undo/unovwr.cxx
index 05fd441..091ea2b 100644
--- a/sw/source/core/undo/unovwr.cxx
+++ b/sw/source/core/undo/unovwr.cxx
@@ -154,8 +154,9 @@
bool bOldExpFlg = pDelTxtNd->IsIgnoreDontExpand();
pDelTxtNd->SetIgnoreDontExpand( true );
- pDelTxtNd->InsertText( rtl::OUString(cIns), rPos.nContent,
- IDocumentContentOperations::INS_EMPTYEXPAND );
+ OUString const ins( pDelTxtNd->InsertText(OUString(cIns), rPos.nContent,
+ IDocumentContentOperations::INS_EMPTYEXPAND) );
+ assert(ins.getLength() == 1); // check in SwDoc::Overwrite => cannot fail
aInsStr.Insert( cIns );
if( !bInsChar )
@@ -210,7 +211,8 @@
{
// do it individually, to keep the attributes!
*pTmpStr = aDelStr.GetChar( n );
- pTxtNd->InsertText( aTmpStr, rIdx /*???, SETATTR_NOTXTATRCHR*/ );
+ OUString const ins( pTxtNd->InsertText(aTmpStr, rIdx) );
+ assert(ins.getLength() == 1); // cannot fail
rIdx -= 2;
pTxtNd->EraseText( rIdx, 1 );
rIdx += 2;
@@ -279,8 +281,10 @@
for( xub_StrLen n = 0; n < aInsStr.Len(); n++ )
{
// do it individually, to keep the attributes!
- pTxtNd->InsertText( rtl::OUString(aInsStr.GetChar(n)), rIdx,
- IDocumentContentOperations::INS_EMPTYEXPAND );
+ OUString const ins(
+ pTxtNd->InsertText( rtl::OUString(aInsStr.GetChar(n)), rIdx,
+ IDocumentContentOperations::INS_EMPTYEXPAND) );
+ assert(ins.getLength() == 1); // cannot fail
if( n < aDelStr.Len() )
{
rIdx -= 2;
--
To view, visit https://gerrit.libreoffice.org/2180
To unsubscribe, visit https://gerrit.libreoffice.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87c9ea8b226ae4a2a6c20c112da76db07051a1bf
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Michael Stahl <mstahl@redhat.com>
Context
- [PATCH libreoffice-4-0] fdo#60732: make callers of SwTxtNode::InsertText more robust... · Michael Stahl (via Code Review)
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.