sc/inc/colorscale.hxx | 8 +++--- sc/source/core/data/colorscale.cxx | 45 ++++++++++++++++++++++++++----------- 2 files changed, 36 insertions(+), 17 deletions(-)
New commits: commit 9c053a2221480acd2e9c1c87dc8fe58c40d84f29 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Oct 22 11:12:18 2025 +0100 Commit: Xisco Fauli <[email protected]> CommitDate: Wed Oct 22 17:14:20 2025 +0200 tdf#168979 survive broken color percentile scales Change-Id: I3180d08272fd7355987f63f5379a044d98467cf0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192855 Tested-by: Jenkins Reviewed-by: Xisco Fauli <[email protected]> diff --git a/sc/inc/colorscale.hxx b/sc/inc/colorscale.hxx index 373c06eda05c..2f7d633c1416 100644 --- a/sc/inc/colorscale.hxx +++ b/sc/inc/colorscale.hxx @@ -256,7 +256,7 @@ private: double GetMaxValue() const; void calcMinMax(double& nMin, double& nMax) const; - double CalcValue(double nMin, double nMax, const ScColorScaleEntries::const_iterator& rItr) const; + std::optional<double> CalcValue(double nMin, double nMax, const ScColorScaleEntries::const_iterator& rItr) const; public: SC_DLLPUBLIC ScColorScaleFormat(ScDocument* pDoc); ScColorScaleFormat(ScDocument* pDoc, const ScColorScaleFormat& rFormat); @@ -327,8 +327,8 @@ public: void EnsureSize(); private: - double getMin(double nMin, double nMax) const; - double getMax(double nMin, double nMax) const; + std::optional<double> getMin(double nMin, double nMax) const; + std::optional<double> getMax(double nMin, double nMax) const; std::unique_ptr<ScDataBarFormatData> mpFormatData; }; @@ -408,7 +408,7 @@ private: double GetMinValue() const; double GetMaxValue() const; - double CalcValue(double nMin, double nMax, const ScIconSetFormat::const_iterator& itr) const; + std::optional<double> CalcValue(double nMin, double nMax, const ScIconSetFormat::const_iterator& itr) const; std::unique_ptr<ScIconSetFormatData> mpFormatData; }; diff --git a/sc/source/core/data/colorscale.cxx b/sc/source/core/data/colorscale.cxx index e947857b7497..4625888184ac 100644 --- a/sc/source/core/data/colorscale.cxx +++ b/sc/source/core/data/colorscale.cxx @@ -606,9 +606,15 @@ Color CalcColor( double nVal, double nVal1, const Color& rCol1, double nVal2, co * @param rVector sorted vector of the array * @param fPercentile percentile */ -double GetPercentile( const std::vector<double>& rArray, double fPercentile ) +std::optional<double> GetPercentile( const std::vector<double>& rArray, double fPercentile ) { - assert(!rArray.empty()); + if (rArray.empty()) + { + // An empty getValues() is invalid for COLORSCALE_PERCENTILE. + SAL_WARN("sc", "empty COLORSCALE_PERCENTILE"); + return std::optional<double>(); + } + SAL_WARN_IF(fPercentile < 0, "sc", "negative percentile"); if (fPercentile < 0) return rArray.front(); @@ -632,7 +638,7 @@ double GetPercentile( const std::vector<double>& rArray, double fPercentile ) } -double ScColorScaleFormat::CalcValue(double nMin, double nMax, const ScColorScaleEntries::const_iterator& itr) const +std::optional<double> ScColorScaleFormat::CalcValue(double nMin, double nMax, const ScColorScaleEntries::const_iterator& itr) const { switch((*itr)->GetType()) { @@ -683,20 +689,16 @@ std::optional<Color> ScColorScaleFormat::GetColor( const ScAddress& rAddr ) cons ScColorScaleEntries::const_iterator itr = begin(); - // CalcValue will call GetPercentile for COLORSCALE_PERCENTILE. An empty - // getValues() is invalid for COLORSCALE_PERCENTILE. - if ((*itr)->GetType() == COLORSCALE_PERCENTILE && getValues().empty()) - { - SAL_WARN("sc", "empty COLORSCALE_PERCENTILE"); - return std::optional<Color>(); - } - - double nValMin = CalcValue(nMin, nMax, itr); + std::optional<double> aValMin = CalcValue(nMin, nMax, itr); Color rColMin = (*itr)->GetColor(); ++itr; - double nValMax = CalcValue(nMin, nMax, itr); + std::optional<double> aValMax = CalcValue(nMin, nMax, itr); Color rColMax = (*itr)->GetColor(); + if (!aValMin || !aValMax) + return std::optional<Color>(); + double nValMin(*aValMin), nValMax(*aValMax); + // tdf#155321 for the last percentile value, use always the end of the color scale, // i.e. not the first possible color in the case of repeating values bool bEqual = COLORSCALE_PERCENTILE == (*itr)->GetType() && nVal == nMax && nVal == nValMax; @@ -707,7 +709,7 @@ std::optional<Color> ScColorScaleFormat::GetColor( const ScAddress& rAddr ) cons rColMin = rColMax; nValMin = !bEqual ? nValMax : nValMax - 1; rColMax = (*itr)->GetColor(); - nValMax = CalcValue(nMin, nMax, itr); + nValMax = *CalcValue(nMin, nMax, itr); ++itr; } @@ -905,7 +907,7 @@ void ScDataBarFormat::UpdateMoveTab( sc::RefUpdateMoveTabContext& rCxt ) mpFormatData->mpLowerLimit->UpdateMoveTab(rCxt); } -double ScDataBarFormat::getMin(double nMin, double nMax) const +std::optional<double> ScDataBarFormat::getMin(double nMin, double nMax) const { switch(mpFormatData->mpLowerLimit->GetType()) { @@ -932,7 +934,7 @@ double ScDataBarFormat::getMin(double nMin, double nMax) const return mpFormatData->mpLowerLimit->GetValue(); } -double ScDataBarFormat::getMax(double nMin, double nMax) const +std::optional<double> ScDataBarFormat::getMax(double nMin, double nMax) const { switch(mpFormatData->mpUpperLimit->GetType()) { @@ -966,8 +968,13 @@ std::unique_ptr<ScDataBarInfo> ScDataBarFormat::GetDataBarInfo(const ScAddress& double nValMin = getMinValue(); double nValMax = getMaxValue(); - double nMin = getMin(nValMin, nValMax); - double nMax = getMax(nValMin, nValMax); + std::optional<double> aMin = getMin(nValMin, nValMax); + std::optional<double> aMax = getMax(nValMin, nValMax); + if (!aMin || !aMax) + return nullptr; + + double nMin(*aMin), nMax(*aMax); + double nMinLength = mpFormatData->mnMinLength; double nMaxLength = mpFormatData->mnMaxLength; @@ -1165,7 +1172,10 @@ std::unique_ptr<ScIconSetInfo> ScIconSetFormat::GetIconSetInfo(const ScAddress& sal_Int32 nIndex = 0; const_iterator itr = begin(); ++itr; - double nValMax = CalcValue(nMin, nMax, itr); + std::optional<double> aValMax = CalcValue(nMin, nMax, itr); + if (!aValMax) + return nullptr; + double nValMax(*aValMax); ++itr; bool bGreaterThanOrEqual = true; @@ -1173,7 +1183,7 @@ std::unique_ptr<ScIconSetInfo> ScIconSetFormat::GetIconSetInfo(const ScAddress& { bGreaterThanOrEqual = (*itr)->GetGreaterThanOrEqual(); ++nIndex; - nValMax = CalcValue(nMin, nMax, itr); + nValMax = *CalcValue(nMin, nMax, itr); ++itr; } @@ -1303,7 +1313,7 @@ double ScIconSetFormat::GetMaxValue() const } } -double ScIconSetFormat::CalcValue(double nMin, double nMax, const ScIconSetFormat::const_iterator& itr) const +std::optional<double> ScIconSetFormat::CalcValue(double nMin, double nMax, const ScIconSetFormat::const_iterator& itr) const { switch ((*itr)->GetType()) { commit 08e79be8d0d73e8c681cf89ebc1a09b70140bd34 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Oct 22 08:43:36 2025 +0100 Commit: Xisco Fauli <[email protected]> CommitDate: Wed Oct 22 17:14:14 2025 +0200 crash in GetPercentile with empty std::vector first arg /opt/collaboraoffice/program/../program/libsclo.so (anonymous namespace)::GetPercentile(std::vector<double, std::allocator<double> > const&, double) sc/source/core/data/colorscale.cxx:624 /opt/collaboraoffice/program/../program/libsclo.so __gnu_cxx::__normal_iterator<std::unique_ptr<ScColorScaleEntry, o3tl::default_delete<ScColorScaleEntry> > const*, std::vector<std::unique_ptr<ScColorScaleEntry, o3tl::default_delete<ScColorScaleEntry> >, std::allocator<std::unique_ptr<ScColorScaleEntry, o3tl::default_delete<ScColorScaleEntry> > > > >::operator*() const /opt/rh/devtoolset-12/root/usr/include/c++/12/bits/stl_iterator.h:1096 /opt/collaboraoffice/program/libscfiltlo.so ScHTMLExport::WriteCell(sc::ColumnBlockPosition&, short, int, short) sc/source/filter/html/htmlexp.cxx:923 /opt/collaboraoffice/program/libscfiltlo.so ScHTMLExport::WriteTables() sc/source/filter/html/htmlexp.cxx:864 /opt/collaboraoffice/program/libscfiltlo.so ScHTMLExport::WriteBody() sc/source/filter/html/htmlexp.cxx:699 /opt/collaboraoffice/program/libscfiltlo.so ScHTMLExport::Write() sc/source/filter/html/htmlexp.cxx:313 /opt/collaboraoffice/program/libscfiltlo.so rtl::OUString::operator=(rtl::OUString const&) include/rtl/ustring.hxx:586 /opt/collaboraoffice/program/../program/libsclo.so ErrCode::IgnoreWarning() const include/comphelper/errcode.hxx:102 (discriminator 1) /opt/collaboraoffice/program/../program/libsclo.so rtl::OUString::~OUString() include/rtl/ustring.hxx:546 /opt/collaboraoffice/program/libmergedlo.so TransferableHelper::SetObject(void*, unsigned int, com::sun::star::datatransfer::DataFlavor const&) vcl/source/treelist/transfer.cxx:912 (discriminator 1) /opt/collaboraoffice/program/../program/libsclo.so ScTransferObj::GetData(com::sun::star::datatransfer::DataFlavor const&, rtl::OUString const&) sc/source/ui/app/transobj.cxx:395 ScColorScaleFormat::CalcValue is optimized out, but it has to be case COLORSCALE_PERCENTILE and getValues() must return an empty vector Change-Id: I2cf09f19edcb5f30c82eca02e2a0fe95e86c78db Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192823 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192837 Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192844 Tested-by: Jenkins Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192854 diff --git a/sc/source/core/data/colorscale.cxx b/sc/source/core/data/colorscale.cxx index 0947971a73ee..e947857b7497 100644 --- a/sc/source/core/data/colorscale.cxx +++ b/sc/source/core/data/colorscale.cxx @@ -682,6 +682,15 @@ std::optional<Color> ScColorScaleFormat::GetColor( const ScAddress& rAddr ) cons return std::optional<Color>(); ScColorScaleEntries::const_iterator itr = begin(); + + // CalcValue will call GetPercentile for COLORSCALE_PERCENTILE. An empty + // getValues() is invalid for COLORSCALE_PERCENTILE. + if ((*itr)->GetType() == COLORSCALE_PERCENTILE && getValues().empty()) + { + SAL_WARN("sc", "empty COLORSCALE_PERCENTILE"); + return std::optional<Color>(); + } + double nValMin = CalcValue(nMin, nMax, itr); Color rColMin = (*itr)->GetColor(); ++itr;
