sc/inc/colorscale.hxx | 8 +++---- sc/source/core/data/colorscale.cxx | 40 +++++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 16 deletions(-)
New commits: commit d833d5d72eea3defe7bf1d741f52d0f3c82f971f Author: Caolán McNamara <[email protected]> AuthorDate: Wed Oct 22 11:12:18 2025 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Wed Oct 22 14:04:56 2025 +0200 tdf#168979 survive broken color percentile scales Change-Id: I3180d08272fd7355987f63f5379a044d98467cf0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192838 Reviewed-by: Caolán McNamara <[email protected]> Tested-by: Jenkins diff --git a/sc/inc/colorscale.hxx b/sc/inc/colorscale.hxx index cd42c67b6513..9c0e8272e1ff 100644 --- a/sc/inc/colorscale.hxx +++ b/sc/inc/colorscale.hxx @@ -260,7 +260,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& rDoc); ScColorScaleFormat(ScDocument& rDoc, const ScColorScaleFormat& rFormat); @@ -331,8 +331,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; }; @@ -412,7 +412,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 82fce7832a2b..e9c15efaa63f 100644 --- a/sc/source/core/data/colorscale.cxx +++ b/sc/source/core/data/colorscale.cxx @@ -612,9 +612,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(); @@ -638,7 +644,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()) { @@ -689,20 +695,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; @@ -713,7 +715,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; } @@ -911,7 +913,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()) { @@ -938,7 +940,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()) { @@ -972,8 +974,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; @@ -1197,7 +1204,7 @@ std::unique_ptr<ScIconSetInfo> ScIconSetFormat::GetIconSetInfo(const ScAddress& int i = 0; while(itr != end()) { - nValRef = CalcValue(nMin, nMax, itr); + nValRef = *CalcValue(nMin, nMax, itr); if (Compare(nVal, nValRef, itr)) { nIndex = i; @@ -1325,7 +1332,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 bde11fe8295fe33c4f02aa79dfbeb7ce2604b11e Author: Caolán McNamara <[email protected]> AuthorDate: Wed Oct 22 08:43:36 2025 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Wed Oct 22 14:04:48 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]> diff --git a/sc/source/core/data/colorscale.cxx b/sc/source/core/data/colorscale.cxx index 5891c10566f1..82fce7832a2b 100644 --- a/sc/source/core/data/colorscale.cxx +++ b/sc/source/core/data/colorscale.cxx @@ -688,6 +688,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;
