sc/inc/dpcache.hxx | 3 sc/qa/unit/PivotTableFormatsImportExport.cxx | 45 +++++----- sc/qa/unit/data/xlsx/pivot-table/PivotTableWithNoSourceData.xlsx |binary sc/source/core/data/PivotTableFormatOutput.cxx | 32 +++++-- sc/source/core/data/dpcache.cxx | 19 +++- 5 files changed, 66 insertions(+), 33 deletions(-)
New commits: commit e8df584b5f86eb5ad5911ecc76fca16e80c8c726 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Wed Nov 12 18:59:58 2025 +0900 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Nov 21 09:16:43 2025 +0100 sc: fix crash in pivot table with formats and no source data Add more strict checking if the dimension exists as a field so it can't happend that we crash when we want a dimension that doesn't exists. Additionally check the group items when filling the string values of the members, because sometimes a dimension is a group and is not found as a normal field. Also make sure that if the dimension is not available we don't crash. In the test the document has no source data and the "Month" is a group dimension. Change-Id: I9db996bb4327b2e081dbfc6dea7d731455111490 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193845 Reviewed-by: Tomaž Vajngerl <[email protected]> Tested-by: Jenkins Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193998 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> (cherry picked from commit 81d59ec3770b841b9c44f33c91e0d9360e1773e2) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194087 Tested-by: Miklos Vajna <[email protected]> diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx index 1e7d7075e813..2e26f75d26e3 100644 --- a/sc/inc/dpcache.hxx +++ b/sc/inc/dpcache.hxx @@ -163,6 +163,7 @@ public: */ SC_DLLPUBLIC sal_Int32 GetGroupType(tools::Long nDim) const; + SC_DLLPUBLIC bool IsValidDimensionIndex(tools::Long nDimensionIndex) const; SC_DLLPUBLIC SCCOL GetDimensionIndex(std::u16string_view sName) const; sal_uInt32 GetNumberFormat( tools::Long nDim ) const; SC_DLLPUBLIC bool IsDateDimension( tools::Long nDim ) const ; @@ -206,11 +207,11 @@ public: #if DUMP_PIVOT_TABLE void Dump() const; #endif + const GroupItems* GetGroupItems(tools::Long nDim) const; private: void PostInit(); void Clear(); - const GroupItems* GetGroupItems(tools::Long nDim) const; }; /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/qa/unit/PivotTableFormatsImportExport.cxx b/sc/qa/unit/PivotTableFormatsImportExport.cxx index 437a25d5e7e6..bbe3c251e22a 100644 --- a/sc/qa/unit/PivotTableFormatsImportExport.cxx +++ b/sc/qa/unit/PivotTableFormatsImportExport.cxx @@ -23,6 +23,8 @@ using namespace css; +namespace +{ class ScPivotTableFormatsImportExport : public ScModelTestBase { public: @@ -34,8 +36,6 @@ ScPivotTableFormatsImportExport::ScPivotTableFormatsImportExport() { } -namespace -{ ScAddress parseAddress(ScDocument& rDoc, OUString const& rAddressString) { ScAddress aAddress; @@ -82,9 +82,7 @@ template <typename T> OUString checkNonEmptyAddresses(ScDocument& rDoc, T const& return aString; } -} // end anonymous namespace - -static void assertDataFieldInRow_RowLabelColor(ScDocument& rDoc) +void assertDataFieldInRow_RowLabelColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"G6"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getFontColor(rDoc, u"G7"_ustr)); @@ -106,7 +104,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInRow_RowLabelColor(*getScDoc()); } -static void assertDataFieldInRow_ColumnLabelColor(ScDocument& rDoc) +void assertDataFieldInRow_ColumnLabelColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(Color(0x00B050), getBackgroundColor(rDoc, u"H5"_ustr)); @@ -128,7 +126,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInRow_ColumnLabelColor(*getScDoc()); } -static void assertDataFieldInColumn_ColumnLabelColor(ScDocument& rDoc) +void assertDataFieldInColumn_ColumnLabelColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getFontColor(rDoc, u"H4"_ustr)); CPPUNIT_ASSERT_EQUAL(Color(0x92D050), getBackgroundColor(rDoc, u"I4"_ustr)); @@ -151,7 +149,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInColumn_ColumnLabelColor(*getScDoc()); } -static void assertDataFieldInColumn_DataColor(ScDocument& rDoc) +void assertDataFieldInColumn_DataColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getFontColor(rDoc, u"H6"_ustr)); CPPUNIT_ASSERT_EQUAL(Color(0x92D050), getBackgroundColor(rDoc, u"I8"_ustr)); @@ -172,7 +170,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInColumn_DataColor(*getScDoc()); } -static void assertDataFieldInColumnAndTwoRowFields_DataColor(ScDocument& rDoc) +void assertDataFieldInColumnAndTwoRowFields_DataColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"I7"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"I10"_ustr)); @@ -199,7 +197,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInColumnAndTwoRowFields_DataColor(*getScDoc()); } -static void assertSingleDataFieldInColumn_DataColor(ScDocument& rDoc) +void assertSingleDataFieldInColumn_DataColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"J8"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"J12"_ustr)); @@ -215,7 +213,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertSingleDataFieldInColumn_DataColor(*getScDoc()); } -static void assertTwoRowTwoColumnFields_DataColor(ScDocument& rDoc) +void assertTwoRowTwoColumnFields_DataColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"I7"_ustr)); CPPUNIT_ASSERT_EQUAL(Color(0xFFC000), getBackgroundColor(rDoc, u"J8"_ustr)); @@ -235,7 +233,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertTwoRowTwoColumnFields_DataColor(*getScDoc()); } -static void assertDataFieldInRow_DataColor(ScDocument& rDoc) +void assertDataFieldInRow_DataColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(Color(0x00B0F0), getBackgroundColor(rDoc, u"I6"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"K7"_ustr)); @@ -257,7 +255,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertDataFieldInRow_DataColor(*getScDoc()); } -static void assertMultipleSelections(ScDocument& rDoc) +void assertMultipleSelections(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"I5"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"I6"_ustr)); @@ -295,7 +293,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, CPPUNIT_ASSERT_EQUAL(u"60"_ustr, rDoc.GetString(aAddress)); } -static void assertWholeDataColumnSelected(ScDocument& rDoc) +void assertWholeDataColumnSelected(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"G2"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"G3"_ustr)); @@ -321,7 +319,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertWholeDataColumnSelected(*getScDoc()); } -static void assertWholeLabelColumnSelected(ScDocument& rDoc) +void assertWholeLabelColumnSelected(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"F2"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"F3"_ustr)); @@ -347,7 +345,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertWholeLabelColumnSelected(*getScDoc()); } -static void assertSelectionInLabelAndData(ScDocument& rDoc) +void assertSelectionInLabelAndData(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"F5"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"G5"_ustr)); @@ -369,7 +367,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertSelectionInLabelAndData(*getScDoc()); } -static void assertTwoRowsDataFieldInColumn_LabelColor(ScDocument& rDoc) +void assertTwoRowsDataFieldInColumn_LabelColor(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"I4"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getBackgroundColor(rDoc, u"J4"_ustr)); @@ -396,7 +394,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertTwoRowsDataFieldInColumn_LabelColor(*getScDoc()); } -static void assertTwoDataFieldColumns_WholeDataColumnSelected(ScDocument& rDoc) +void assertTwoDataFieldColumns_WholeDataColumnSelected(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"H2"_ustr)); CPPUNIT_ASSERT_EQUAL(COL_YELLOW, getBackgroundColor(rDoc, u"H3"_ustr)); @@ -424,7 +422,7 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, assertTwoDataFieldColumns_WholeDataColumnSelected(*getScDoc()); } -static void assertFields_WithCellProtection(ScDocument& rDoc) +void assertFields_WithCellProtection(ScDocument& rDoc) { CPPUNIT_ASSERT_EQUAL(false, getCellProtection(rDoc, u"F18"_ustr)); CPPUNIT_ASSERT_EQUAL(false, getCellProtection(rDoc, u"F19"_ustr)); @@ -449,6 +447,15 @@ CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, Pivot_Table_with_Cell_Prot assertFields_WithCellProtection(*getScDoc()); } +CPPUNIT_TEST_FIXTURE(ScPivotTableFormatsImportExport, Pivot_Table_with_No_Source_Data) +{ + // We need to round-trip this document without crashing + createScDoc("xlsx/pivot-table/PivotTableWithNoSourceData.xlsx"); + saveAndReload(u"Calc Office Open XML"_ustr); +} + +} // end anonymous namespace + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/qa/unit/data/xlsx/pivot-table/PivotTableWithNoSourceData.xlsx b/sc/qa/unit/data/xlsx/pivot-table/PivotTableWithNoSourceData.xlsx new file mode 100644 index 000000000000..d28a70eda335 Binary files /dev/null and b/sc/qa/unit/data/xlsx/pivot-table/PivotTableWithNoSourceData.xlsx differ diff --git a/sc/source/core/data/PivotTableFormatOutput.cxx b/sc/source/core/data/PivotTableFormatOutput.cxx index c9073d6be07b..bc40739ff28c 100644 --- a/sc/source/core/data/PivotTableFormatOutput.cxx +++ b/sc/source/core/data/PivotTableFormatOutput.cxx @@ -25,29 +25,43 @@ namespace class NameResolver { private: - ScDPTableData& mrTableData; ScDPCache const& mrCache; std::unordered_map<sal_Int32, std::vector<OUString>> maNameCache; - void fillNamesForDimension(std::vector<OUString>& rNames, sal_Int32 nDimension) + void fillNamesForItems(std::vector<OUString>& rNames, ScDPCache::ScDPItemDataVec const& rItems, + sal_Int32 nDimension) { - for (const auto& rItemData : mrCache.GetDimMemberValues(nDimension)) + for (const auto& rItemData : rItems) { OUString sFormattedName; if (rItemData.HasStringData() || rItemData.IsEmpty()) sFormattedName = rItemData.GetString(); else - sFormattedName = ScDPObject::GetFormattedString(&mrTableData, nDimension, - rItemData.GetValue()); + sFormattedName = mrCache.GetFormattedString(nDimension, rItemData, false); rNames.push_back(sFormattedName); } } + void fillNamesForDimension(std::vector<OUString>& rNames, sal_Int32 nDimension) + { + if (mrCache.IsValidDimensionIndex(nDimension)) + { + fillNamesForItems(rNames, mrCache.GetDimMemberValues(nDimension), nDimension); + } + else + { + auto* pGroup = mrCache.GetGroupItems(nDimension); + if (pGroup) + { + fillNamesForItems(rNames, pGroup->maItems, nDimension); + } + } + } + public: - NameResolver(ScDPTableData& rTableData, ScDPCache const& rCache) - : mrTableData(rTableData) - , mrCache(rCache) + NameResolver(ScDPCache const& rCache) + : mrCache(rCache) { } @@ -165,7 +179,7 @@ void FormatOutput::prepare(SCTAB nTab, std::vector<ScDPOutLevelData> const& rCol ScDPFilteredCache const& rFilteredCache = pTableData->GetCacheTable(); ScDPCache const& rCache = rFilteredCache.getCache(); - NameResolver aNameResolver(*pTableData, rCache); + NameResolver aNameResolver(rCache); // Initialize format output entries (FormatOutputEntry) and set the data already available from output fields // (rColumnFields and rRowFields) and the pivot table format list (PivotTableFormat). diff --git a/sc/source/core/data/dpcache.cxx b/sc/source/core/data/dpcache.cxx index d5ee6bcb2483..709cd8777835 100644 --- a/sc/source/core/data/dpcache.cxx +++ b/sc/source/core/data/dpcache.cxx @@ -1049,13 +1049,15 @@ const ScDPCache::IndexArrayType* ScDPCache::GetFieldIndexArray( size_t nDim ) co const ScDPCache::ScDPItemDataVec& ScDPCache::GetDimMemberValues(SCCOL nDim) const { - OSL_ENSURE( nDim>=0 && nDim < mnColumnCount ," nDim < mnColumnCount "); + SAL_WARN_IF(!IsValidDimensionIndex(nDim) ,"sc.core", "dimension index out of bound"); + + assert(IsValidDimensionIndex(nDim)); return maFields.at(nDim)->maItems; } sal_uInt32 ScDPCache::GetNumberFormat( tools::Long nDim ) const { - if ( nDim >= mnColumnCount ) + if (!IsValidDimensionIndex(nDim)) return 0; // TODO: Find a way to determine the dominant number format in presence of @@ -1065,7 +1067,7 @@ sal_uInt32 ScDPCache::GetNumberFormat( tools::Long nDim ) const bool ScDPCache::IsDateDimension( tools::Long nDim ) const { - if (nDim >= mnColumnCount) + if (!IsValidDimensionIndex(nDim)) return false; ScInterpreterContext& rContext = mrDoc.GetNonThreadedContext(); @@ -1075,7 +1077,11 @@ bool ScDPCache::IsDateDimension( tools::Long nDim ) const tools::Long ScDPCache::GetDimMemberCount(tools::Long nDim) const { - OSL_ENSURE( nDim>=0 && nDim < mnColumnCount ," ScDPTableDataCache::GetDimMemberCount : out of bound "); + SAL_WARN_IF(!IsValidDimensionIndex(nDim) ,"sc.core", "dimension index out of bound"); + + if (!IsValidDimensionIndex(nDim)) + return 0; + return maFields[nDim]->maItems.size(); } @@ -1089,6 +1095,11 @@ SCCOL ScDPCache::GetDimensionIndex(std::u16string_view sName) const return -1; } +bool ScDPCache::IsValidDimensionIndex(tools::Long nDimensionIndex) const +{ + return nDimensionIndex >= 0 && nDimensionIndex < mnColumnCount; +} + rtl_uString* ScDPCache::InternString( size_t nDim, const OUString& rStr ) { assert(nDim < maStringPools.size());
