tdf#146795: Make sure to use valid position hints to avoid crash.

This commit also unifies the code path for both "skip empty cells"
option is selected and when that option is not selected, which
simplifies the code quite a bit.

Change-Id: I21054cccdd9f60e073695091740b9415dfef2985
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129267
Tested-by: Jenkins
Reviewed-by: Kohei Yoshida <kohei@libreoffice.org>

4 files changed, 71 insertions, 21 deletions

diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 905fc0c3373c..ef20c848e622 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -1228,10 +1228,13 @@ public:
 
 }
 
-void ScColumn::RemoveEditAttribs( SCROW nStartRow, SCROW nEndRow )
+void ScColumn::RemoveEditAttribs( sc::ColumnBlockPosition& rBlockPos, SCROW nStartRow, SCROW nEndRow )
 {
     RemoveEditAttribsHandler aFunc(maCells, &GetDoc());
-    sc::ProcessEditText(maCells.begin(), maCells, nStartRow, nEndRow, aFunc);
+
+    rBlockPos.miCellPos = sc::ProcessEditText(
+        rBlockPos.miCellPos, maCells, nStartRow, nEndRow, aFunc);
+
     aFunc.commitStrings();
 }
 
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index 68eb921fd354..74b2e1ac7fad 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -1023,7 +1023,7 @@ void ScColumn::DeleteArea(
     if ( nDelFlag & InsertDeleteFlags::EDITATTR )
     {
         OSL_ENSURE( nContFlag == InsertDeleteFlags::NONE, "DeleteArea: Wrong Flags" );
-        RemoveEditAttribs( nStartRow, nEndRow );
+        RemoveEditAttribs(aBlockPos, nStartRow, nEndRow);
     }
 
     // Delete attributes just now
@@ -1123,7 +1123,16 @@ public:
         mpSharedStringPool(pSharedStringPool)
     {
         if (mpDestBlockPos)
+        {
+            {
+                // Re-initialize the broadcaster position hint, which may have
+                // become invalid by the time it gets here...
+                sc::ColumnBlockPosition aTempPos;
+                mrDestCol.InitBlockPosition(aTempPos);
+                mpDestBlockPos->miBroadcasterPos = aTempPos.miBroadcasterPos;
+            }
             maDestBlockPos = *mpDestBlockPos;
+        }
         else
             mrDestCol.InitBlockPosition(maDestBlockPos);
     }
@@ -1357,7 +1366,7 @@ public:
         mpDestBlockPos(rCxt.getBlockPosition(nDestTab, nDestCol))
     {
         if (mpDestBlockPos)
-            maDestBlockPos = *mpDestBlockPos;
+            maDestBlockPos.miCellTextAttrPos = mpDestBlockPos->miCellTextAttrPos;
         else
             rDestCol.InitBlockPosition(maDestBlockPos);
     }
@@ -1366,7 +1375,7 @@ public:
     {
         if (mpDestBlockPos)
             // Don't forget to save this to the context!
-            *mpDestBlockPos = maDestBlockPos;
+            mpDestBlockPos->miCellTextAttrPos = maDestBlockPos.miCellTextAttrPos;
     }
 
     void operator() ( const sc::CellTextAttrStoreType::value_type& aNode, size_t nOffset, size_t nDataSize )
@@ -1392,6 +1401,10 @@ public:
 void ScColumn::CopyFromClip(
     sc::CopyFromClipContext& rCxt, SCROW nRow1, SCROW nRow2, tools::Long nDy, ScColumn& rColumn )
 {
+    sc::ColumnBlockPosition* pBlockPos = rCxt.getBlockPosition(nTab, nCol);
+    if (!pBlockPos)
+        return;
+
     if ((rCxt.getInsertFlag() & InsertDeleteFlags::ATTRIB) != InsertDeleteFlags::NONE)
     {
         if (rCxt.isSkipEmptyCells())
@@ -1433,7 +1446,7 @@ void ScColumn::CopyFromClip(
 
             ScTokenArray aArr(GetDoc());
             aArr.AddSingleReference( aRef );
-            SetFormulaCell(nDestRow, new ScFormulaCell(rDocument, aDestPos, aArr));
+            SetFormulaCell(*pBlockPos, nDestRow, new ScFormulaCell(rDocument, aDestPos, aArr));
         }
 
         // Don't forget to copy the cell text attributes.
diff --git a/sc/source/core/data/column4.cxx b/sc/source/core/data/column4.cxx
index e3bcfe169941..980ceb5ac3a2 100644
--- a/sc/source/core/data/column4.cxx
+++ b/sc/source/core/data/column4.cxx
@@ -100,6 +100,50 @@ void ScColumn::DeleteBeforeCopyFromClip(
     if (!rDocument.ValidRow(aRange.mnRow1) || !rDocument.ValidRow(aRange.mnRow2))
         return;
 
+    sc::ColumnBlockPosition* pBlockPos = rCxt.getBlockPosition(nTab, nCol);
+    if (!pBlockPos)
+        return;
+
+    InsertDeleteFlags nDelFlag = rCxt.getDeleteFlag();
+
+    if (!rCxt.isSkipEmptyCells())
+    {
+        // Delete the whole destination range.
+
+        if (nDelFlag & InsertDeleteFlags::CONTENTS)
+        {
+            sc::SingleColumnSpanSet aDeletedRows(GetDoc().GetSheetLimits());
+            DeleteCells(*pBlockPos, aRange.mnRow1, aRange.mnRow2, nDelFlag, aDeletedRows);
+            rBroadcastSpans.set(GetDoc(), nTab, nCol, aDeletedRows, true);
+        }
+
+        if (nDelFlag & InsertDeleteFlags::NOTE)
+            DeleteCellNotes(*pBlockPos, aRange.mnRow1, aRange.mnRow2, false);
+
+        if (nDelFlag & InsertDeleteFlags::EDITATTR)
+            RemoveEditAttribs(*pBlockPos, aRange.mnRow1, aRange.mnRow2);
+
+        if (nDelFlag & InsertDeleteFlags::ATTRIB)
+        {
+            pAttrArray->DeleteArea(aRange.mnRow1, aRange.mnRow2);
+
+            if (rCxt.isTableProtected())
+            {
+                ScPatternAttr aPattern(rDocument.GetPool());
+                aPattern.GetItemSet().Put(ScProtectionAttr(false));
+                ApplyPatternArea(aRange.mnRow1, aRange.mnRow2, aPattern);
+            }
+
+            ScConditionalFormatList* pCondList = rCxt.getCondFormatList();
+            if (pCondList)
+                pCondList->DeleteArea(nCol, aRange.mnRow1, nCol, aRange.mnRow2);
+        }
+        else if ((nDelFlag & InsertDeleteFlags::HARDATTR) == InsertDeleteFlags::HARDATTR)
+            pAttrArray->DeleteHardAttr(aRange.mnRow1, aRange.mnRow2);
+
+        return;
+    }
+
     ScRange aClipRange = rCxt.getClipDoc()->GetClipParam().getWholeRange();
     SCROW nClipRow1 = aClipRange.aStart.Row();
     SCROW nClipRow2 = aClipRange.aEnd.Row();
@@ -149,10 +193,6 @@ void ScColumn::DeleteBeforeCopyFromClip(
         nDestOffset += nClipRowLen;
     }
 
-    InsertDeleteFlags nDelFlag = rCxt.getDeleteFlag();
-    sc::ColumnBlockPosition aBlockPos;
-    InitBlockPosition(aBlockPos);
-
     for (const auto& rDestSpan : aDestSpans)
     {
         SCROW nRow1 = rDestSpan.mnRow1;
@@ -161,15 +201,15 @@ void ScColumn::DeleteBeforeCopyFromClip(
         if (nDelFlag & InsertDeleteFlags::CONTENTS)
         {
             sc::SingleColumnSpanSet aDeletedRows(GetDoc().GetSheetLimits());
-            DeleteCells(aBlockPos, nRow1, nRow2, nDelFlag, aDeletedRows);
+            DeleteCells(*pBlockPos, nRow1, nRow2, nDelFlag, aDeletedRows);
             rBroadcastSpans.set(GetDoc(), nTab, nCol, aDeletedRows, true);
         }
 
         if (nDelFlag & InsertDeleteFlags::NOTE)
-            DeleteCellNotes(aBlockPos, nRow1, nRow2, false);
+            DeleteCellNotes(*pBlockPos, nRow1, nRow2, false);
 
         if (nDelFlag & InsertDeleteFlags::EDITATTR)
-            RemoveEditAttribs(nRow1, nRow2);
+            RemoveEditAttribs(*pBlockPos, nRow1, nRow2);
 
         // Delete attributes just now
         if (nDelFlag & InsertDeleteFlags::ATTRIB)
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index b8239e95fc8b..3592eb4a3ee1 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -2914,14 +2914,8 @@ void ScDocument::CopyFromClip(
         SCCOL nCol2 = rRange.aEnd.Col();
         SCROW nRow2 = rRange.aEnd.Row();
 
-        if (bSkipEmptyCells)
-        {
-            // Delete cells in the destination only if their corresponding clip cells are not empty.
-            aCxt.setDestRange(nCol1, nRow1, nCol2, nRow2);
-            DeleteBeforeCopyFromClip(aCxt, rMark, aBroadcastSpans);
-        }
-        else
-            DeleteArea(nCol1, nRow1, nCol2, nRow2, rMark, nDelFlag, false, &aBroadcastSpans);
+        aCxt.setDestRange(nCol1, nRow1, nCol2, nRow2);
+        DeleteBeforeCopyFromClip(aCxt, rMark, aBroadcastSpans);
 
         if (CopyOneCellFromClip(aCxt, nCol1, nRow1, nCol2, nRow2))
             continue;