[Libreoffice-commits] core.git: sc/inc sc/qa sc/source
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Mon Jul 5 12:04:45 UTC 2021
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 <vmiklos at collabora.com>
AuthorDate: Mon Jul 5 12:29:18 2021 +0200
Commit: Miklos Vajna <vmiklos at 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 <vmiklos at 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;
More information about the Libreoffice-commits
mailing list