sc/inc/address.hxx | 26 +++++++++++++------------- sc/qa/unit/data/xlsx/invalid-named-range.xlsx |binary sc/qa/unit/subsequent_export-test2.cxx | 19 +++++++++++++++++++ sc/source/filter/inc/defnamesbuffer.hxx | 1 + sc/source/filter/oox/defnamesbuffer.cxx | 22 ++++++++++++++++++++++ 5 files changed, 55 insertions(+), 13 deletions(-)
New commits: commit db1c8df98a23d687d6806f371bdd416dd1b84589 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Jul 5 12:29:18 2021 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Mon Jul 5 14:04:11 2021 +0200 XLSX import: fix handling of named ranges referring to PathMissing sheets In case xl/externalLinks/externalLink1.xml refers to a sheet where type is PathMissing, then both <sheetName> and <sheetData> gets ignored on import. Make sure to also ignore named ranges referring to such external documents. The resulting named range was just a string anyway, and exporting this back to XLSX results in Excel marking the whole file as corrupted. Change-Id: Ifde07b5e59fba371f1f8ab3e82861c6997c6dbf0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118401 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx index f7e928b88b6a..2f3a987b45bc 100644 --- a/sc/inc/address.hxx +++ b/sc/inc/address.hxx @@ -493,7 +493,7 @@ struct ScAddressHashFunctor } // ScRange -class SAL_WARN_UNUSED ScRange final +class SAL_WARN_UNUSED SC_DLLPUBLIC ScRange final { public: ScAddress aStart; @@ -550,18 +550,18 @@ public: inline bool In( const ScAddress& ) const; ///< is Address& in Range? inline bool In( const ScRange& ) const; ///< is Range& in Range? - SC_DLLPUBLIC ScRefFlags Parse( const OUString&, const ScDocument&, + ScRefFlags Parse( const OUString&, const ScDocument&, const ScAddress::Details& rDetails = ScAddress::detailsOOOa1, ScAddress::ExternalInfo* pExtInfo = nullptr, const css::uno::Sequence<css::sheet::ExternalLinkInfo>* pExternalLinks = nullptr, const OUString* pErrRef = nullptr ); - SC_DLLPUBLIC ScRefFlags ParseAny( const OUString&, const ScDocument&, + ScRefFlags ParseAny( const OUString&, const ScDocument&, const ScAddress::Details& rDetails = ScAddress::detailsOOOa1 ); - SC_DLLPUBLIC ScRefFlags ParseCols( const ScDocument& rDoc, + ScRefFlags ParseCols( const ScDocument& rDoc, const OUString&, const ScAddress::Details& rDetails = ScAddress::detailsOOOa1 ); - SC_DLLPUBLIC void ParseRows( const ScDocument& rDoc, + void ParseRows( const ScDocument& rDoc, const OUString&, const ScAddress::Details& rDetails = ScAddress::detailsOOOa1 ); @@ -613,14 +613,14 @@ public: @returns String contains formatted cell range in address convention */ - SC_DLLPUBLIC OUString Format( const ScDocument& rDocument, + OUString Format( const ScDocument& rDocument, ScRefFlags nFlags = ScRefFlags::ZERO, const ScAddress::Details& rDetails = ScAddress::detailsOOOa1, bool bFullAddressNotation = false ) const; inline void GetVars( SCCOL& nCol1, SCROW& nRow1, SCTAB& nTab1, SCCOL& nCol2, SCROW& nRow2, SCTAB& nTab2 ) const; - SC_DLLPUBLIC void PutInOrder(); + void PutInOrder(); /** @param rErrorRange @@ -629,18 +629,18 @@ public: @param pDocument The document for the maximum defined sheet number. */ - [[nodiscard]] SC_DLLPUBLIC bool Move( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ, + [[nodiscard]] bool Move( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ, ScRange& rErrorRange, const ScDocument* pDocument = nullptr ); /** Same as Move() but with sticky end col/row anchors. */ - [[nodiscard]] SC_DLLPUBLIC bool MoveSticky( const ScDocument& rDoc, SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ, + [[nodiscard]] bool MoveSticky( const ScDocument& rDoc, SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ, ScRange& rErrorRange ); - SC_DLLPUBLIC void IncColIfNotLessThan(const ScDocument& rDoc, SCCOL nStartCol, SCCOL nOffset); - SC_DLLPUBLIC void IncRowIfNotLessThan(const ScDocument& rDoc, SCROW nStartRow, SCROW nOffset); + void IncColIfNotLessThan(const ScDocument& rDoc, SCCOL nStartCol, SCCOL nOffset); + void IncRowIfNotLessThan(const ScDocument& rDoc, SCROW nStartRow, SCROW nOffset); - SC_DLLPUBLIC void ExtendTo( const ScRange& rRange ); - SC_DLLPUBLIC bool Intersects( const ScRange& rRange ) const; // do two ranges intersect? + void ExtendTo( const ScRange& rRange ); + bool Intersects( const ScRange& rRange ) const; // do two ranges intersect? ScRange Intersection( const ScRange& rOther ) const; diff --git a/sc/qa/unit/data/xlsx/invalid-named-range.xlsx b/sc/qa/unit/data/xlsx/invalid-named-range.xlsx new file mode 100644 index 000000000000..53feabec382f Binary files /dev/null and b/sc/qa/unit/data/xlsx/invalid-named-range.xlsx differ diff --git a/sc/qa/unit/subsequent_export-test2.cxx b/sc/qa/unit/subsequent_export-test2.cxx index d23145c67cf2..ad08aa2802d0 100644 --- a/sc/qa/unit/subsequent_export-test2.cxx +++ b/sc/qa/unit/subsequent_export-test2.cxx @@ -189,6 +189,7 @@ public: void testTdf140431(); void testCheckboxFormControlXlsxExport(); void testButtonFormControlXlsxExport(); + void testInvalidNamedRange(); CPPUNIT_TEST_SUITE(ScExportTest2); @@ -286,6 +287,7 @@ public: CPPUNIT_TEST(testTdf140431); CPPUNIT_TEST(testCheckboxFormControlXlsxExport); CPPUNIT_TEST(testButtonFormControlXlsxExport); + CPPUNIT_TEST(testInvalidNamedRange); CPPUNIT_TEST_SUITE_END(); @@ -2348,6 +2350,23 @@ void ScExportTest2::testButtonFormControlXlsxExport() assertXPathContent(pDoc, "//x:anchor/x:to/xdr:row", "7"); } +void ScExportTest2::testInvalidNamedRange() +{ + // Given a document which has a named range (myname) that refers to the "1" external link, but + // the link's type is xlPathMissing, when importing that document: + ScDocShellRef xDocSh = loadDoc(u"invalid-named-range.", FORMAT_XLSX); + CPPUNIT_ASSERT(xDocSh.is()); + + // Then make sure that named range is ignored, as "1" can't be resolved, and exporting it back + // to XLSX (without the xlPathMissing link) would corrupt the document: + uno::Reference<beans::XPropertySet> xDocProps(xDocSh->GetModel(), uno::UNO_QUERY); + uno::Reference<container::XNameAccess> xNamedRanges(xDocProps->getPropertyValue("NamedRanges"), + uno::UNO_QUERY); + // Without the fix in place, this test would have failed, we didn't ignore the problematic named + // range on import. + CPPUNIT_ASSERT(!xNamedRanges->hasByName("myname")); +} + CPPUNIT_TEST_SUITE_REGISTRATION(ScExportTest2); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sc/source/filter/inc/defnamesbuffer.hxx b/sc/source/filter/inc/defnamesbuffer.hxx index c6a553a999b6..ade123682711 100644 --- a/sc/source/filter/inc/defnamesbuffer.hxx +++ b/sc/source/filter/inc/defnamesbuffer.hxx @@ -118,6 +118,7 @@ public: sal_Int32 getTokenIndex() const { return mnTokenIndex; } /** Tries to resolve the defined name to an absolute cell range. */ bool getAbsoluteRange( ScRange& orRange ) const; + bool isValid(const css::uno::Sequence<css::sheet::ExternalLinkInfo>& rExternalLinks) const; private: typedef ::std::unique_ptr< StreamDataSequence > StreamDataSeqPtr; diff --git a/sc/source/filter/oox/defnamesbuffer.cxx b/sc/source/filter/oox/defnamesbuffer.cxx index 9eebaf1633a7..72f28cd4edd3 100644 --- a/sc/source/filter/oox/defnamesbuffer.cxx +++ b/sc/source/filter/oox/defnamesbuffer.cxx @@ -239,6 +239,23 @@ void DefinedName::createNameObject( sal_Int32 nIndex ) mnTokenIndex = nIndex; } +bool DefinedName::isValid( + const css::uno::Sequence<css::sheet::ExternalLinkInfo>& rExternalLinks) const +{ + ScRange aRange; + OUString aExternDocName; + OUString aStartTabName; + OUString aEndTabName; + ScRefFlags nFlags = ScRefFlags::VALID | ScRefFlags::TAB_VALID; + aRange.Parse_XL_Header(maModel.maFormula.getStr(), getScDocument(), aExternDocName, + aStartTabName, aEndTabName, nFlags, /*bOnlyAcceptSingle=*/false, + &rExternalLinks); + // aExternDocName is something like 'file:///path/to/my.xlsx' in the valid case, and it's an int + // when it's invalid. + bool bInvalidExternalRef = aExternDocName.toInt32() > 0; + return !bInvalidExternalRef; +} + std::unique_ptr<ScTokenArray> DefinedName::getScTokens( const css::uno::Sequence<css::sheet::ExternalLinkInfo>& rExternalLinks ) { @@ -366,6 +383,11 @@ void DefinedNamesBuffer::finalizeImport() int index = 0; for( DefinedNameRef& xDefName : maDefNames ) { + if (!xDefName->isValid(getExternalLinks().getLinkInfos())) + { + continue; + } + xDefName->createNameObject( ++index ); // map by sheet index and original model name maModelNameMap[ SheetNameKey( xDefName->getLocalCalcSheet(), xDefName->getUpcaseModelName() ) ] = xDefName; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits