sc/inc/address.hxx | 26 +++++++++++++------------- sc/qa/unit/data/xlsx/invalid-named-range.xlsx |binary sc/qa/unit/subsequent_export-test.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 4873bb10c136a503be00bd73d1e7c4ec66e9a10c Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Jul 5 12:29:18 2021 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Jul 8 08:39:26 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. (cherry picked from commit db1c8df98a23d687d6806f371bdd416dd1b84589) Conflicts: sc/qa/unit/subsequent_export-test2.cxx Change-Id: Ifde07b5e59fba371f1f8ab3e82861c6997c6dbf0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118551 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Luboš Luňák <l.lu...@collabora.com> diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx index 4b2ad09691ba..8f3b6830e030 100644 --- a/sc/inc/address.hxx +++ b/sc/inc/address.hxx @@ -494,7 +494,7 @@ inline bool ValidAddress( const ScAddress& rAddress, SCCOL nMaxCol = MAXCOL, SCR } // ScRange -class SAL_WARN_UNUSED ScRange final +class SAL_WARN_UNUSED SC_DLLPUBLIC ScRange final { public: ScAddress aStart; @@ -551,18 +551,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 ); @@ -614,14 +614,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 @@ -630,18 +630,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-test.cxx b/sc/qa/unit/subsequent_export-test.cxx index 3b5e3903290a..f75b81c1a277 100644 --- a/sc/qa/unit/subsequent_export-test.cxx +++ b/sc/qa/unit/subsequent_export-test.cxx @@ -283,6 +283,7 @@ public: void testTdf139258_rotated_image(); void testCheckboxFormControlXlsxExport(); void testButtonFormControlXlsxExport(); + void testInvalidNamedRange(); CPPUNIT_TEST_SUITE(ScExportTest); CPPUNIT_TEST(test); @@ -464,6 +465,7 @@ public: CPPUNIT_TEST(testTdf139258_rotated_image); CPPUNIT_TEST(testCheckboxFormControlXlsxExport); CPPUNIT_TEST(testButtonFormControlXlsxExport); + CPPUNIT_TEST(testInvalidNamedRange); CPPUNIT_TEST_SUITE_END(); @@ -5894,6 +5896,23 @@ void ScExportTest::testButtonFormControlXlsxExport() assertXPathContent(pDoc, "//x:anchor/x:to/xdr:row", "7"); } +void ScExportTest::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(ScExportTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sc/source/filter/inc/defnamesbuffer.hxx b/sc/source/filter/inc/defnamesbuffer.hxx index 77ee38fd5ccb..67c5930682ae 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 733d4790f8ff..769e36f24939 100644 --- a/sc/source/filter/oox/defnamesbuffer.cxx +++ b/sc/source/filter/oox/defnamesbuffer.cxx @@ -238,6 +238,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 ) { @@ -356,6 +373,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