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 <pmladek@suse.cz>
Tested-by: Petr Mladek <pmladek@suse.cz>

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