From bfd73a7c7b8c7e87f1edafa9d8ec257bfae26a0f Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Fri, 15 Feb 2013 13:29:04 +0100 Subject: 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) Reviewed-on: https://gerrit.libreoffice.org/2180 Reviewed-by: Petr Mladek Tested-by: Petr Mladek --- sw/inc/ndtxt.hxx | 5 ++++- sw/source/core/doc/doc.cxx | 19 +++++++++---------- sw/source/core/doc/docedt.cxx | 5 ++++- sw/source/core/txtnode/ndtxt.cxx | 35 +++++++++++++++++++++++------------ sw/source/core/txtnode/thints.cxx | 25 ++++++++++++++++++++++--- sw/source/core/undo/undel.cxx | 10 ++++++---- sw/source/core/undo/unins.cxx | 9 ++++++--- sw/source/core/undo/unovwr.cxx | 14 +++++++++----- 8 files changed, 83 insertions(+), 39 deletions(-) diff --git a/sw/inc/ndtxt.hxx b/sw/inc/ndtxt.hxx index 0865fa33d059..57bca6bd53d2 100644 --- a/sw/inc/ndtxt.hxx +++ b/sw/inc/ndtxt.hxx @@ -244,7 +244,10 @@ public: 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 7a27c238b531..b71288c1a1ea 100644 --- a/sw/source/core/doc/doc.cxx +++ b/sw/source/core/doc/doc.cxx @@ -945,12 +945,11 @@ bool SwDoc::InsertString( const SwPaM &rRg, const String &rStr, 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 @@ bool SwDoc::InsertString( const SwPaM &rRg, const String &rStr, 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 47829ea68ae0..6d34685e883c 100644 --- a/sw/source/core/doc/docedt.cxx +++ b/sw/source/core/doc/docedt.cxx @@ -744,8 +744,11 @@ bool SwDoc::Overwrite( const SwPaM &rRg, const String &rStr ) } SwTxtNode *pNode = rPt.nNode.GetNode().GetTxtNode(); - if(!pNode) + if (!pNode || ( static_cast(rStr.Len()) // worst case: no erase + + static_cast(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 f634571a2fdc..db87a6ad1d98 100644 --- a/sw/source/core/txtnode/ndtxt.cxx +++ b/sw/source/core/txtnode/ndtxt.cxx @@ -1704,23 +1704,27 @@ void SwTxtNode::CopyText( SwTxtNode *const pDest, } -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(m_Text.Len()) + static_cast(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 @@ void SwTxtNode::InsertText( const XubString & rStr, const SwIndex & rIdx, SetCalcHiddenCharFlags(); CHECK_SWPHINTS(this); + return sInserted; } /************************************************************************* @@ -3010,9 +3015,12 @@ sal_Bool SwTxtNode::GetExpandTxt( SwTxtNode& rDestNd, const SwIndex* pDestIdx, 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 @@ sal_Bool SwTxtNode::GetExpandTxt( SwTxtNode& rDestNd, const SwIndex* pDestIdx, 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 1e65ae6be012..cf549e3a8758 100644 --- a/sw/source/core/txtnode/thints.cxx +++ b/sw/source/core/txtnode/thints.cxx @@ -1254,7 +1254,15 @@ bool SwTxtNode::InsertHint( SwTxtAttr * const pAttr, const SetAttrMode nMode ) 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 @@ bool SwTxtNode::InsertHint( SwTxtAttr * const pAttr, const SetAttrMode nMode ) // 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 @@ bool SwTxtNode::InsertHint( SwTxtAttr * const pAttr, const SetAttrMode nMode ) 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 dc922336eaa6..06d4529c5fd0 100644 --- a/sw/source/core/undo/undel.cxx +++ b/sw/source/core/undo/undel.cxx @@ -789,8 +789,9 @@ void SwUndoDelete::UndoImpl(::sw::UndoRedoContext & rContext) } 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 @@ void SwUndoDelete::UndoImpl(::sw::UndoRedoContext & rContext) // 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 03cc45447b21..7018dd1b6c1c 100644 --- a/sw/source/core/undo/unins.cxx +++ b/sw/source/core/undo/unins.cxx @@ -347,8 +347,10 @@ void SwUndoInsert::RedoImpl(::sw::UndoRedoContext & rContext) { 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 @@ void SwUndoReplace::Impl::UndoImpl(::sw::UndoRedoContext & rContext) 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 05fd44120cbe..091ea2bd7ae2 100644 --- a/sw/source/core/undo/unovwr.cxx +++ b/sw/source/core/undo/unovwr.cxx @@ -154,8 +154,9 @@ sal_Bool SwUndoOverwrite::CanGrouping( SwDoc* pDoc, SwPosition& rPos, 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 @@ void SwUndoOverwrite::UndoImpl(::sw::UndoRedoContext & rContext) { // 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 @@ void SwUndoOverwrite::RedoImpl(::sw::UndoRedoContext & rContext) 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; -- cgit v1.2.3