diff options
15 files changed, 157 insertions, 19 deletions
@@ -334,7 +334,7 @@ under the License. <dependency prefix="ooxml.curvesapi" artifact="com.github.virtuald:curvesapi:1.08" usage="ooxml"/> <dependency prefix="ooxml.xmlbeans" artifact="org.apache.xmlbeans:xmlbeans:5.3.0" usage="ooxml"/> <dependency prefix="ooxml.commons-compress" artifact="org.apache.commons:commons-compress:1.28.0" usage="ooxml"/> - <dependency prefix="ooxml.commons-lang3" artifact="org.apache.commons:commons-lang3:3.12.0" usage="ooxml"/> + <dependency prefix="ooxml.commons-lang3" artifact="org.apache.commons:commons-lang3:3.18.0" usage="ooxml"/> <!-- jars in the ooxml-test-lib directory, see the fetch-ooxml-jars target--> <dependency prefix="ooxml.test.reflections" artifact="org.reflections:reflections:0.10.2" usage="ooxml-tests"/> diff --git a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java index b3aff917ac..957b804ff5 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java @@ -97,6 +97,9 @@ public class TestAllFiles { "poifs/60320-protected.xlsx", "poifs/protected_sha512.xlsx", + // stress docs + "document/deep-table-cell.docx", + // NOTE: Expected failures should usually be added in file "stress.xls" instead // of being listed here in order to also verify the expected exception details! }; diff --git a/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java b/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java index f949eccb90..be53202057 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java +++ b/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java @@ -19,7 +19,7 @@ package org.apache.poi.ooxml; /** * Indicates a generic OOXML error. */ -public final class POIXMLException extends RuntimeException{ +public final class POIXMLException extends RuntimeException { /** * Create a new {@code POIXMLException} with no * detail message. diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFReader.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFReader.java index c9822434e9..09e4c1df77 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFReader.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFReader.java @@ -178,7 +178,7 @@ public class XSSFReader { // Create the Styles Table, and associate the Themes if present StylesTable styles = new StylesTable(parts.get(0)); parts = pkg.getPartsByContentType(XSSFRelation.THEME.getContentType()); - if (parts.size() != 0) { + if (!parts.isEmpty()) { styles.setTheme(new ThemesTable(parts.get(0))); } return styles; diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java index 054a3dfb07..2565b12b9a 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -45,6 +45,7 @@ import org.apache.commons.collections4.ListValuedMap; import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.logging.log4j.Logger; +import org.apache.poi.POIException; import org.apache.poi.logging.PoiLogManager; import org.apache.poi.hpsf.ClassIDPredefined; import org.apache.poi.ooxml.HyperlinkRelationship; @@ -89,6 +90,7 @@ import org.apache.poi.util.IOUtils; import org.apache.poi.util.Internal; import org.apache.poi.util.NotImplemented; import org.apache.poi.util.Removal; +import org.apache.poi.util.XMLHelper; import org.apache.poi.xssf.XLSBUnsupportedException; import org.apache.poi.xssf.model.CalculationChain; import org.apache.poi.xssf.model.ExternalLinksTable; @@ -110,6 +112,7 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.*; public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Support { private static final Pattern COMMA_PATTERN = Pattern.compile(","); private static final Pattern GET_ALL_PICTURES_PATTERN = Pattern.compile("/xl/media/.*?"); + private static final int MAX_NODE_DEPTH = 1000; /** * Images formats supported by XSSF but not by HSSF @@ -396,6 +399,13 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su WorkbookDocument doc = WorkbookDocument.Factory.parse(stream, DEFAULT_XML_OPTIONS); this.workbook = doc.getWorkbook(); } + final int nodeDepth = XMLHelper.getDepthOfChildNodes(this.workbook.getDomNode(), MAX_NODE_DEPTH); + if (nodeDepth > MAX_NODE_DEPTH) { + throw new IOException(String.format(Locale.ROOT, + "The document is too complex, it has a node depth of %s, which exceeds the maximum allowed of %s", + nodeDepth, + MAX_NODE_DEPTH)); + } ThemesTable theme = null; Map<String, XSSFSheet> shIdMap = new HashMap<>(); @@ -476,6 +486,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su // Process the named ranges reprocessNamedRanges(); + } catch (POIException e) { + throw new IOException(e); } catch (XmlException e) { throw new POIXMLException(e); } diff --git a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java index eb8c4c30c9..56db1b0420 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java @@ -23,22 +23,11 @@ import static org.apache.poi.ooxml.POIXMLTypeLoader.DEFAULT_XML_OPTIONS; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Deque; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Optional; -import java.util.Spliterator; +import java.util.*; import javax.xml.namespace.QName; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.logging.log4j.Logger; +import org.apache.poi.POIException; import org.apache.poi.logging.PoiLogManager; import org.apache.poi.common.usermodel.PictureType; import org.apache.poi.ooxml.POIXMLDocument; @@ -61,6 +50,7 @@ import org.apache.poi.poifs.crypt.HashAlgorithm; import org.apache.poi.util.IOUtils; import org.apache.poi.util.Internal; import org.apache.poi.util.Removal; +import org.apache.poi.util.XMLHelper; import org.apache.poi.wp.usermodel.HeaderFooterType; import org.apache.poi.xddf.usermodel.chart.XDDFChart; import org.apache.poi.xwpf.model.XWPFHeaderFooterPolicy; @@ -105,6 +95,7 @@ import org.openxmlformats.schemas.wordprocessingml.x2006.main.StylesDocument; @SuppressWarnings("unused") public class XWPFDocument extends POIXMLDocument implements Document, IBody { private static final Logger LOG = PoiLogManager.getLogger(XWPFDocument.class); + private static final int MAX_NODE_DEPTH = 1000; protected List<XWPFFooter> footers = new ArrayList<>(); protected List<XWPFHeader> headers = new ArrayList<>(); @@ -214,6 +205,13 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody { doc = DocumentDocument.Factory.parse(stream, DEFAULT_XML_OPTIONS); ctDocument = doc.getDocument(); } + final int nodeDepth = XMLHelper.getDepthOfChildNodes(ctDocument.getDomNode(), MAX_NODE_DEPTH); + if (nodeDepth > MAX_NODE_DEPTH) { + throw new IOException(String.format(Locale.ROOT, + "The document is too complex, it has a node depth of %s, which exceeds the maximum allowed of %s", + nodeDepth, + MAX_NODE_DEPTH)); + } initFootnotes(); @@ -304,6 +302,8 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody { } } initHyperlinks(); + } catch (POIException e) { + throw new IOException(e); } catch (XmlException e) { throw new POIXMLException(e); } diff --git a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFTableCell.java b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFTableCell.java index 27f7dbfaf9..6e590e5f9e 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFTableCell.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFTableCell.java @@ -79,10 +79,16 @@ public class XWPFTableCell implements IBody, ICell { * If a table cell does not include at least one block-level element, then this document shall be considered corrupt */ public XWPFTableCell(CTTc cell, XWPFTableRow tableRow, IBody part) { + if (cell == null) { + throw new IllegalArgumentException("CTTc cannot be null"); + } + if (tableRow == null) { + throw new IllegalArgumentException("tableRow cannot be null"); + } this.ctTc = cell; this.part = part; this.tableRow = tableRow; - this.xwpfDocument = part.getXWPFDocument(); + this.xwpfDocument = part == null ? null : part.getXWPFDocument(); bodyElements = new ArrayList<>(); paragraphs = new ArrayList<>(); @@ -530,9 +536,10 @@ public class XWPFTableCell implements IBody, ICell { return xwpfDocument; } else if (part instanceof XWPFTableCell) { return getCellDocument((XWPFTableCell) part, 0); - } else { + } else if (part != null) { return part.getXWPFDocument(); } + return null; } private static final int MAX_RECURSION_DEPTH = 1000; diff --git a/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java b/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java index c39c7fe7cd..170473396e 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java @@ -28,6 +28,7 @@ import javax.crypto.Cipher; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.poi.POIDataSamples; +import org.apache.poi.POIException; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackagePart; import org.apache.poi.openxml4j.opc.PackagePartName; @@ -274,4 +275,13 @@ class TestXWPFBugs { assertEquals(STJcTable.END, tbl5.getCTTbl().getTblPr().getJc().xgetVal().getEnumValue()); } } + + @Test + public void testDeepTableCell() throws Exception { + // Document contains a table with nested cells. + IOException ex = assertThrows(IOException.class, + () -> XWPFTestDataSamples.openSampleDocument("deep-table-cell.docx")); + assertInstanceOf(POIException.class, ex.getCause()); + assertTrue(ex.getMessage().contains("Node depth exceeds maximum supported depth")); + } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xwpf/usermodel/TestXWPFTableCell.java b/poi-ooxml/src/test/java/org/apache/poi/xwpf/usermodel/TestXWPFTableCell.java index 9d29cb8247..688133d772 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xwpf/usermodel/TestXWPFTableCell.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xwpf/usermodel/TestXWPFTableCell.java @@ -271,6 +271,7 @@ class TestXWPFTableCell { XWPFDocument doc = new XWPFDocument(); XWPFTable table = doc.createTable(1, 1); XWPFTableCell cell = table.getRow(0).getCell(0); + assertEquals(doc, cell.getXWPFDocument()); // cell have at least one paragraph by default when creating a cell assertEquals(1, cell.getParagraphs().size()); @@ -282,6 +283,7 @@ class TestXWPFTableCell { XWPFDocument readDoc = XWPFTestDataSamples.writeOutAndReadBack(doc); XWPFTableCell readCell = readDoc.getTableArray(0).getRow(0).getCell(0); assertEquals(0, readCell.getParagraphs().size()); + assertEquals(readDoc, readCell.getXWPFDocument()); } @Test diff --git a/poi/src/main/java/org/apache/poi/POIException.java b/poi/src/main/java/org/apache/poi/POIException.java new file mode 100644 index 0000000000..ef656b6598 --- /dev/null +++ b/poi/src/main/java/org/apache/poi/POIException.java @@ -0,0 +1,59 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi; + +/** + * Indicates a generic POI exception. This is not commonly used in POI + * but this is intended to be a base class for some new POI exceptions. + * Historically, POI has used {@link RuntimeException} for most of its + * exceptions, but this is not a good practice. This class is a checked + * class that extends {@link Exception} so needs to be explicitly + * caught or declared in the method signature. + * + * @since POI 5.5.0 + */ +public class POIException extends Exception { + private static final long serialVersionUID = 1L; + + /** + * Create a new {@code POIException} with the specified message. + * + * @param msg The error message for the exception. + */ + public POIException(String msg) { + super(msg); + } + + /** + * Create a new {@code POIException} with the specified cause. + * + * @param cause the cause of this exception + */ + public POIException(Throwable cause) { + super(cause); + } + + /** + * Create a new {@code POIException} with the specified message and cause. + * + * @param msg The error message for the exception. + * @param cause the cause of this exception + */ + public POIException(String msg, Throwable cause) { + super(msg, cause); + } +} diff --git a/poi/src/main/java/org/apache/poi/util/XMLHelper.java b/poi/src/main/java/org/apache/poi/util/XMLHelper.java index efeca688a5..071ae08f54 100644 --- a/poi/src/main/java/org/apache/poi/util/XMLHelper.java +++ b/poi/src/main/java/org/apache/poi/util/XMLHelper.java @@ -30,6 +30,7 @@ import static javax.xml.stream.XMLOutputFactory.IS_REPAIRING_NAMESPACES; import java.io.StringReader; import java.lang.reflect.Method; +import java.util.Locale; import java.util.concurrent.TimeUnit; import javax.xml.parsers.DocumentBuilder; @@ -49,7 +50,9 @@ import javax.xml.validation.SchemaFactory; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogBuilder; import org.apache.logging.log4j.Logger; +import org.apache.poi.POIException; import org.apache.poi.logging.PoiLogManager; +import org.w3c.dom.Node; import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -77,7 +80,6 @@ public final class XMLHelper { "org.apache.xerces.util.SecurityManager" }; - private static final Logger LOG = PoiLogManager.getLogger(XMLHelper.class); private static long lastLog; @@ -253,6 +255,38 @@ public final class XMLHelper { return factory; } + /** + * Counts the depth of the DOM tree starting from the given node. + * + * @param node the node to check + * @param maxSupportedDepth the maximum supported depth of the DOM tree + * @return the depth + * @throws POIException if the depth exceeds <code>maxSupportedDepth</code> + */ + public static int getDepthOfChildNodes(final Node node, final int maxSupportedDepth) throws POIException { + return getDepthOfChildNodes(node, maxSupportedDepth, 0); + } + + private static int getDepthOfChildNodes(final Node node, final int maxSupportedDepth, + final int nodeDepth) throws POIException { + final int currentDepth = nodeDepth + 1; + int maxDepth = currentDepth; + Node child = node.getFirstChild(); + while (child != null) { + int childDepth = getDepthOfChildNodes(child, maxSupportedDepth, currentDepth); + if (childDepth > maxDepth) { + maxDepth = childDepth; + if (maxDepth > maxSupportedDepth) { + throw new POIException(String.format(Locale.ROOT, + "Node depth exceeds maximum supported depth of %s" , + maxSupportedDepth)); + } + } + child = child.getNextSibling(); + } + return maxDepth; + } + private static Object _xercesSecurityManager; private static volatile boolean _xercesSecurityManagerSet = false; diff --git a/poi/src/test/java/org/apache/poi/poifs/filesystem/TestPOIFSStream.java b/poi/src/test/java/org/apache/poi/poifs/filesystem/TestPOIFSStream.java index dcbf5145a0..9dd6043ae1 100644 --- a/poi/src/test/java/org/apache/poi/poifs/filesystem/TestPOIFSStream.java +++ b/poi/src/test/java/org/apache/poi/poifs/filesystem/TestPOIFSStream.java @@ -2734,6 +2734,15 @@ final class TestPOIFSStream { } } + @Test + void testDeepData() throws IOException { + try (InputStream stream = POIDataSamples.getPOIFSInstance().openResourceAsStream("deep-data.bin")) { + IOException ex = assertThrows(IOException.class, + () -> new POIFSFileSystem(stream)); + assertEquals("Property tree too deep, likely a corrupt file", ex.getMessage()); + } + } + @Disabled("Takes a long time to run") @Test void performance() throws Exception { @@ -2781,4 +2790,5 @@ final class TestPOIFSStream { } } } + } diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 895d0773dc..9fecf0a1e4 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -93,6 +93,7 @@ <action type="fix" fixes-bug="github-770" context="XSLF">Issue in setBulletStyle in XSLFTextParagraph</action> <action type="add" fixes-bug="github-803" context="SS_Common">Add support for SHEET function</action> <action type="add" fixes-bug="github-804" context="XWPF">Add getters and setters for XWPFTable indentation</action> + <action type="fix" fixes-bug="github-864" context="XDDF">Remove deprecated method XDDFChartData getSeries()</action> </actions> </release> diff --git a/test-data/document/deep-table-cell.docx b/test-data/document/deep-table-cell.docx Binary files differnew file mode 100644 index 0000000000..6bb54e6c6d --- /dev/null +++ b/test-data/document/deep-table-cell.docx diff --git a/test-data/poifs/deep-data.bin b/test-data/poifs/deep-data.bin Binary files differnew file mode 100644 index 0000000000..a06aae8694 --- /dev/null +++ b/test-data/poifs/deep-data.bin |