sc/source/filter/inc/defnamesbuffer.hxx | 2 +- sc/source/filter/inc/workbookhelper.hxx | 7 +++++-- sc/source/filter/oox/defnamesbuffer.cxx | 23 ++++++++++++----------- sc/source/filter/oox/workbookhelper.cxx | 32 +++++++++++++++++--------------- 4 files changed, 35 insertions(+), 29 deletions(-)
New commits: commit 9bf1089965672d3825b587cbd888093ac362013e Author: Caolán McNamara <caol...@redhat.com> AuthorDate: Mon May 31 09:30:52 2021 +0100 Commit: Caolán McNamara <caol...@redhat.com> CommitDate: Mon May 31 11:46:32 2021 +0200 crashtesting: use after free on export of tdf122510-1.xlsx to ods and also tdf95549-3.xlsm related to: commit f8c1048eb437b1e685b76198165844e2ecc97a56 Date: Wed May 19 19:04:02 2021 +0200 fix leak in oox import and: commit 3cd6402c5443c8069c07d9e420d5ef5b43af6bef Date: Thu May 6 18:47:30 2021 +0200 tdf#127301 XLSX import: hide hidden named range of autofilter clearly this is fragile so just explicitly return who owns the ScRangeData* Change-Id: Ic3210bb8788bbbc85609bb384fa4a4625c15e487 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/116432 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caol...@redhat.com> diff --git a/sc/source/filter/inc/defnamesbuffer.hxx b/sc/source/filter/inc/defnamesbuffer.hxx index 03eec97af201..3200bd5dc8de 100644 --- a/sc/source/filter/inc/defnamesbuffer.hxx +++ b/sc/source/filter/inc/defnamesbuffer.hxx @@ -122,7 +122,7 @@ public: private: typedef ::std::unique_ptr< StreamDataSequence > StreamDataSeqPtr; - ScRangeData* mpScRangeData; /// ScRangeData of the defined name. + RangeDataRet maScRangeData; /// ScRangeData of the defined name. sal_Int32 mnTokenIndex; /// Name index used in API token array. sal_Int16 mnCalcSheet; /// Calc sheet index for sheet-local names. sal_Unicode mcBuiltinId; /// Identifier for built-in defined names. diff --git a/sc/source/filter/inc/workbookhelper.hxx b/sc/source/filter/inc/workbookhelper.hxx index e6633d15f4f9..d6d04007d39c 100644 --- a/sc/source/filter/inc/workbookhelper.hxx +++ b/sc/source/filter/inc/workbookhelper.hxx @@ -160,10 +160,13 @@ public: css::uno::Reference< css::style::XStyle > getStyleObject( const OUString& rStyleName, bool bPageStyle ) const; + // second is true if ownership belongs to the caller + typedef std::pair<ScRangeData*, bool> RangeDataRet; + /** Creates and returns a defined name on-the-fly in the Calc document. The name will not be buffered in the global defined names buffer. @param orName (in/out-parameter) Returns the resulting used name. */ - ScRangeData* createNamedRangeObject( + RangeDataRet createNamedRangeObject( OUString& orName, const css::uno::Sequence< css::sheet::FormulaToken>& rTokens, sal_Int32 nIndex, @@ -172,7 +175,7 @@ public: /** Creates and returns a defined name on-the-fly in the sheet. The name will not be buffered in the global defined names buffer. @param orName (in/out-parameter) Returns the resulting used name. */ - ScRangeData* createLocalNamedRangeObject( + RangeDataRet createLocalNamedRangeObject( OUString& orName, const css::uno::Sequence< css::sheet::FormulaToken>& rTokens, sal_Int32 nIndex, diff --git a/sc/source/filter/oox/defnamesbuffer.cxx b/sc/source/filter/oox/defnamesbuffer.cxx index a554ab9cd0f1..9eebaf1633a7 100644 --- a/sc/source/filter/oox/defnamesbuffer.cxx +++ b/sc/source/filter/oox/defnamesbuffer.cxx @@ -146,7 +146,7 @@ const OUString& DefinedNameBase::getUpcaseModelName() const DefinedName::DefinedName( const WorkbookHelper& rHelper ) : DefinedNameBase( rHelper ), - mpScRangeData(nullptr), + maScRangeData(nullptr, false), mnTokenIndex( -1 ), mnCalcSheet( 0 ), mcBuiltinId( BIFF_DEFNAME_UNKNOWN ) @@ -233,9 +233,9 @@ void DefinedName::createNameObject( sal_Int32 nIndex ) // create the name and insert it into the document, maCalcName will be changed to the resulting name if (maModel.mnSheet >= 0) - mpScRangeData = createLocalNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mnSheet, maModel.mbHidden ); + maScRangeData = createLocalNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mnSheet, maModel.mbHidden ); else - mpScRangeData = createNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mbHidden ); + maScRangeData = createNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mbHidden ); mnTokenIndex = nIndex; } @@ -259,17 +259,18 @@ std::unique_ptr<ScTokenArray> DefinedName::getScTokens( void DefinedName::convertFormula( const css::uno::Sequence<css::sheet::ExternalLinkInfo>& rExternalLinks ) { + ScRangeData* pScRangeData = maScRangeData.first; // macro function or vba procedure - if(!mpScRangeData) + if (!pScRangeData) return; // convert and set formula of the defined name { std::unique_ptr<ScTokenArray> pTokenArray = getScTokens( rExternalLinks); - mpScRangeData->SetCode( *pTokenArray ); + pScRangeData->SetCode( *pTokenArray ); } - ScTokenArray* pTokenArray = mpScRangeData->GetCode(); + ScTokenArray* pTokenArray = pScRangeData->GetCode(); Sequence< FormulaToken > aFTokenSeq; ScTokenConversion::ConvertToTokenSequence( getScDocument(), aFTokenSeq, *pTokenArray ); // set built-in names (print ranges, repeated titles, filter ranges) @@ -327,7 +328,8 @@ void DefinedName::convertFormula( const css::uno::Sequence<css::sheet::ExternalL bool DefinedName::getAbsoluteRange( ScRange& orRange ) const { - ScTokenArray* pTokenArray = mpScRangeData->GetCode(); + ScRangeData* pScRangeData = maScRangeData.first; + ScTokenArray* pTokenArray = pScRangeData->GetCode(); Sequence< FormulaToken > aFTokenSeq; ScTokenConversion::ConvertToTokenSequence(getScDocument(), aFTokenSeq, *pTokenArray); return getFormulaParser().extractCellRange( orRange, aFTokenSeq ); @@ -336,12 +338,11 @@ bool DefinedName::getAbsoluteRange( ScRange& orRange ) const DefinedName::~DefinedName() { // this kind of field is owned by us - see lcl_addNewByNameAndTokens - if (mpScRangeData && maModel.mbHidden && - (mcBuiltinId == BIFF_DEFNAME_CRITERIA || mcBuiltinId == BIFF_DEFNAME_FILTERDATABASE)) - delete mpScRangeData; + bool bOwned = maScRangeData.second; + if (bOwned) + delete maScRangeData.first; } - DefinedNamesBuffer::DefinedNamesBuffer( const WorkbookHelper& rHelper ) : WorkbookHelper( rHelper ) { diff --git a/sc/source/filter/oox/workbookhelper.cxx b/sc/source/filter/oox/workbookhelper.cxx index 8c7ffb3de4d8..8a7804102246 100644 --- a/sc/source/filter/oox/workbookhelper.cxx +++ b/sc/source/filter/oox/workbookhelper.cxx @@ -149,9 +149,9 @@ public: /** Returns the specified cell or page style from the Calc document. */ Reference< XStyle > getStyleObject( const OUString& rStyleName, bool bPageStyle ) const; /** Creates and returns a defined name on-the-fly in the Calc document. */ - ScRangeData* createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ); + WorkbookHelper::RangeDataRet createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ); /** Creates and returns a defined name on the-fly in the correct Calc sheet. */ - ScRangeData* createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ); + WorkbookHelper::RangeDataRet createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ); /** Creates and returns a database range on-the-fly in the Calc document. */ Reference< XDatabaseRange > createDatabaseRangeObject( OUString& orName, const ScRange& rRangeAddr ); /** Creates and returns an unnamed database range on-the-fly in the Calc document. */ @@ -350,7 +350,7 @@ Reference< XStyle > WorkbookGlobals::getStyleObject( const OUString& rStyleName, namespace { -ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& rName, const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType, bool bHidden ) +WorkbookHelper::RangeDataRet lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& rName, const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType, bool bHidden ) { bool bDone = false; ScRangeData::Type nNewType = ScRangeData::Type::Name; @@ -366,7 +366,9 @@ ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, c pNew->SetIndex( nIndex ); // create but not insert hidden FILTER_CRITERIA named ranges to ScRangeName if ( bHidden && nNewType == ScRangeData::Type::Criteria ) - return pNew; + { + return WorkbookHelper::RangeDataRet(pNew, true); + } if ( pNames->insert(pNew) ) bDone = true; if (!bDone) @@ -374,7 +376,7 @@ ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, c delete pNew; throw RuntimeException(); } - return pNew; + return WorkbookHelper::RangeDataRet(pNew, false); } OUString findUnusedName( const ScRangeName* pRangeName, const OUString& rSuggestedName ) @@ -389,11 +391,11 @@ OUString findUnusedName( const ScRangeName* pRangeName, const OUString& rSuggest } -ScRangeData* WorkbookGlobals::createNamedRangeObject( +WorkbookHelper::RangeDataRet WorkbookGlobals::createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ) { // create the name and insert it into the Calc document - ScRangeData* pScRangeData = nullptr; + WorkbookHelper::RangeDataRet aScRangeData(nullptr, false); if( !orName.isEmpty() ) { ScDocument& rDoc = getScDocument(); @@ -401,16 +403,16 @@ ScRangeData* WorkbookGlobals::createNamedRangeObject( // find an unused name orName = findUnusedName( pNames, orName ); // create the named range - pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden ); + aScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden ); } - return pScRangeData; + return aScRangeData; } -ScRangeData* WorkbookGlobals::createLocalNamedRangeObject( +WorkbookHelper::RangeDataRet WorkbookGlobals::createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken >& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ) { // create the name and insert it into the Calc document - ScRangeData* pScRangeData = nullptr; + WorkbookHelper::RangeDataRet aScRangeData(nullptr, false); if( !orName.isEmpty() ) { ScDocument& rDoc = getScDocument(); @@ -420,9 +422,9 @@ ScRangeData* WorkbookGlobals::createLocalNamedRangeObject( // find an unused name orName = findUnusedName( pNames, orName ); // create the named range - pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden ); + aScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden ); } - return pScRangeData; + return aScRangeData; } Reference< XDatabaseRange > WorkbookGlobals::createDatabaseRangeObject( OUString& orName, const ScRange& rRangeAddr ) @@ -865,12 +867,12 @@ Reference< XStyle > WorkbookHelper::getStyleObject( const OUString& rStyleName, return mrBookGlob.getStyleObject( rStyleName, bPageStyle ); } -ScRangeData* WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ) const +WorkbookHelper::RangeDataRet WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ) const { return mrBookGlob.createNamedRangeObject( orName, rTokens, nIndex, nNameFlags, bHidden ); } -ScRangeData* WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ) const +WorkbookHelper::RangeDataRet WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ) const { return mrBookGlob.createLocalNamedRangeObject( orName, rTokens, nIndex, nNameFlags, nTab, bHidden ); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits