[PATCH libreoffice-4-0] fdo#60732: make callers of SwTxtNode::InsertText more robust...
Michael Stahl (via Code Review)
gerrit at gerrit.libreoffice.org
Fri Feb 15 14:36:00 PST 2013
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 at redhat.com>
More information about the LibreOffice
mailing list