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 5aaedc8c6accb93475c0e83b1fac4cc9bb76c43d Author: Tomaž Vajngerl <[email protected]> AuthorDate: Wed Nov 12 18:59:58 2025 +0900 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Thu Nov 13 13:23:27 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 diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx index 9e7e3397f69f..f74258599beb 100644 --- a/sc/inc/dpcache.hxx +++ b/sc/inc/dpcache.hxx @@ -162,6 +162,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 ; @@ -205,11 +206,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 caf95dd1785f..ac7215a2ec5b 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 098bcafa8dc5..a4d38a91d3ad 100644 --- a/sc/source/core/data/dpcache.cxx +++ b/sc/source/core/data/dpcache.cxx @@ -1047,13 +1047,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 @@ -1063,7 +1065,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(); @@ -1073,7 +1075,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(); } @@ -1087,6 +1093,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());
