From 4e26e6a8d8fea055f17fe856c01f359362c4ff58 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Wed, 25 Apr 2018 10:03:39 +0000 Subject: [PATCH] Bug 62187 - commit Commons Compress unrelated changes git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1830061 13f79535-47bb-0310-9956-ffa450edef68 --- .../xssf/usermodel/examples/BigGridDemo.java | 17 +- .../poi/UnsupportedFileFormatException.java | 6 +- .../java/org/apache/poi/POIXMLFactory.java | 2 +- .../exceptions/NotOfficeXmlFileException.java | 4 + .../poi/openxml4j/opc/CompressionOption.java | 47 -- .../apache/poi/openxml4j/opc/OPCPackage.java | 91 +--- .../apache/poi/openxml4j/opc/PackagePart.java | 5 +- .../opc/PackageRelationshipCollection.java | 8 +- .../apache/poi/openxml4j/opc/ZipPackage.java | 269 +++++----- .../poi/openxml4j/opc/ZipPackagePart.java | 11 +- .../opc/internal/ZipContentTypeManager.java | 17 +- .../poi/openxml4j/opc/internal/ZipHelper.java | 32 +- .../ZipPackagePropertiesMarshaller.java | 12 +- .../marshallers/ZipPartMarshaller.java | 24 +- .../openxml4j/util/ZipArchiveFakeEntry.java | 53 ++ .../util/ZipArchiveThresholdInputStream.java | 243 +++++++++ .../poi/openxml4j/util/ZipEntrySource.java | 19 +- .../openxml4j/util/ZipFileZipEntrySource.java | 27 + .../util/ZipInputStreamZipEntrySource.java | 115 ++--- .../poi/openxml4j/util/ZipSecureFile.java | 214 +------- .../crypt/temp/AesZipFileZipEntrySource.java | 29 +- .../poifs/crypt/temp/EncryptedTempData.java | 5 +- .../org/apache/poi/xssf/dev/XSSFDump.java | 20 +- .../poi/xssf/streaming/SXSSFWorkbook.java | 28 +- .../apache/poi/openxml4j/opc/TestPackage.java | 462 ++++++++++++------ .../poi/openxml4j/opc/TestZipPackage.java | 246 ---------- .../poi/openxml4j/opc/ZipFileAssert.java | 75 ++- .../TestZipPackagePropertiesMarshaller.java | 16 +- .../apache/poi/poifs/crypt/TestDecryptor.java | 37 +- .../poi/xssf/streaming/TestSXSSFWorkbook.java | 112 ++--- 30 files changed, 1059 insertions(+), 1187 deletions(-) delete mode 100644 src/ooxml/java/org/apache/poi/openxml4j/opc/CompressionOption.java create mode 100644 src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java create mode 100644 src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java delete mode 100644 src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java index 439b16ce85..75e116a589 100644 --- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java +++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java @@ -74,12 +74,11 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook; *

* See * http://poi.apache.org/spreadsheet/how-to.html#sxssf. - - * - * @author Yegor Kozlov */ -public class BigGridDemo { +public final class BigGridDemo { private static final String XML_ENCODING = "UTF-8"; + + private BigGridDemo() {} public static void main(String[] args) throws Exception { @@ -229,17 +228,17 @@ public class BigGridDemo { private final Writer _out; private int _rownum; - public SpreadsheetWriter(Writer out){ + SpreadsheetWriter(Writer out){ _out = out; } - public void beginSheet() throws IOException { + void beginSheet() throws IOException { _out.write("" + "" ); _out.write("\n"); } - public void endSheet() throws IOException { + void endSheet() throws IOException { _out.write(""); _out.write(""); } @@ -249,7 +248,7 @@ public class BigGridDemo { * * @param rownum 0-based row number */ - public void insertRow(int rownum) throws IOException { + void insertRow(int rownum) throws IOException { _out.write("\n"); this._rownum = rownum; } @@ -257,7 +256,7 @@ public class BigGridDemo { /** * Insert row end marker */ - public void endRow() throws IOException { + void endRow() throws IOException { _out.write("\n"); } diff --git a/src/java/org/apache/poi/UnsupportedFileFormatException.java b/src/java/org/apache/poi/UnsupportedFileFormatException.java index a8caebb4bf..b893639514 100644 --- a/src/java/org/apache/poi/UnsupportedFileFormatException.java +++ b/src/java/org/apache/poi/UnsupportedFileFormatException.java @@ -23,7 +23,11 @@ package org.apache.poi; public abstract class UnsupportedFileFormatException extends IllegalArgumentException { private static final long serialVersionUID = -8281969197282030046L; - public UnsupportedFileFormatException(String s) { + protected UnsupportedFileFormatException(String s) { super(s); } + + protected UnsupportedFileFormatException(String message, Throwable cause) { + super(message, cause); + } } \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/POIXMLFactory.java b/src/ooxml/java/org/apache/poi/POIXMLFactory.java index eefa591877..651f40cbc1 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLFactory.java +++ b/src/ooxml/java/org/apache/poi/POIXMLFactory.java @@ -60,7 +60,7 @@ public abstract class POIXMLFactory { return createDocumentPart(cls, ORPHAN_PART, new Object[]{part}); } } catch (Exception e) { - throw new POIXMLException(e); + throw new POIXMLException((e.getCause() != null ? e.getCause() : e).getMessage(), e); } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/exceptions/NotOfficeXmlFileException.java b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/NotOfficeXmlFileException.java index 901967e202..bd6c6378ba 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/exceptions/NotOfficeXmlFileException.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/NotOfficeXmlFileException.java @@ -26,4 +26,8 @@ public class NotOfficeXmlFileException extends UnsupportedFileFormatException { public NotOfficeXmlFileException(String message) { super(message); } + + public NotOfficeXmlFileException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/CompressionOption.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/CompressionOption.java deleted file mode 100644 index 583b5c51de..0000000000 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/CompressionOption.java +++ /dev/null @@ -1,47 +0,0 @@ -/* ==================================================================== - 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.openxml4j.opc; - -import java.util.zip.Deflater; - -/** - * Specifies the compression level for content that is stored in a PackagePart. - * - * @author Julien Chable - * @version 1.0 - */ -public enum CompressionOption { - /** Compression is optimized for performance. */ - FAST(Deflater.BEST_SPEED), - /** Compression is optimized for size. */ - MAXIMUM(Deflater.BEST_COMPRESSION), - /** Compression is optimized for a balance between size and performance. */ - NORMAL(Deflater.DEFAULT_COMPRESSION), - /** Compression is turned off. */ - NOT_COMPRESSED(Deflater.NO_COMPRESSION); - - private final int value; - - CompressionOption(int value) { - this.value = value; - } - - public int value() { - return this.value; - } -} diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java index ec5892040a..7bb7e1367a 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java @@ -76,12 +76,12 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { /** * Package access. */ - private PackageAccess packageAccess; + private final PackageAccess packageAccess; /** * Package parts collection. */ - protected PackagePartCollection partList; + private PackagePartCollection partList; /** * Package relationships. @@ -91,17 +91,17 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { /** * Part marshallers by content type. */ - protected Map partMarshallers; + protected final Map partMarshallers = new HashMap<>(5); /** * Default part marshaller. */ - protected PartMarshaller defaultPartMarshaller; + protected final PartMarshaller defaultPartMarshaller = new DefaultMarshaller(); /** * Part unmarshallers by content type. */ - protected Map partUnmarshallers; + protected final Map partUnmarshallers = new HashMap<>(2); /** * Core package properties. @@ -138,41 +138,27 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { if (getClass() != ZipPackage.class) { throw new IllegalArgumentException("PackageBase may not be subclassed"); } - init(); this.packageAccess = access; - } - /** - * Initialize the package instance. - */ - private void init() { - this.partMarshallers = new HashMap<>(5); - this.partUnmarshallers = new HashMap<>(2); + final ContentType contentType = newCorePropertiesPart(); + // TODO Delocalize specialized marshallers + this.partUnmarshallers.put(contentType, new PackagePropertiesUnmarshaller()); + this.partMarshallers.put(contentType, new ZipPackagePropertiesMarshaller()); + } + private static ContentType newCorePropertiesPart() { try { - // Add 'default' unmarshaller - this.partUnmarshallers.put(new ContentType( - ContentTypes.CORE_PROPERTIES_PART), - new PackagePropertiesUnmarshaller()); - - // Add default marshaller - this.defaultPartMarshaller = new DefaultMarshaller(); - // TODO Delocalize specialized marshallers - this.partMarshallers.put(new ContentType( - ContentTypes.CORE_PROPERTIES_PART), - new ZipPackagePropertiesMarshaller()); + return new ContentType(ContentTypes.CORE_PROPERTIES_PART); } catch (InvalidFormatException e) { // Should never happen throw new OpenXML4JRuntimeException( - "Package.init() : this exception should never happen, " + - "if you read this message please send a mail to the developers team. : " + - e.getMessage(), - e + "Package.init() : this exception should never happen, " + + "if you read this message please send a mail to the developers team. : " + + e.getMessage(), e ); } } - /** * Open a package with read/write permission. * @@ -625,7 +611,8 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { return null; } } - return getPartImpl(partName); + + return partList.get(partName); } /** @@ -737,25 +724,16 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { // Check rule M4.1 -> A format consumer shall consider more than // one core properties relationship for a package to be an error - // (We just log it and move on, as real files break this!) + // (We just log it and move on, as real files break this!) boolean hasCorePropertiesPart = false; boolean needCorePropertiesPart = true; - PackagePart[] parts = this.getPartsImpl(); - this.partList = new PackagePartCollection(); - for (PackagePart part : parts) { - if (partList.containsKey(part._partName)) { - throw new InvalidFormatException( - "A part with the name '" + - part._partName + - "' already exist : Packages shall not contain equivalent " + - "part names and package implementers shall neither create " + - "nor recognize packages with equivalent part names. [M1.12]"); - } + partList = getPartsImpl(); + for (PackagePart part : new ArrayList<>(partList.sortedValues())) { + part.loadRelationships(); // Check OPC compliance rule M4.1 - if (part.getContentType().equals( - ContentTypes.CORE_PROPERTIES_PART)) { + if (ContentTypes.CORE_PROPERTIES_PART.equals(part.getContentType())) { if (!hasCorePropertiesPart) { hasCorePropertiesPart = true; } else { @@ -768,11 +746,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { PartUnmarshaller partUnmarshaller = partUnmarshallers.get(part._contentType); if (partUnmarshaller != null) { - UnmarshallContext context = new UnmarshallContext(this, - part._partName); + UnmarshallContext context = new UnmarshallContext(this, part._partName); try { - PackagePart unmarshallPart = partUnmarshaller - .unmarshall(context, part.getInputStream()); + PackagePart unmarshallPart = partUnmarshaller.unmarshall(context, part.getInputStream()); + partList.remove(part.getPartName()); partList.put(unmarshallPart._partName, unmarshallPart); // Core properties case-- use first CoreProperties part we come across @@ -790,12 +767,6 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { } catch (InvalidOperationException invoe) { throw new InvalidFormatException(invoe.getMessage(), invoe); } - } else { - try { - partList.put(part._partName, part); - } catch (InvalidOperationException e) { - throw new InvalidFormatException(e.getMessage(), e); - } } } } @@ -1457,6 +1428,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { } } + /* Accesseurs */ /** @@ -1575,21 +1547,12 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { protected abstract void saveImpl(OutputStream outputStream) throws IOException; - /** - * Get the package part mapped to the specified URI. - * - * @param partName - * The URI of the part to retrieve. - * @return The package part located by the specified URI, else null. - */ - protected abstract PackagePart getPartImpl(PackagePartName partName); - /** * Get all parts link to the package. * * @return A list of the part owned by the package. */ - protected abstract PackagePart[] getPartsImpl() + protected abstract PackagePartCollection getPartsImpl() throws InvalidFormatException; /** diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java index 09fb30f70b..3a19020a3a 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java @@ -106,8 +106,9 @@ public abstract class PackagePart implements RelationshipSource, Comparable relationshipsByID; + private final TreeMap relationshipsByID = new TreeMap<>(); /** * Package relationships ordered by type. */ - private TreeMap relationshipsByType; + private final TreeMap relationshipsByType = new TreeMap<>(); /** * A lookup of internal relationships to avoid @@ -88,8 +88,6 @@ public final class PackageRelationshipCollection implements * Constructor. */ PackageRelationshipCollection() { - relationshipsByID = new TreeMap<>(); - relationshipsByType = new TreeMap<>(); } /** @@ -149,8 +147,6 @@ public final class PackageRelationshipCollection implements */ public PackageRelationshipCollection(OPCPackage container, PackagePart part) throws InvalidFormatException { - this(); - if (container == null) throw new IllegalArgumentException("container needs to be specified"); diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java index 7dcf8b5f91..37750afc61 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java @@ -17,17 +17,23 @@ package org.apache.poi.openxml4j.opc; +import static org.apache.poi.openxml4j.opc.ContentTypes.RELATIONSHIPS_PART; +import static org.apache.poi.openxml4j.opc.internal.ContentTypeManager.CONTENT_TYPES_PART_NAME; + import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.Enumeration; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; +import org.apache.poi.UnsupportedFileFormatException; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException; @@ -41,10 +47,10 @@ import org.apache.poi.openxml4j.opc.internal.PartMarshaller; import org.apache.poi.openxml4j.opc.internal.ZipContentTypeManager; import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.openxml4j.opc.internal.marshallers.ZipPartMarshaller; +import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream; import org.apache.poi.openxml4j.util.ZipEntrySource; import org.apache.poi.openxml4j.util.ZipFileZipEntrySource; import org.apache.poi.openxml4j.util.ZipInputStreamZipEntrySource; -import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; import org.apache.poi.util.IOUtils; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -95,12 +101,12 @@ public final class ZipPackage extends OPCPackage { */ ZipPackage(InputStream in, PackageAccess access) throws IOException { super(access); - ThresholdInputStream zis = ZipHelper.openZipStream(in); + ZipArchiveThresholdInputStream zis = ZipHelper.openZipStream(in); try { this.zipArchive = new ZipInputStreamZipEntrySource(zis); } catch (final IOException e) { IOUtils.closeQuietly(zis); - throw new IOException("Failed to read zip entry source", e); + throw e; } } @@ -138,6 +144,9 @@ public final class ZipPackage extends OPCPackage { if (access == PackageAccess.WRITE) { throw new InvalidOperationException("Can't open the specified file: '" + file + "'", e); } + if ("java.util.zip.ZipException: archive is not a ZIP archive".equals(e.getMessage())) { + throw new NotOfficeXmlFileException("archive is not a ZIP archive", e); + } LOG.log(POILogger.ERROR, "Error in zip file "+file+" - falling back to stream processing (i.e. ignoring zip central directory)"); ze = openZipEntrySourceStream(file); } @@ -159,19 +168,19 @@ public final class ZipPackage extends OPCPackage { try { // read from the file input stream return openZipEntrySourceStream(fis); + } catch (final InvalidOperationException|UnsupportedFileFormatException e) { + // abort: close the zip input stream + IOUtils.closeQuietly(fis); + throw e; } catch (final Exception e) { // abort: close the file input stream IOUtils.closeQuietly(fis); - if (e instanceof InvalidOperationException) { - throw (InvalidOperationException)e; - } else { - throw new InvalidOperationException("Failed to read the file input stream from file: '" + file + "'", e); - } + throw new InvalidOperationException("Failed to read the file input stream from file: '" + file + "'", e); } } private static ZipEntrySource openZipEntrySourceStream(FileInputStream fis) throws InvalidOperationException { - final ThresholdInputStream zis; + final ZipArchiveThresholdInputStream zis; // Acquire a resource that is needed to read the next level of openZipEntrySourceStream try { // open the zip input stream @@ -185,18 +194,18 @@ public final class ZipPackage extends OPCPackage { try { // read from the zip input stream return openZipEntrySourceStream(zis); + } catch (final InvalidOperationException|UnsupportedFileFormatException e) { + // abort: close the zip input stream + IOUtils.closeQuietly(zis); + throw e; } catch (final Exception e) { // abort: close the zip input stream IOUtils.closeQuietly(zis); - if (e instanceof InvalidOperationException) { - throw (InvalidOperationException)e; - } else { - throw new InvalidOperationException("Failed to read the zip entry source stream", e); - } + throw new InvalidOperationException("Failed to read the zip entry source stream", e); } } - private static ZipEntrySource openZipEntrySourceStream(ThresholdInputStream zis) throws InvalidOperationException { + private static ZipEntrySource openZipEntrySourceStream(ZipArchiveThresholdInputStream zis) throws InvalidOperationException { // Acquire the final level resource. If this is acquired successfully, the zip package was read successfully from the input stream try { // open the zip entry source stream @@ -224,149 +233,115 @@ public final class ZipPackage extends OPCPackage { /** * Retrieves the parts from this package. We assume that the package has not * been yet inspect to retrieve all the parts, this method will open the - * archive and look for all parts contain inside it. If the package part - * list is not empty, it will be emptied. + * archive and look for all parts contain inside it. * * @return All parts contain in this package. * @throws InvalidFormatException if the package is not valid. */ @Override - protected PackagePart[] getPartsImpl() throws InvalidFormatException { - if (this.partList == null) { - // The package has just been created, we create an empty part - // list. - this.partList = new PackagePartCollection(); - } + protected PackagePartCollection getPartsImpl() throws InvalidFormatException { + final PackagePartCollection newPartList = new PackagePartCollection(); - if (this.zipArchive == null) { - return this.partList.sortedValues().toArray( - new PackagePart[this.partList.size()]); + if (zipArchive == null) { + return newPartList; } // First we need to parse the content type part - Enumeration entries = this.zipArchive.getEntries(); - while (entries.hasMoreElements()) { - ZipEntry entry = entries.nextElement(); - if (entry.getName().equalsIgnoreCase( - ContentTypeManager.CONTENT_TYPES_PART_NAME)) { - try { - this.contentTypeManager = new ZipContentTypeManager( - getZipArchive().getInputStream(entry), this); - } catch (IOException e) { - throw new InvalidFormatException(e.getMessage(), e); - } - break; + final ZipEntry contentTypeEntry = + zipArchive.getEntry(CONTENT_TYPES_PART_NAME); + if (contentTypeEntry != null) { + try { + this.contentTypeManager = new ZipContentTypeManager( + zipArchive.getInputStream(contentTypeEntry), this); + } catch (IOException e) { + throw new InvalidFormatException(e.getMessage(), e); } - } - - // At this point, we should have loaded the content type part - if (this.contentTypeManager == null) { + } else { // Is it a different Zip-based format? - int numEntries = 0; - boolean hasMimetype = false; - boolean hasSettingsXML = false; - entries = this.zipArchive.getEntries(); - while (entries.hasMoreElements()) { - final ZipEntry entry = entries.nextElement(); - final String name = entry.getName(); - if (MIMETYPE.equals(name)) { - hasMimetype = true; - } - if (SETTINGS_XML.equals(name)) { - hasSettingsXML = true; - } - numEntries++; - } + final boolean hasMimetype = zipArchive.getEntry(MIMETYPE) != null; + final boolean hasSettingsXML = zipArchive.getEntry(SETTINGS_XML) != null; if (hasMimetype && hasSettingsXML) { throw new ODFNotOfficeXmlFileException( - "The supplied data appears to be in ODF (Open Document) Format. " + - "Formats like these (eg ODS, ODP) are not supported, try Apache ODFToolkit"); + "The supplied data appears to be in ODF (Open Document) Format. " + + "Formats like these (eg ODS, ODP) are not supported, try Apache ODFToolkit"); } - if (numEntries == 0) { + if (!zipArchive.getEntries().hasMoreElements()) { throw new NotOfficeXmlFileException( - "No valid entries or contents found, this is not a valid OOXML " + - "(Office Open XML) file"); + "No valid entries or contents found, this is not a valid OOXML " + + "(Office Open XML) file"); } - // Fallback exception throw new InvalidFormatException( - "Package should contain a content type part [M1.13]"); + "Package should contain a content type part [M1.13]"); } // Now create all the relationships // (Need to create relationships before other // parts, otherwise we might create a part before // its relationship exists, and then it won't tie up) - entries = this.zipArchive.getEntries(); - while (entries.hasMoreElements()) { - ZipEntry entry = entries.nextElement(); - PackagePartName partName = buildPartName(entry); - if(partName == null) { - continue; - } + final List entries = + Collections.list(zipArchive.getEntries()).stream() + .map(zae -> new EntryTriple(zae, contentTypeManager)) + .filter(mm -> mm.partName != null) + .sorted() + .collect(Collectors.toList()); + + for (final EntryTriple et : entries) { + et.register(newPartList); + } - // Only proceed for Relationships at this stage - String contentType = contentTypeManager.getContentType(partName); - if (contentType != null && contentType.equals(ContentTypes.RELATIONSHIPS_PART)) { - try { - PackagePart part = new ZipPackagePart(this, entry, partName, contentType); - partList.put(partName, part); - } catch (InvalidOperationException e) { - throw new InvalidFormatException(e.getMessage(), e); - } + return newPartList; + } + + private class EntryTriple implements Comparable { + final ZipEntry zipArchiveEntry; + final PackagePartName partName; + final String contentType; + + EntryTriple(final ZipEntry zipArchiveEntry, final ContentTypeManager contentTypeManager) { + this.zipArchiveEntry = zipArchiveEntry; + + final String entryName = zipArchiveEntry.getName(); + PackagePartName ppn = null; + try { + // We get an error when we parse [Content_Types].xml + // because it's not a valid URI. + ppn = (CONTENT_TYPES_PART_NAME.equalsIgnoreCase(entryName)) ? null + : PackagingURIHelper.createPartName(ZipHelper.getOPCNameFromZipItemName(entryName)); + } catch (Exception e) { + // We assume we can continue, even in degraded mode ... + LOG.log(POILogger.WARN,"Entry " + entryName + " is not valid, so this part won't be add to the package.", e); } + + this.partName = ppn; + this.contentType = (ppn == null) ? null : contentTypeManager.getContentType(partName); } - // Then we can go through all the other parts - entries = this.zipArchive.getEntries(); - while (entries.hasMoreElements()) { - ZipEntry entry = entries.nextElement(); - PackagePartName partName = buildPartName(entry); - if(partName == null) { - continue; + void register(final PackagePartCollection partList) throws InvalidFormatException { + if (contentType == null) { + throw new InvalidFormatException("The part " + partName.getURI().getPath() + " does not have any " + + "content type ! Rule: Package require content types when retrieving a part from a package. [M.1.14]"); } - String contentType = contentTypeManager.getContentType(partName); - if (contentType != null && contentType.equals(ContentTypes.RELATIONSHIPS_PART)) { - // Already handled - } - else if (contentType != null) { - try { - PackagePart part = new ZipPackagePart(this, entry, partName, contentType); - partList.put(partName, part); - } catch (InvalidOperationException e) { - throw new InvalidFormatException(e.getMessage(), e); - } - } else { + if (partList.containsKey(partName)) { throw new InvalidFormatException( - "The part " + partName.getURI().getPath() - + " does not have any content type ! Rule: Package require content types when retrieving a part from a package. [M.1.14]"); + "A part with the name '"+partName+"' already exist : Packages shall not contain equivalent part names " + + "and package implementers shall neither create nor recognize packages with equivalent part names. [M1.12]"); } - } - - return partList.sortedValues().toArray(new PackagePart[partList.size()]); - } - /** - * Builds a PackagePartName for the given ZipEntry, - * or null if it's the content types / invalid part - */ - private PackagePartName buildPartName(ZipEntry entry) { - try { - // We get an error when we parse [Content_Types].xml - // because it's not a valid URI. - if (entry.getName().equalsIgnoreCase( - ContentTypeManager.CONTENT_TYPES_PART_NAME)) { - return null; + try { + partList.put(partName, new ZipPackagePart(ZipPackage.this, zipArchiveEntry, partName, contentType, false)); + } catch (InvalidOperationException e) { + throw new InvalidFormatException(e.getMessage(), e); } - return PackagingURIHelper.createPartName(ZipHelper - .getOPCNameFromZipItemName(entry.getName())); - } catch (Exception e) { - // We assume we can continue, even in degraded mode ... - LOG.log(POILogger.WARN,"Entry " - + entry.getName() - + " is not valid, so this part won't be add to the package.", e); - return null; + } + + @Override + public int compareTo(EntryTriple o) { + final int contentTypeOrder1 = RELATIONSHIPS_PART.equals(contentType) ? -1 : 1; + final int contentTypeOrder2 = RELATIONSHIPS_PART.equals(o.contentType) ? -1 : 1; + final int cmpCT = Integer.compare(contentTypeOrder1, contentTypeOrder2); + return cmpCT != 0 ? cmpCT : partName.compareTo(o.partName); } } @@ -494,21 +469,6 @@ public final class ZipPackage extends OPCPackage { } } - /** - * Implement the getPart() method to retrieve a part from its URI in the - * current package - * - * - * @see #getPart(PackageRelationship) - */ - @Override - protected PackagePart getPartImpl(PackagePartName partName) { - if (partList.containsKey(partName)) { - return partList.get(partName); - } - return null; - } - /** * Save this package into the specified stream * @@ -523,13 +483,8 @@ public final class ZipPackage extends OPCPackage { // Check that the document was open in write mode throwExceptionIfReadOnly(); - final ZipOutputStream zos; - try { - if (!(outputStream instanceof ZipOutputStream)) { - zos = new ZipOutputStream(outputStream); - } else { - zos = (ZipOutputStream) outputStream; - } + try (final ZipOutputStream zos = (outputStream instanceof ZipOutputStream) + ? (ZipOutputStream) outputStream : new ZipOutputStream(outputStream)) { // If the core properties part does not exist in the part list, // we save it as well @@ -574,20 +529,14 @@ public final class ZipPackage extends OPCPackage { final PackagePartName ppn = part.getPartName(); LOG.log(POILogger.DEBUG,"Save part '" + ZipHelper.getZipItemNameFromOPCName(ppn.getName()) + "'"); - PartMarshaller marshaller = partMarshallers.get(part._contentType); - String errMsg = "The part " + ppn.getURI() + " failed to be saved in the stream with marshaller "; - - if (marshaller != null) { - if (!marshaller.marshall(part, zos)) { - throw new OpenXML4JException(errMsg + marshaller); - } - } else { - if (!defaultPartMarshaller.marshall(part, zos)) { - throw new OpenXML4JException(errMsg + defaultPartMarshaller); - } - } + final PartMarshaller marshaller = partMarshallers.get(part._contentType); + + final PartMarshaller pm = (marshaller != null) ? marshaller : defaultPartMarshaller; + if (!pm.marshall(part, zos)) { + String errMsg = "The part " + ppn.getURI() + " failed to be saved in the stream with marshaller "; + throw new OpenXML4JException(errMsg + pm); + } } - zos.close(); } catch (OpenXML4JRuntimeException e) { // no need to wrap this type of Exception throw e; diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java index 58bbbfe225..57e649c281 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java @@ -25,6 +25,7 @@ import java.util.zip.ZipEntry; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.exceptions.OpenXML4JException; +import org.apache.poi.openxml4j.opc.internal.ContentType; import org.apache.poi.openxml4j.opc.internal.marshallers.ZipPartMarshaller; import org.apache.poi.util.NotImplemented; @@ -48,11 +49,11 @@ public class ZipPackagePart extends PackagePart { * @param container * The container package. * @param partName - * Part name. + * The part name. * @param contentType * Content type. * @throws InvalidFormatException - * Throws if the content of this part invalid. + * Throws if the content of this part is invalid. */ public ZipPackagePart(OPCPackage container, PackagePartName partName, String contentType) throws InvalidFormatException { @@ -73,10 +74,10 @@ public class ZipPackagePart extends PackagePart { * @throws InvalidFormatException * Throws if the content of this part is invalid. */ - public ZipPackagePart(OPCPackage container, ZipEntry zipEntry, - PackagePartName partName, String contentType) + /* package */ ZipPackagePart(OPCPackage container, ZipEntry zipEntry, + PackagePartName partName, String contentType, boolean loadRelationships) throws InvalidFormatException { - super(container, partName, contentType); + super(container, partName, new ContentType(contentType), loadRelationships); this.zipEntry = zipEntry; } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipContentTypeManager.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipContentTypeManager.java index 70bdb39e12..a51d6353b4 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipContentTypeManager.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipContentTypeManager.java @@ -57,26 +57,23 @@ public class ZipContentTypeManager extends ContentTypeManager { @SuppressWarnings("resource") @Override public boolean saveImpl(Document content, OutputStream out) { - ZipOutputStream zos = null; - if (out instanceof ZipOutputStream) - zos = (ZipOutputStream) out; - else - zos = new ZipOutputStream(out); + final ZipOutputStream zos = (out instanceof ZipOutputStream) + ? (ZipOutputStream) out : new ZipOutputStream(out); ZipEntry partEntry = new ZipEntry(CONTENT_TYPES_PART_NAME); try { // Referenced in ZIP zos.putNextEntry(partEntry); - // Saving data in the ZIP file - if (!StreamHelper.saveXmlInStream(content, zos)) { - return false; + try { + // Saving data in the ZIP file + return StreamHelper.saveXmlInStream(content, zos); + } finally { + zos.closeEntry(); } - zos.closeEntry(); } catch (IOException ioe) { logger.log(POILogger.ERROR, "Cannot write: " + CONTENT_TYPES_PART_NAME + " in Zip !", ioe); return false; } - return true; } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java index a5c741e287..cd38adcea7 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java @@ -24,9 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; -import java.util.Enumeration; import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException; @@ -34,8 +32,8 @@ import org.apache.poi.openxml4j.exceptions.OLE2NotOfficeXmlFileException; import org.apache.poi.openxml4j.opc.PackageRelationship; import org.apache.poi.openxml4j.opc.PackageRelationshipTypes; import org.apache.poi.openxml4j.opc.ZipPackage; +import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream; import org.apache.poi.openxml4j.util.ZipSecureFile; -import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; import org.apache.poi.poifs.filesystem.FileMagic; import org.apache.poi.util.Internal; @@ -72,24 +70,6 @@ public final class ZipHelper { return new ZipEntry(corePropsRel.getTargetURI().getPath()); } - /** - * Retrieve the Zip entry of the content types part. - */ - public static ZipEntry getContentTypeZipEntry(ZipPackage pkg) { - Enumeration entries = pkg.getZipArchive().getEntries(); - - // Enumerate through the Zip entries until we find the one named - // '[Content_Types].xml'. - while (entries.hasMoreElements()) { - ZipEntry entry = entries.nextElement(); - if (entry.getName().equals( - ContentTypeManager.CONTENT_TYPES_PART_NAME)) { - return entry; - } - } - return null; - } - /** * Convert a zip name into an OPC name by adding a leading forward slash to * the specified item name. @@ -158,7 +138,7 @@ public final class ZipHelper { * Warning - this will consume the first few bytes of the stream, * you should push-back or reset the stream after use! */ - public static void verifyZipHeader(InputStream stream) throws NotOfficeXmlFileException, IOException { + private static void verifyZipHeader(InputStream stream) throws NotOfficeXmlFileException, IOException { InputStream is = FileMagic.prepareToCheckMagic(stream); FileMagic fm = FileMagic.valueOf(is); @@ -191,14 +171,14 @@ public final class ZipHelper { * @return The zip stream freshly open. */ @SuppressWarnings("resource") - public static ThresholdInputStream openZipStream(InputStream stream) throws IOException { + public static ZipArchiveThresholdInputStream openZipStream(InputStream stream) throws IOException { // Peek at the first few bytes to sanity check InputStream checkedStream = FileMagic.prepareToCheckMagic(stream); verifyZipHeader(checkedStream); // Open as a proper zip stream InputStream zis = new ZipInputStream(checkedStream); - return ZipSecureFile.addThreshold(zis); + return new ZipArchiveThresholdInputStream(zis); } /** @@ -211,7 +191,7 @@ public final class ZipHelper { * @throws IOException if the zip file cannot be opened or closed to read the header signature * @throws NotOfficeXmlFileException if stream does not start with zip header signature */ - public static ZipFile openZipFile(File file) throws IOException, NotOfficeXmlFileException { + public static ZipSecureFile openZipFile(File file) throws IOException, NotOfficeXmlFileException { if (!file.exists()) { throw new FileNotFoundException("File does not exist"); } @@ -235,7 +215,7 @@ public final class ZipHelper { * The file path. * @return The zip archive freshly open. */ - public static ZipFile openZipFile(String path) throws IOException { + public static ZipSecureFile openZipFile(String path) throws IOException { return openZipFile(new File(path)); } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPackagePropertiesMarshaller.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPackagePropertiesMarshaller.java index 6b4b215518..7b50139604 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPackagePropertiesMarshaller.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPackagePropertiesMarshaller.java @@ -49,15 +49,15 @@ public final class ZipPackagePropertiesMarshaller extends PackagePropertiesMarsh try { // Save in ZIP zos.putNextEntry(ctEntry); // Add entry in ZIP - super.marshall(part, out); // Marshall the properties inside a XML - // Document - if (!StreamHelper.saveXmlInStream(xmlDoc, out)) { - return false; + try { + super.marshall(part, out); // Marshall the properties inside a XML + // Document + return StreamHelper.saveXmlInStream(xmlDoc, out); + } finally { + zos.closeEntry(); } - zos.closeEntry(); } catch (IOException e) { throw new OpenXML4JException(e.getLocalizedMessage(), e); } - return true; } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPartMarshaller.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPartMarshaller.java index 6836d3c55a..6ba8339d1a 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPartMarshaller.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/marshallers/ZipPartMarshaller.java @@ -36,6 +36,7 @@ import org.apache.poi.openxml4j.opc.TargetMode; import org.apache.poi.openxml4j.opc.internal.PartMarshaller; import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.util.DocumentHelper; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.xssf.usermodel.XSSFRelation; @@ -47,7 +48,6 @@ import org.w3c.dom.Element; */ public final class ZipPartMarshaller implements PartMarshaller { private final static POILogger logger = POILogFactory.getLogger(ZipPartMarshaller.class); - private final static int READ_WRITE_FILE_BUFFER_SIZE = 8192; /** * Save the specified part. @@ -80,17 +80,11 @@ public final class ZipPartMarshaller implements PartMarshaller { zos.putNextEntry(partEntry); // Saving data in the ZIP file - InputStream ins = part.getInputStream(); - byte[] buff = new byte[READ_WRITE_FILE_BUFFER_SIZE]; - while (ins.available() > 0) { - int resultRead = ins.read(buff); - if (resultRead == -1) { - // End of file reached - break; - } - zos.write(buff, 0, resultRead); + try (final InputStream ins = part.getInputStream()) { + IOUtils.copy(ins, zos); + } finally { + zos.closeEntry(); } - zos.closeEntry(); } catch (IOException ioe) { logger.log(POILogger.ERROR,"Cannot write: " + part.getPartName() + ": in ZIP", ioe); @@ -178,14 +172,14 @@ public final class ZipPartMarshaller implements PartMarshaller { relPartName.getURI().toASCIIString()).getPath()); try { zos.putNextEntry(ctEntry); - if (!StreamHelper.saveXmlInStream(xmlOutDoc, zos)) { - return false; + try { + return StreamHelper.saveXmlInStream(xmlOutDoc, zos); + } finally { + zos.closeEntry(); } - zos.closeEntry(); } catch (IOException e) { logger.log(POILogger.ERROR,"Cannot create zip entry " + relPartName, e); return false; } - return true; // success } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java new file mode 100644 index 0000000000..347ea50225 --- /dev/null +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java @@ -0,0 +1,53 @@ +/* ==================================================================== + 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.openxml4j.util; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.zip.ZipEntry; + +import org.apache.poi.util.IOUtils; + + +/** + * So we can close the real zip entry and still + * effectively work with it. + * Holds the (decompressed!) data in memory, so + * close this as soon as you can! + */ +/* package */ class ZipArchiveFakeEntry extends ZipEntry { + private final byte[] data; + + ZipArchiveFakeEntry(ZipEntry entry, InputStream inp) throws IOException { + super(entry.getName()); + + final long entrySize = entry.getSize(); + + if (entrySize < -1 || entrySize>=Integer.MAX_VALUE) { + throw new IOException("ZIP entry size is too large or invalid"); + } + + // Grab the de-compressed contents for later + data = (entrySize == -1) ? IOUtils.toByteArray(inp) : IOUtils.toByteArray(inp, (int)entrySize); + } + + public InputStream getInputStream() { + return new ByteArrayInputStream(data); + } +} diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java new file mode 100644 index 0000000000..447baa0d06 --- /dev/null +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java @@ -0,0 +1,243 @@ +/* ==================================================================== + 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.openxml4j.util; + +import static org.apache.poi.openxml4j.util.ZipSecureFile.MAX_ENTRY_SIZE; +import static org.apache.poi.openxml4j.util.ZipSecureFile.MIN_INFLATE_RATIO; + +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.PushbackInputStream; +import java.lang.reflect.Field; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.Locale; +import java.util.zip.InflaterInputStream; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; + +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; +import org.apache.poi.util.SuppressForbidden; + +public class ZipArchiveThresholdInputStream extends PushbackInputStream { + private static final POILogger LOG = POILogFactory.getLogger(ZipArchiveThresholdInputStream.class); + + // don't alert for expanded sizes smaller than 100k + private static final long GRACE_ENTRY_SIZE = 100*1024L; + + private static final String MAX_ENTRY_SIZE_MSG = + "Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file.\n" + + "This may indicates that the file is used to inflate memory usage and thus could pose a security risk.\n" + + "You can adjust this limit via ZipSecureFile.setMaxEntrySize() if you need to work with files which are very large.\n" + + "Counter: %d, cis.counter: %d\n" + + "Limits: MAX_ENTRY_SIZE: %d, Entry: %s"; + + private static final String MIN_INFLATE_RATIO_MSG = + "Zip bomb detected! The file would exceed the max. ratio of compressed file size to the size of the expanded data.\n" + + "This may indicate that the file is used to inflate memory usage and thus could pose a security risk.\n" + + "You can adjust this limit via ZipSecureFile.setMinInflateRatio() if you need to work with files which exceed this limit.\n" + + "Counter: %d, cis.counter: %d, ratio: %f\n" + + "Limits: MIN_INFLATE_RATIO: %f, Entry: %s"; + + private static final String SECURITY_BLOCKED = + "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream"; + + /** + * the reference to the current entry is only used for a more detailed log message in case of an error + */ + private ZipEntry entry; + + private long counter; + private long markPos; + private final ZipArchiveThresholdInputStream cis; + private boolean guardState = true; + + + public ZipArchiveThresholdInputStream(final InputStream zipIS) throws IOException { + super(zipIS); + if (zipIS instanceof InflaterInputStream) { + cis = AccessController.doPrivileged(inject(zipIS)); + } else { + // the inner stream is a ZipFileInputStream, i.e. the data wasn't compressed + cis = null; + } + } + + private ZipArchiveThresholdInputStream(InputStream is, ZipArchiveThresholdInputStream cis) { + super(is); + this.cis = cis; + } + + @SuppressForbidden + private static PrivilegedAction inject(final InputStream zipIS) { + return () -> { + try { + final Field f = FilterInputStream.class.getDeclaredField("in"); + f.setAccessible(true); + final InputStream oldInner = (InputStream)f.get(zipIS); + final ZipArchiveThresholdInputStream inner = new ZipArchiveThresholdInputStream(oldInner, null); + f.set(zipIS, inner); + return inner; + } catch (Exception ex) { + LOG.log(POILogger.WARN, SECURITY_BLOCKED, ex); + } + return null; + }; + } + + @Override + public int read() throws IOException { + int b = in.read(); + if (b > -1) { + advance(1); + } + return b; + } + + @Override + public int read(byte b[], int off, int len) throws IOException { + int cnt = in.read(b, off, len); + if (cnt > -1) { + advance(cnt); + } + return cnt; + } + + @Override + public long skip(long n) throws IOException { + long s = in.skip(n); + counter += s; + return s; + } + + @Override + public synchronized void reset() throws IOException { + counter = markPos; + super.reset(); + } + + /** + * De-/activate threshold check. + * A disabled guard might make sense, when POI is processing its own temporary data (see #59743) + * + * @param guardState {@code true} (= default) enables the threshold check + */ + public void setGuardState(boolean guardState) { + this.guardState = guardState; + } + + public void advance(int advance) throws IOException { + counter += advance; + + if (!guardState) { + return; + } + + final String entryName = entry == null ? "not set" : entry.getName(); + final long cisCount = (cis == null ? 0 : cis.counter); + + // check the file size first, in case we are working on uncompressed streams + if(counter > MAX_ENTRY_SIZE) { + throw new IOException(String.format(Locale.ROOT, MAX_ENTRY_SIZE_MSG, counter, cisCount, MAX_ENTRY_SIZE, entryName)); + } + + // no expanded size? + if (cis == null) { + return; + } + + // don't alert for small expanded size + if (counter <= GRACE_ENTRY_SIZE) { + return; + } + + double ratio = (double)cis.counter/(double)counter; + if (ratio >= MIN_INFLATE_RATIO) { + return; + } + + // one of the limits was reached, report it + throw new IOException(String.format(Locale.ROOT, MIN_INFLATE_RATIO_MSG, counter, cisCount, ratio, MIN_INFLATE_RATIO, entryName)); + } + + public ZipEntry getNextEntry() throws IOException { + if (!(in instanceof ZipInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); + } + counter = 0; + return ((ZipInputStream)in).getNextEntry(); + } + + public void closeEntry() throws IOException { + if (!(in instanceof ZipInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); + } + counter = 0; + ((ZipInputStream)in).closeEntry(); + } + + @Override + public void unread(int b) throws IOException { + if (!(in instanceof PushbackInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); + } + if (--counter < 0) { + counter = 0; + } + ((PushbackInputStream)in).unread(b); + } + + @Override + public void unread(byte[] b, int off, int len) throws IOException { + if (!(in instanceof PushbackInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); + } + counter -= len; + if (--counter < 0) { + counter = 0; + } + ((PushbackInputStream)in).unread(b, off, len); + } + + @Override + @SuppressForbidden("just delegating") + public int available() throws IOException { + return in.available(); + } + + @Override + public boolean markSupported() { + return in.markSupported(); + } + + @Override + public synchronized void mark(int readlimit) { + markPos = counter; + in.mark(readlimit); + } + + /** + * Sets the zip entry for a detailed logging + * @param entry the entry + */ + void setEntry(ZipEntry entry) { + this.entry = entry; + } +} \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java index 8ce82325c5..65ba942941 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java @@ -33,23 +33,32 @@ public interface ZipEntrySource extends Closeable { /** * Returns an Enumeration of all the Entries */ - public Enumeration getEntries(); - + Enumeration getEntries(); + + /** + * Return an entry by its path + * @param path the path in unix-notation + * @return the entry or {@code null} if not found + * + * @since POI 4.0.0 + */ + ZipEntry getEntry(String path); + /** * Returns an InputStream of the decompressed * data that makes up the entry */ - public InputStream getInputStream(ZipEntry entry) throws IOException; + InputStream getInputStream(ZipEntry entry) throws IOException; /** * Indicates we are done with reading, and * resources may be freed */ @Override - public void close() throws IOException; + void close() throws IOException; /** * Has close been called already? */ - public boolean isClosed(); + boolean isClosed(); } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java index 09317d361e..221db9a9ec 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java @@ -16,6 +16,9 @@ ==================================================================== */ package org.apache.poi.openxml4j.util; +import static org.apache.commons.collections4.IteratorUtils.asIterable; +import static org.apache.commons.collections4.IteratorUtils.asIterator; + import java.io.IOException; import java.io.InputStream; import java.util.Enumeration; @@ -33,16 +36,20 @@ public class ZipFileZipEntrySource implements ZipEntrySource { this.zipArchive = zipFile; } + @Override public void close() throws IOException { if(zipArchive != null) { zipArchive.close(); } zipArchive = null; } + + @Override public boolean isClosed() { return (zipArchive == null); } + @Override public Enumeration getEntries() { if (zipArchive == null) throw new IllegalStateException("Zip File is closed"); @@ -50,10 +57,30 @@ public class ZipFileZipEntrySource implements ZipEntrySource { return zipArchive.entries(); } + @Override public InputStream getInputStream(ZipEntry entry) throws IOException { if (zipArchive == null) throw new IllegalStateException("Zip File is closed"); return zipArchive.getInputStream(entry); } + + @Override + public ZipEntry getEntry(final String path) { + String normalizedPath = path.replace('\\', '/'); + + final ZipEntry entry = zipArchive.getEntry(normalizedPath); + if (entry != null) { + return entry; + } + + // the opc spec allows case-insensitive filename matching (see #49609) + for (final ZipEntry ze : asIterable(asIterator(zipArchive.entries()))) { + if (normalizedPath.equalsIgnoreCase(ze.getName().replace('\\','/'))) { + return ze; + } + } + + return null; + } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java index dfa9924617..9c12431ada 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java @@ -16,16 +16,14 @@ ==================================================================== */ package org.apache.poi.openxml4j.util; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; import java.util.Enumeration; -import java.util.Iterator; +import java.util.HashMap; +import java.util.Map; import java.util.zip.ZipEntry; -import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; +import org.apache.commons.collections4.IteratorUtils; /** * Provides a way to get at all the ZipEntries @@ -36,7 +34,7 @@ import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; * done, to free up that memory! */ public class ZipInputStreamZipEntrySource implements ZipEntrySource { - private ArrayList zipEntries; + private final Map zipEntries = new HashMap<>(); /** * Reads all the entries from the ZipInputStream @@ -44,100 +42,53 @@ public class ZipInputStreamZipEntrySource implements ZipEntrySource { * We'll then eat lots of memory, but be able to * work with the entries at-will. */ - public ZipInputStreamZipEntrySource(ThresholdInputStream inp) throws IOException { - zipEntries = new ArrayList<>(); - - boolean going = true; - while(going) { - ZipEntry zipEntry = inp.getNextEntry(); - if(zipEntry == null) { - going = false; - } else { - FakeZipEntry entry = new FakeZipEntry(zipEntry, inp); - inp.closeEntry(); - - zipEntries.add(entry); + public ZipInputStreamZipEntrySource(ZipArchiveThresholdInputStream inp) throws IOException { + for (;;) { + final ZipEntry zipEntry = inp.getNextEntry(); + if (zipEntry == null) { + break; } + zipEntries.put(zipEntry.getName(), new ZipArchiveFakeEntry(zipEntry, inp)); } inp.close(); } + @Override public Enumeration getEntries() { - return new EntryEnumerator(); + return IteratorUtils.asEnumeration(zipEntries.values().iterator()); } - + + @Override public InputStream getInputStream(ZipEntry zipEntry) { - assert (zipEntry instanceof FakeZipEntry); - FakeZipEntry entry = (FakeZipEntry)zipEntry; - return entry.getInputStream(); + assert (zipEntry instanceof ZipArchiveFakeEntry); + return ((ZipArchiveFakeEntry)zipEntry).getInputStream(); } - + + @Override public void close() { // Free the memory - zipEntries = null; + zipEntries.clear(); } + + @Override public boolean isClosed() { - return (zipEntries == null); + return zipEntries.isEmpty(); } - - /** - * Why oh why oh why are Iterator and Enumeration - * still not compatible? - */ - private class EntryEnumerator implements Enumeration { - private Iterator iterator; - - private EntryEnumerator() { - iterator = zipEntries.iterator(); - } - - public boolean hasMoreElements() { - return iterator.hasNext(); - } - public ZipEntry nextElement() { - return iterator.next(); + @Override + public ZipEntry getEntry(final String path) { + final String normalizedPath = path.replace('\\', '/'); + final ZipEntry ze = zipEntries.get(normalizedPath); + if (ze != null) { + return ze; } - } - - /** - * So we can close the real zip entry and still - * effectively work with it. - * Holds the (decompressed!) data in memory, so - * close this as soon as you can! - */ - public static class FakeZipEntry extends ZipEntry { - private byte[] data; - - public FakeZipEntry(ZipEntry entry, InputStream inp) throws IOException { - super(entry.getName()); - - // Grab the de-compressed contents for later - ByteArrayOutputStream baos; - - long entrySize = entry.getSize(); - if (entrySize !=-1) { - if (entrySize>=Integer.MAX_VALUE) { - throw new IOException("ZIP entry size is too large"); - } - - baos = new ByteArrayOutputStream((int) entrySize); - } else { - baos = new ByteArrayOutputStream(); - } - - byte[] buffer = new byte[4096]; - int read = 0; - while( (read = inp.read(buffer)) != -1 ) { - baos.write(buffer, 0, read); + for (final Map.Entry fze : zipEntries.entrySet()) { + if (normalizedPath.equalsIgnoreCase(fze.getKey())) { + return fze.getValue(); } - - data = baos.toByteArray(); - } - - public InputStream getInputStream() { - return new ByteArrayInputStream(data); } + + return null; } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java index 971ba36690..ae2bc192d4 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -18,43 +18,27 @@ package org.apache.poi.openxml4j.util; import java.io.File; -import java.io.FilterInputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.PushbackInputStream; -import java.lang.reflect.Field; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.util.zip.InflaterInputStream; import java.util.zip.ZipEntry; -import java.util.zip.ZipException; import java.util.zip.ZipFile; -import java.util.zip.ZipInputStream; -import org.apache.poi.util.POILogFactory; -import org.apache.poi.util.POILogger; -import org.apache.poi.util.SuppressForbidden; /** * This class wraps a {@link ZipFile} in order to check the * entries for zip bombs - * while reading the archive. - * If a {@link ZipInputStream} is directly used, the wrapper - * can be applied via {@link #addThreshold(InputStream)}. + * while reading the archive.

+ * * The alert limits can be globally defined via {@link #setMaxEntrySize(long)} * and {@link #setMinInflateRatio(double)}. */ public class ZipSecureFile extends ZipFile { - private static final POILogger LOG = POILogFactory.getLogger(ZipSecureFile.class); + /* package */ static double MIN_INFLATE_RATIO = 0.01d; + /* package */ static long MAX_ENTRY_SIZE = 0xFFFFFFFFL; - private static double MIN_INFLATE_RATIO = 0.01d; - private static long MAX_ENTRY_SIZE = 0xFFFFFFFFL; - - // don't alert for expanded sizes smaller than 100k - private final static long GRACE_ENTRY_SIZE = 100*1024L; - - // The default maximum size of extracted text + // The default maximum size of extracted text private static long MAX_TEXT_SIZE = 10*1024*1024L; + + private final String fileName; /** * Sets the ratio between de- and inflated bytes to detect zipbomb. @@ -134,16 +118,14 @@ public class ZipSecureFile extends ZipFile { return MAX_TEXT_SIZE; } - public ZipSecureFile(File file, int mode) throws ZipException, IOException { - super(file, mode); - } - - public ZipSecureFile(File file) throws ZipException, IOException { + public ZipSecureFile(File file) throws IOException { super(file); + this.fileName = file.getAbsolutePath(); } - public ZipSecureFile(String name) throws ZipException, IOException { + public ZipSecureFile(String name) throws IOException { super(name); + this.fileName = new File(name).getAbsolutePath(); } /** @@ -156,176 +138,22 @@ public class ZipSecureFile extends ZipFile { * @param entry the zip file entry * @return the input stream for reading the contents of the specified * zip file entry. - * @throws ZipException if a ZIP format error has occurred * @throws IOException if an I/O error has occurred * @throws IllegalStateException if the zip file has been closed */ @Override @SuppressWarnings("resource") - public InputStream getInputStream(ZipEntry entry) throws IOException { - InputStream zipIS = super.getInputStream(entry); - return addThreshold(zipIS); + public ZipArchiveThresholdInputStream getInputStream(ZipEntry entry) throws IOException { + ZipArchiveThresholdInputStream zatis = new ZipArchiveThresholdInputStream(super.getInputStream(entry)); + zatis.setEntry(entry); + return zatis; } - public static ThresholdInputStream addThreshold(final InputStream zipIS) throws IOException { - ThresholdInputStream newInner; - if (zipIS instanceof InflaterInputStream) { - newInner = AccessController.doPrivileged(new PrivilegedAction() { // NOSONAR - @Override - @SuppressForbidden("TODO: Fix this to not use reflection (it will break in Java 9)! " + - "Better would be to wrap *before* instead of trying to insert wrapper afterwards.") - public ThresholdInputStream run() { - try { - Field f = FilterInputStream.class.getDeclaredField("in"); - f.setAccessible(true); - InputStream oldInner = (InputStream)f.get(zipIS); - ThresholdInputStream newInner2 = new ThresholdInputStream(oldInner, null); - f.set(zipIS, newInner2); - return newInner2; - } catch (Exception ex) { - LOG.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); - } - return null; - } - }); - } else { - // the inner stream is a ZipFileInputStream, i.e. the data wasn't compressed - newInner = null; - } - - return new ThresholdInputStream(zipIS, newInner); - } - - public static class ThresholdInputStream extends PushbackInputStream { - long counter; - long markPos; - ThresholdInputStream cis; - - public ThresholdInputStream(InputStream is, ThresholdInputStream cis) { - super(is); - this.cis = cis; - } - - @Override - public int read() throws IOException { - int b = in.read(); - if (b > -1) { - advance(1); - } - return b; - } - - @Override - public int read(byte b[], int off, int len) throws IOException { - int cnt = in.read(b, off, len); - if (cnt > -1) { - advance(cnt); - } - return cnt; - } - - @Override - public long skip(long n) throws IOException { - long s = in.skip(n); - counter += s; - return s; - } - - @Override - public synchronized void reset() throws IOException { - counter = markPos; - super.reset(); - } - - public void advance(int advance) throws IOException { - counter += advance; - - // check the file size first, in case we are working on uncompressed streams - if(counter > MAX_ENTRY_SIZE) { - throw new IOException("Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file. " - + "This may indicates that the file is used to inflate memory usage and thus could pose a security risk. " - + "You can adjust this limit via ZipSecureFile.setMaxEntrySize() if you need to work with files which are very large. " - + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter) - + "Limits: MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE); - } - - // no expanded size? - if (cis == null) { - return; - } - - // don't alert for small expanded size - if (counter <= GRACE_ENTRY_SIZE) { - return; - } - - double ratio = (double)cis.counter/(double)counter; - if (ratio >= MIN_INFLATE_RATIO) { - return; - } - - // one of the limits was reached, report it - throw new IOException("Zip bomb detected! The file would exceed the max. ratio of compressed file size to the size of the expanded data.\n" - + "This may indicate that the file is used to inflate memory usage and thus could pose a security risk.\n" - + "You can adjust this limit via ZipSecureFile.setMinInflateRatio() if you need to work with files which exceed this limit.\n" - + "Counter: " + counter + ", cis.counter: " + cis.counter + ", ratio: " + ratio + "\n" - + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO); - } - - public ZipEntry getNextEntry() throws IOException { - if (!(in instanceof ZipInputStream)) { - throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); - } - counter = 0; - return ((ZipInputStream)in).getNextEntry(); - } - - public void closeEntry() throws IOException { - if (!(in instanceof ZipInputStream)) { - throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); - } - counter = 0; - ((ZipInputStream)in).closeEntry(); - } - - @Override - public void unread(int b) throws IOException { - if (!(in instanceof PushbackInputStream)) { - throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); - } - if (--counter < 0) { - counter = 0; - } - ((PushbackInputStream)in).unread(b); - } - - @Override - public void unread(byte[] b, int off, int len) throws IOException { - if (!(in instanceof PushbackInputStream)) { - throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); - } - counter -= len; - if (--counter < 0) { - counter = 0; - } - ((PushbackInputStream)in).unread(b, off, len); - } - - @Override - @SuppressForbidden("just delegating") - public int available() throws IOException { - return in.available(); - } - - @Override - public boolean markSupported() { - return in.markSupported(); - } - - @Override - public synchronized void mark(int readlimit) { - markPos = counter; - in.mark(readlimit); - } + /** + * Returns the path name of the ZIP file. + * @return the path name of the ZIP file + */ + public String getName() { + return fileName; } } diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java index 3c748a8f63..ea430bad08 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java @@ -53,15 +53,17 @@ import org.apache.poi.util.TempFile; * sensitive data is not stored in raw format on disk. */ @Beta -public class AesZipFileZipEntrySource implements ZipEntrySource { +public final class AesZipFileZipEntrySource implements ZipEntrySource { private static final POILogger LOG = POILogFactory.getLogger(AesZipFileZipEntrySource.class); + + private static final String PADDING = "PKCS5Padding"; private final File tmpFile; private final ZipFile zipFile; private final Cipher ci; private boolean closed; - public AesZipFileZipEntrySource(File tmpFile, Cipher ci) throws IOException { + private AesZipFileZipEntrySource(File tmpFile, Cipher ci) throws IOException { this.tmpFile = tmpFile; this.zipFile = new ZipFile(tmpFile); this.ci = ci; @@ -76,7 +78,12 @@ public class AesZipFileZipEntrySource implements ZipEntrySource { public Enumeration getEntries() { return zipFile.entries(); } - + + @Override + public ZipEntry getEntry(String path) { + return zipFile.getEntry(path); + } + @Override public InputStream getInputStream(ZipEntry entry) throws IOException { InputStream is = zipFile.getInputStream(entry); @@ -106,14 +113,14 @@ public class AesZipFileZipEntrySource implements ZipEntrySource { sr.nextBytes(ivBytes); sr.nextBytes(keyBytes); final File tmpFile = TempFile.createTempFile("protectedXlsx", ".zip"); - copyToFile(is, tmpFile, CipherAlgorithm.aes128, keyBytes, ivBytes); + copyToFile(is, tmpFile, keyBytes, ivBytes); IOUtils.closeQuietly(is); - return fileToSource(tmpFile, CipherAlgorithm.aes128, keyBytes, ivBytes); + return fileToSource(tmpFile, keyBytes, ivBytes); } - private static void copyToFile(InputStream is, File tmpFile, CipherAlgorithm cipherAlgorithm, byte keyBytes[], byte ivBytes[]) throws IOException, GeneralSecurityException { - SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, cipherAlgorithm.jceId); - Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, "PKCS5Padding"); + private static void copyToFile(InputStream is, File tmpFile, byte keyBytes[], byte ivBytes[]) throws IOException, GeneralSecurityException { + SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, CipherAlgorithm.aes128.jceId); + Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, PADDING); ZipInputStream zis = new ZipInputStream(is); FileOutputStream fos = new FileOutputStream(tmpFile); @@ -146,9 +153,9 @@ public class AesZipFileZipEntrySource implements ZipEntrySource { zis.close(); } - private static AesZipFileZipEntrySource fileToSource(File tmpFile, CipherAlgorithm cipherAlgorithm, byte keyBytes[], byte ivBytes[]) throws ZipException, IOException { - SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, cipherAlgorithm.jceId); - Cipher ciDec = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.DECRYPT_MODE, "PKCS5Padding"); + private static AesZipFileZipEntrySource fileToSource(File tmpFile, byte keyBytes[], byte ivBytes[]) throws ZipException, IOException { + SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, CipherAlgorithm.aes128.jceId); + Cipher ciDec = CryptoFunctions.getCipher(skeySpec, CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.DECRYPT_MODE, PADDING); return new AesZipFileZipEntrySource(tmpFile, ciDec); } diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/EncryptedTempData.java b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/EncryptedTempData.java index 7175217d8a..8cfd93c0f5 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/EncryptedTempData.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/EncryptedTempData.java @@ -48,6 +48,7 @@ public class EncryptedTempData { private static POILogger LOG = POILogFactory.getLogger(EncryptedTempData.class); private final static CipherAlgorithm cipherAlgorithm = CipherAlgorithm.aes128; + private final static String PADDING = "PKCS5Padding"; private final SecretKeySpec skeySpec; private final byte[] ivBytes; private final File tempFile; @@ -63,12 +64,12 @@ public class EncryptedTempData { } public OutputStream getOutputStream() throws IOException { - Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, null); + Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, PADDING); return new CipherOutputStream(new FileOutputStream(tempFile), ciEnc); } public InputStream getInputStream() throws IOException { - Cipher ciDec = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.DECRYPT_MODE, null); + Cipher ciDec = CryptoFunctions.getCipher(skeySpec, cipherAlgorithm, ChainingMode.cbc, ivBytes, Cipher.DECRYPT_MODE, PADDING); return new CipherInputStream(new FileInputStream(tempFile), ciDec); } diff --git a/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java b/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java index ffe9d2a737..e525415794 100644 --- a/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java +++ b/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java @@ -24,9 +24,9 @@ import java.io.FileOutputStream; import java.io.OutputStream; import java.util.Enumeration; import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; import org.apache.poi.openxml4j.opc.internal.ZipHelper; +import org.apache.poi.openxml4j.util.ZipSecureFile; import org.apache.poi.util.DocumentHelper; import org.apache.poi.util.IOUtils; import org.apache.xmlbeans.XmlException; @@ -41,14 +41,13 @@ import org.w3c.dom.Document; */ public final class XSSFDump { + private XSSFDump() {} + public static void main(String[] args) throws Exception { - for (int i = 0; i < args.length; i++) { - System.out.println("Dumping " + args[i]); - ZipFile zip = ZipHelper.openZipFile(args[i]); - try { + for (String arg : args) { + System.out.println("Dumping " + arg); + try (ZipSecureFile zip = ZipHelper.openZipFile(arg)) { dump(zip); - } finally { - zip.close(); } } } @@ -72,7 +71,7 @@ public final class XSSFDump { } - public static void dump(ZipFile zip) throws Exception { + public static void dump(ZipSecureFile zip) throws Exception { String zipname = zip.getName(); int sep = zipname.lastIndexOf('.'); File root = new File(zipname.substring(0, sep)); @@ -90,8 +89,7 @@ public final class XSSFDump { } File f = new File(root, entry.getName()); - OutputStream out = new FileOutputStream(f); - try { + try (final OutputStream out = new FileOutputStream(f)) { if (entry.getName().endsWith(".xml") || entry.getName().endsWith(".vml") || entry.getName().endsWith(".rels")) { try { Document doc = DocumentHelper.readDocument(zip.getInputStream(entry)); @@ -106,8 +104,6 @@ public final class XSSFDump { } else { IOUtils.copy(zip.getInputStream(entry), out); } - } finally { - out.close(); } } } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java index acf9d0f303..73141707c9 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -36,6 +36,7 @@ import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; import org.apache.poi.openxml4j.opc.OPCPackage; +import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream; import org.apache.poi.openxml4j.util.ZipEntrySource; import org.apache.poi.openxml4j.util.ZipFileZipEntrySource; import org.apache.poi.ss.SpreadsheetVersion; @@ -50,7 +51,13 @@ import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.SheetVisibility; import org.apache.poi.ss.usermodel.Workbook; -import org.apache.poi.util.*; +import org.apache.poi.util.IOUtils; +import org.apache.poi.util.Internal; +import org.apache.poi.util.NotImplemented; +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; +import org.apache.poi.util.Removal; +import org.apache.poi.util.TempFile; import org.apache.poi.xssf.model.SharedStringsTable; import org.apache.poi.xssf.usermodel.XSSFChartSheet; import org.apache.poi.xssf.usermodel.XSSFSheet; @@ -369,13 +376,17 @@ public class SXSSFWorkbook implements Workbook { } protected void injectData(ZipEntrySource zipEntrySource, OutputStream out) throws IOException { - try { - try (ZipOutputStream zos = new ZipOutputStream(out)) { - Enumeration en = zipEntrySource.getEntries(); - while (en.hasMoreElements()) { - ZipEntry ze = en.nextElement(); - zos.putNextEntry(new ZipEntry(ze.getName())); - InputStream is = zipEntrySource.getInputStream(ze); + try (ZipOutputStream zos = new ZipOutputStream(out)) { + Enumeration en = zipEntrySource.getEntries(); + while (en.hasMoreElements()) { + ZipEntry ze = en.nextElement(); + zos.putNextEntry(new ZipEntry(ze.getName())); + try (final InputStream is = zipEntrySource.getInputStream(ze)) { + if (is instanceof ZipArchiveThresholdInputStream) { + // #59743 - disable Threshold handling for SXSSF copy + // as users tend to put too much repetitive data in when using SXSSF :) + ((ZipArchiveThresholdInputStream)is).setGuardState(false); + } XSSFSheet xSheet = getSheetFromZipEntryName(ze.getName()); // See bug 56557, we should not inject data into the special ChartSheets if (xSheet != null && !(xSheet instanceof XSSFChartSheet)) { @@ -386,7 +397,6 @@ public class SXSSFWorkbook implements Workbook { } else { IOUtils.copy(is, zos); } - is.close(); } } } finally { diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java index 8cdcd61ad9..b37de1816e 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java @@ -17,6 +17,7 @@ package org.apache.poi.openxml4j.opc; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -34,13 +35,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.PushbackInputStream; -import java.lang.reflect.InvocationTargetException; import java.net.URI; import java.net.URISyntaxException; +import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; import java.util.List; import java.util.TreeMap; +import java.util.function.BiConsumer; import java.util.regex.Pattern; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -53,7 +55,6 @@ import org.apache.poi.POITextExtractor; import org.apache.poi.POIXMLException; import org.apache.poi.UnsupportedFileFormatException; import org.apache.poi.extractor.ExtractorFactory; -import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.InvalidOperationException; @@ -66,6 +67,8 @@ import org.apache.poi.openxml4j.opc.internal.FileHelper; import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart; import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.openxml4j.util.ZipSecureFile; +import org.apache.poi.sl.usermodel.SlideShow; +import org.apache.poi.sl.usermodel.SlideShowFactory; import org.apache.poi.ss.usermodel.Workbook; import org.apache.poi.ss.usermodel.WorkbookFactory; import org.apache.poi.util.DocumentHelper; @@ -74,17 +77,26 @@ import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.util.TempFile; import org.apache.poi.xssf.XSSFTestDataSamples; +import org.apache.poi.xwpf.usermodel.XWPFRelation; import org.apache.xmlbeans.XmlException; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; public final class TestPackage { private static final POILogger logger = POILogFactory.getLogger(TestPackage.class); + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + /** * Test that just opening and closing the file doesn't alter the document. */ @@ -114,7 +126,7 @@ public final class TestPackage { */ @Test public void createGetsContentTypes() - throws IOException, InvalidFormatException, SecurityException, IllegalArgumentException, NoSuchFieldException, IllegalAccessException { + throws IOException, InvalidFormatException, SecurityException, IllegalArgumentException { File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestCreatePackageTMP.docx"); // Zap the target file, in case of an earlier run @@ -596,7 +608,7 @@ public final class TestPackage { } @Test - public void getPartsByName() throws IOException, InvalidFormatException { + public void getPartsByName() throws InvalidFormatException { String filepath = OpenXML4JTestDataSamples.getSampleFileName("sample.docx"); @SuppressWarnings("resource") @@ -653,7 +665,7 @@ public final class TestPackage { @Test public void replaceContentType() - throws IOException, InvalidFormatException, SecurityException, IllegalArgumentException, NoSuchFieldException, IllegalAccessException { + throws IOException, InvalidFormatException, SecurityException, IllegalArgumentException { InputStream is = OpenXML4JTestDataSamples.openSampleStream("sample.xlsx"); @SuppressWarnings("resource") OPCPackage p = OPCPackage.open(is); @@ -760,163 +772,175 @@ public final class TestPackage { } } - @Test(expected=IOException.class) + /** + * Zip bomb handling test + * + * see bug #50090 / #56865 + */ + @Test public void zipBombCreateAndHandle() throws IOException, EncryptedDocumentException, InvalidFormatException { - // #50090 / #56865 - ZipFile zipFile = ZipHelper.openZipFile(OpenXML4JTestDataSamples.getSampleFile("sample.xlsx")); - assertNotNull(zipFile); - ByteArrayOutputStream bos = new ByteArrayOutputStream(2500000); - ZipOutputStream append = new ZipOutputStream(bos); - // first, copy contents from existing war - Enumeration entries = zipFile.entries(); - while (entries.hasMoreElements()) { - ZipEntry e2 = entries.nextElement(); - ZipEntry e = new ZipEntry(e2.getName()); - e.setTime(e2.getTime()); - e.setComment(e2.getComment()); - e.setSize(e2.getSize()); - - append.putNextEntry(e); - if (!e.isDirectory()) { - InputStream is = zipFile.getInputStream(e); - if (e.getName().equals("[Content_Types].xml")) { - ByteArrayOutputStream bos2 = new ByteArrayOutputStream(); - IOUtils.copy(is, bos2); - long size = bos2.size()-"".length(); - append.write(bos2.toByteArray(), 0, (int)size); - byte spam[] = new byte[0x7FFF]; - for (int i=0; i".getBytes("UTF-8")); - size += 8; - e.setSize(size); - } else { - IOUtils.copy(is, append); - } - is.close(); - } - append.closeEntry(); - } - - append.close(); - zipFile.close(); - byte buf[] = bos.toByteArray(); - //noinspection UnusedAssignment - bos = null; - - Workbook wb = WorkbookFactory.create(new ByteArrayInputStream(buf)); - wb.getSheetAt(0); - wb.close(); - zipFile.close(); + try (ZipFile zipFile = ZipHelper.openZipFile(OpenXML4JTestDataSamples.getSampleFile("sample.xlsx")); + ZipOutputStream append = new ZipOutputStream(bos)) { + assertNotNull(zipFile); + + // first, copy contents from existing war + Enumeration entries = zipFile.entries(); + while (entries.hasMoreElements()) { + final ZipEntry eIn = entries.nextElement(); + final ZipEntry eOut = new ZipEntry(eIn.getName()); + eOut.setTime(eIn.getTime()); + eOut.setComment(eIn.getComment()); + eOut.setSize(eIn.getSize()); + + append.putNextEntry(eOut); + if (!eOut.isDirectory()) { + try (InputStream is = zipFile.getInputStream(eIn)) { + if (eOut.getName().equals("[Content_Types].xml")) { + ByteArrayOutputStream bos2 = new ByteArrayOutputStream(); + IOUtils.copy(is, bos2); + long size = bos2.size() - "".length(); + append.write(bos2.toByteArray(), 0, (int) size); + byte spam[] = new byte[0x7FFF]; + Arrays.fill(spam, (byte) ' '); + // 0x7FFF0000 is the maximum for 32-bit zips, but less still works + while (size < 0x7FFF00) { + append.write(spam); + size += spam.length; + } + append.write("".getBytes("UTF-8")); + size += 8; + eOut.setSize(size); + } else { + IOUtils.copy(is, append); + } + } + } + append.closeEntry(); + } + } + + expectedEx.expect(IOException.class); + expectedEx.expectMessage("Zip bomb detected!"); + + try (Workbook wb = WorkbookFactory.create(new ByteArrayInputStream(bos.toByteArray()))) { + wb.getSheetAt(0); + } } - @Test - public void zipBombSampleFiles() throws IOException, OpenXML4JException, XmlException { - openZipBombFile("poc-shared-strings.xlsx"); - openZipBombFile("poc-xmlbomb.xlsx"); - openZipBombFile("poc-xmlbomb-empty.xlsx"); + @Test + public void testZipEntityExpansionTerminates() throws IOException, OpenXML4JException, XmlException { + expectedEx.expect(IllegalStateException.class); + expectedEx.expectMessage("The text would exceed the max allowed overall size of extracted text."); + openXmlBombFile("poc-shared-strings.xlsx"); } - private void openZipBombFile(String file) throws IOException, OpenXML4JException, XmlException { - try { - Workbook wb = XSSFTestDataSamples.openSampleWorkbook(file); - wb.close(); + @Test + public void testZipEntityExpansionSharedStringTableEvents() throws IOException, OpenXML4JException, XmlException { + boolean before = ExtractorFactory.getThreadPrefersEventExtractors(); + ExtractorFactory.setThreadPrefersEventExtractors(true); + try { + expectedEx.expect(IllegalStateException.class); + expectedEx.expectMessage("The text would exceed the max allowed overall size of extracted text."); + openXmlBombFile("poc-shared-strings.xlsx"); + } finally { + ExtractorFactory.setThreadPrefersEventExtractors(before); + } + } - try (POITextExtractor extractor = ExtractorFactory.createExtractor(HSSFTestDataSamples.getSampleFile("poc-shared-strings.xlsx"))) { - assertNotNull(extractor); - extractor.getText(); - } - fail("Should catch an exception because of a ZipBomb"); - } catch (IllegalStateException e) { - if(!e.getMessage().contains("The text would exceed the max allowed overall size of extracted text.")) { - throw e; - } - } catch (POIXMLException e) { - checkForZipBombException(e); + @Test + public void testZipEntityExpansionExceedsMemory() throws IOException, OpenXML4JException, XmlException { + expectedEx.expect(POIXMLException.class); + expectedEx.expectMessage("Unable to parse xml bean"); + expectedEx.expectCause(getCauseMatcher(SAXParseException.class, "The parser has encountered more than")); + openXmlBombFile("poc-xmlbomb.xlsx"); + } + + @Test + public void testZipEntityExpansionExceedsMemory2() throws IOException, OpenXML4JException, XmlException { + expectedEx.expect(POIXMLException.class); + expectedEx.expectMessage("Unable to parse xml bean"); + expectedEx.expectCause(getCauseMatcher(SAXParseException.class, "The parser has encountered more than")); + openXmlBombFile("poc-xmlbomb-empty.xlsx"); + } + + private void openXmlBombFile(String file) throws IOException, OpenXML4JException, XmlException { + final double minInf = ZipSecureFile.getMinInflateRatio(); + ZipSecureFile.setMinInflateRatio(0.002); + try (POITextExtractor extractor = ExtractorFactory.createExtractor(XSSFTestDataSamples.getSampleFile(file))) { + assertNotNull(extractor); + extractor.getText(); + } finally { + ZipSecureFile.setMinInflateRatio(minInf); } } - + @Test - public void zipBombCheckSizes() throws IOException, EncryptedDocumentException, InvalidFormatException { - File file = OpenXML4JTestDataSamples.getSampleFile("sample.xlsx"); + public void zipBombCheckSizesWithinLimits() throws IOException, EncryptedDocumentException, InvalidFormatException { + getZipStatsAndConsume((max_size, min_ratio) -> { + // use values close to, but within the limits + ZipSecureFile.setMinInflateRatio(min_ratio - 0.002); + assertEquals(min_ratio - 0.002, ZipSecureFile.getMinInflateRatio(), 0.00001); + ZipSecureFile.setMaxEntrySize(max_size + 1); + assertEquals(max_size + 1, ZipSecureFile.getMaxEntrySize()); + }); + } - try { - double min_ratio = Double.MAX_VALUE; - long max_size = 0; - ZipFile zf = ZipHelper.openZipFile(file); - assertNotNull(zf); - Enumeration entries = zf.entries(); - while (entries.hasMoreElements()) { - ZipEntry ze = entries.nextElement(); - double ratio = (double)ze.getCompressedSize() / (double)ze.getSize(); - min_ratio = Math.min(min_ratio, ratio); - max_size = Math.max(max_size, ze.getSize()); - } - zf.close(); - - // use values close to, but within the limits - ZipSecureFile.setMinInflateRatio(min_ratio-0.002); - assertEquals(min_ratio-0.002, ZipSecureFile.getMinInflateRatio(), 0.00001); - ZipSecureFile.setMaxEntrySize(max_size+1); - assertEquals(max_size+1, ZipSecureFile.getMaxEntrySize()); - - WorkbookFactory.create(file, null, true).close(); - - // check ratio out of bounds - ZipSecureFile.setMinInflateRatio(min_ratio+0.002); - try { - WorkbookFactory.create(file, null, true).close(); - // this is a bit strange, as there will be different exceptions thrown - // depending if this executed via "ant test" or within eclipse - // maybe a difference in JDK ... - } catch (InvalidFormatException | POIXMLException e) { - checkForZipBombException(e); - } + @Test + public void zipBombCheckSizesRatioTooSmall() throws IOException, EncryptedDocumentException, InvalidFormatException { + expectedEx.expect(POIXMLException.class); + expectedEx.expectMessage("You can adjust this limit via ZipSecureFile.setMinInflateRatio()"); + getZipStatsAndConsume((max_size, min_ratio) -> { + // check ratio out of bounds + ZipSecureFile.setMinInflateRatio(min_ratio+0.002); + }); + } + @Test + public void zipBombCheckSizesSizeTooBig() throws IOException, EncryptedDocumentException, InvalidFormatException { + expectedEx.expect(POIXMLException.class); + expectedEx.expectMessage("You can adjust this limit via ZipSecureFile.setMaxEntrySize()"); + getZipStatsAndConsume((max_size, min_ratio) -> { // check max entry size ouf of bounds - ZipSecureFile.setMinInflateRatio(min_ratio-0.002); - ZipSecureFile.setMaxEntrySize(max_size-1); - try { - WorkbookFactory.create(file, null, true).close(); - } catch (InvalidFormatException | POIXMLException e) { - checkForZipBombException(e); - } - } finally { - // reset otherwise a lot of ooxml tests will fail - ZipSecureFile.setMinInflateRatio(0.01d); - ZipSecureFile.setMaxEntrySize(0xFFFFFFFFL); - } - } + ZipSecureFile.setMinInflateRatio(min_ratio-0.002); + ZipSecureFile.setMaxEntrySize(max_size-100); + }); + } - private void checkForZipBombException(Throwable e) { - // unwrap InvocationTargetException as they usually contain the nested exception in the "target" member - if(e instanceof InvocationTargetException) { - e = ((InvocationTargetException)e).getTargetException(); - } - - String msg = e.getMessage(); - if(msg != null && (msg.startsWith("Zip bomb detected!") || - msg.contains("The parser has encountered more than \"4,096\" entity expansions in this document;") || - msg.contains("The parser has encountered more than \"4096\" entity expansions in this document;"))) { - return; - } - - // recursively check the causes for the message as it can be nested further down in the exception-tree - if(e.getCause() != null && e.getCause() != e) { - checkForZipBombException(e.getCause()); - return; - } + private void getZipStatsAndConsume(BiConsumer ratioCon) throws IOException, InvalidFormatException { + // use a test file with a xml file bigger than 100k (ZipArchiveThresholdInputStream.GRACE_ENTRY_SIZE) + final File file = XSSFTestDataSamples.getSampleFile("poc-shared-strings.xlsx"); - throw new IllegalStateException("Expected to catch an Exception because of a detected Zip Bomb, but did not find the related error message in the exception", e); - } + double min_ratio = Double.MAX_VALUE; + long max_size = 0; + try (ZipFile zf = ZipHelper.openZipFile(file)) { + assertNotNull(zf); + Enumeration entries = zf.entries(); + while (entries.hasMoreElements()) { + ZipEntry ze = entries.nextElement(); + if (ze.getSize() == 0) { + continue; + } + // add zip entry header ~ 30 bytes + long size = ze.getSize()+30; + double ratio = ze.getCompressedSize() / (double)size; + min_ratio = Math.min(min_ratio, ratio); + max_size = Math.max(max_size, size); + } + } + ratioCon.accept(max_size, min_ratio); + + //noinspection EmptyTryBlock,unused + try (Workbook wb = WorkbookFactory.create(file, null, true)) { + } finally { + // reset otherwise a lot of ooxml tests will fail + ZipSecureFile.setMinInflateRatio(0.01d); + ZipSecureFile.setMaxEntrySize(0xFFFFFFFFL); + } + } @Test public void testConstructors() throws IOException { @@ -926,10 +950,6 @@ public final class TestPackage { assertNotNull(zipFile.getName()); zipFile.close(); - zipFile = new ZipSecureFile(file, ZipFile.OPEN_READ); - assertNotNull(zipFile.getName()); - zipFile.close(); - zipFile = new ZipSecureFile(file.getAbsolutePath()); assertNotNull(zipFile.getName()); zipFile.close(); @@ -948,7 +968,7 @@ public final class TestPackage { // bug 60128 @Test(expected=NotOfficeXmlFileException.class) - public void testCorruptFile() throws IOException, InvalidFormatException { + public void testCorruptFile() throws InvalidFormatException { File file = OpenXML4JTestDataSamples.getSampleFile("invalid.xlsx"); OPCPackage.open(file, PackageAccess.READ); } @@ -976,4 +996,148 @@ public final class TestPackage { } } } + + @Test + public void testBug56479() throws Exception { + InputStream is = OpenXML4JTestDataSamples.openSampleStream("dcterms_bug_56479.zip"); + OPCPackage p = OPCPackage.open(is); + + // Check we found the contents of it + boolean foundCoreProps = false, foundDocument = false, foundTheme1 = false; + for (final PackagePart part : p.getParts()) { + final String partName = part.getPartName().toString(); + final String contentType = part.getContentType(); + if ("/docProps/core.xml".equals(partName)) { + assertEquals(ContentTypes.CORE_PROPERTIES_PART, contentType); + foundCoreProps = true; + } + if ("/word/document.xml".equals(partName)) { + assertEquals(XWPFRelation.DOCUMENT.getContentType(), contentType); + foundDocument = true; + } + if ("/word/theme/theme1.xml".equals(partName)) { + assertEquals(XWPFRelation.THEME.getContentType(), contentType); + foundTheme1 = true; + } + } + assertTrue("Core not found in " + p.getParts(), foundCoreProps); + assertFalse("Document should not be found in " + p.getParts(), foundDocument); + assertFalse("Theme1 should not found in " + p.getParts(), foundTheme1); + p.close(); + is.close(); + } + + @Test + public void unparseableCentralDirectory() throws IOException { + File f = OpenXML4JTestDataSamples.getSampleFile("at.pzp.www_uploads_media_PP_Scheinecker-jdk6error.pptx"); + SlideShow ppt = SlideShowFactory.create(f, null, true); + ppt.close(); + } + + @Test + public void testClosingStreamOnException() throws IOException { + InputStream is = OpenXML4JTestDataSamples.openSampleStream("dcterms_bug_56479.zip"); + File tmp = File.createTempFile("poi-test-truncated-zip", ""); + // create a corrupted zip file by truncating a valid zip file to the first 100 bytes + OutputStream os = new FileOutputStream(tmp); + for (int i = 0; i < 100; i++) { + os.write(is.read()); + } + os.flush(); + os.close(); + is.close(); + + // feed the corrupted zip file to OPCPackage + try { + OPCPackage.open(tmp, PackageAccess.READ); + } catch (Exception e) { + // expected: the zip file is invalid + // this test does not care if open() throws an exception or not. + } + // If the stream is not closed on exception, it will keep a file descriptor to tmp, + // and requests to the OS to delete the file will fail. + assertTrue("Can't delete tmp file", tmp.delete()); + } + + /** + * If ZipPackage is passed an invalid file, a call to close + * (eg from the OPCPackage open method) should tidy up the + * stream / file the broken file is being read from. + * See bug #60128 for more + */ + @Test(expected = NotOfficeXmlFileException.class) + public void testTidyStreamOnInvalidFile1() throws Exception { + openInvalidFile("SampleSS.ods", false); + } + + @Test(expected = NotOfficeXmlFileException.class) + public void testTidyStreamOnInvalidFile2() throws Exception { + openInvalidFile("SampleSS.ods", true); + } + + @Test(expected = NotOfficeXmlFileException.class) + public void testTidyStreamOnInvalidFile3() throws Exception { + openInvalidFile("SampleSS.txt", false); + } + + @Test(expected = NotOfficeXmlFileException.class) + public void testTidyStreamOnInvalidFile4() throws Exception { + openInvalidFile("SampleSS.txt", true); + } + + private static void openInvalidFile(final String name, final boolean useStream) throws IOException, InvalidFormatException { + // Spreadsheet has a good mix of alternate file types + final POIDataSamples files = POIDataSamples.getSpreadSheetInstance(); + ZipPackage pkgTest = null; + try (final InputStream is = (useStream) ? files.openResourceAsStream(name) : null) { + try (final ZipPackage pkg = (useStream) ? new ZipPackage(is, PackageAccess.READ) : new ZipPackage(files.getFile(name), PackageAccess.READ)) { + pkgTest = pkg; + assertNotNull(pkg.getZipArchive()); +// assertFalse(pkg.getZipArchive().isClosed()); + pkg.getParts(); + fail("Shouldn't work"); + } + } finally { + if (pkgTest != null) { + assertNotNull(pkgTest.getZipArchive()); + assertTrue(pkgTest.getZipArchive().isClosed()); + } + } + } + + @SuppressWarnings("SameParameterValue") + private static AnyCauseMatcher getCauseMatcher(Class cause, String message) { + // junit is only using hamcrest-core, so instead of adding hamcrest-beans, we provide the throwable + // search with the basics... + // see https://stackoverflow.com/a/47703937/2066598 + return new AnyCauseMatcher<>(cause, message); + } + + private static class AnyCauseMatcher extends TypeSafeMatcher { + private final Class expectedType; + private final String expectedMessage; + + AnyCauseMatcher(Class expectedType, String expectedMessage) { + this.expectedType = expectedType; + this.expectedMessage = expectedMessage; + } + + @Override + protected boolean matchesSafely(final Throwable root) { + for (Throwable t = root; t != null; t = t.getCause()) { + if (t.getClass().isAssignableFrom(expectedType) && t.getMessage().contains(expectedMessage)) { + return true; + } + } + return false; + } + + @Override + public void describeTo(Description description) { + description.appendText("expects type ") + .appendValue(expectedType) + .appendText(" and a message ") + .appendValue(expectedMessage); + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java deleted file mode 100644 index 88c5bbb1ad..0000000000 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java +++ /dev/null @@ -1,246 +0,0 @@ -/* ==================================================================== - 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.openxml4j.opc; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.io.UnsupportedEncodingException; - -import org.apache.poi.POIDataSamples; -import org.apache.poi.POITextExtractor; -import org.apache.poi.POIXMLException; -import org.apache.poi.extractor.ExtractorFactory; -import org.apache.poi.hssf.HSSFTestDataSamples; -import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; -import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException; -import org.apache.poi.sl.usermodel.SlideShow; -import org.apache.poi.sl.usermodel.SlideShowFactory; -import org.apache.poi.ss.usermodel.Workbook; -import org.apache.poi.ss.usermodel.WorkbookFactory; -import org.apache.poi.xssf.XSSFTestDataSamples; -import org.apache.poi.xwpf.usermodel.XWPFRelation; -import org.apache.xmlbeans.XmlException; -import org.junit.Test; - -public class TestZipPackage { - @Test - public void testBug56479() throws Exception { - InputStream is = OpenXML4JTestDataSamples.openSampleStream("dcterms_bug_56479.zip"); - OPCPackage p = OPCPackage.open(is); - - // Check we found the contents of it - boolean foundCoreProps = false, foundDocument = false, foundTheme1 = false; - for (final PackagePart part : p.getParts()) { - final String partName = part.getPartName().toString(); - final String contentType = part.getContentType(); - if ("/docProps/core.xml".equals(partName)) { - assertEquals(ContentTypes.CORE_PROPERTIES_PART, contentType); - foundCoreProps = true; - } - if ("/word/document.xml".equals(partName)) { - assertEquals(XWPFRelation.DOCUMENT.getContentType(), contentType); - foundDocument = true; - } - if ("/word/theme/theme1.xml".equals(partName)) { - assertEquals(XWPFRelation.THEME.getContentType(), contentType); - foundTheme1 = true; - } - } - assertTrue("Core not found in " + p.getParts(), foundCoreProps); - assertFalse("Document should not be found in " + p.getParts(), foundDocument); - assertFalse("Theme1 should not found in " + p.getParts(), foundTheme1); - p.close(); - is.close(); - } - - @Test - public void testZipEntityExpansionTerminates() throws IOException { - try { - Workbook wb = XSSFTestDataSamples.openSampleWorkbook("poc-xmlbomb.xlsx"); - wb.close(); - fail("Should catch exception due to entity expansion limitations"); - } catch (POIXMLException e) { - assertEntityLimitReached(e); - } - } - - private void assertEntityLimitReached(Exception e) throws UnsupportedEncodingException { - ByteArrayOutputStream str = new ByteArrayOutputStream(); - try (PrintWriter writer = new PrintWriter(new OutputStreamWriter(str, "UTF-8"))) { - e.printStackTrace(writer); - } - String string = new String(str.toByteArray(), "UTF-8"); - assertTrue("Had: " + string, string.contains("The parser has encountered more than")); - } - - @Test - public void testZipEntityExpansionExceedsMemory() throws Exception { - try { - Workbook wb = WorkbookFactory.create(XSSFTestDataSamples.openSamplePackage("poc-xmlbomb.xlsx")); - wb.close(); - fail("Should catch exception due to entity expansion limitations"); - } catch (POIXMLException e) { - assertEntityLimitReached(e); - } - - try { - try (POITextExtractor extractor = ExtractorFactory.createExtractor(HSSFTestDataSamples.getSampleFile("poc-xmlbomb.xlsx"))) { - assertNotNull(extractor); - - try { - extractor.getText(); - } catch (IllegalStateException e) { - // expected due to shared strings expansion - } - } - } catch (POIXMLException e) { - assertEntityLimitReached(e); - } - } - - @Test - public void testZipEntityExpansionSharedStringTable() throws Exception { - Workbook wb = WorkbookFactory.create(XSSFTestDataSamples.openSamplePackage("poc-shared-strings.xlsx")); - wb.close(); - - try (POITextExtractor extractor = ExtractorFactory.createExtractor(HSSFTestDataSamples.getSampleFile("poc-shared-strings.xlsx"))) { - assertNotNull(extractor); - - try { - extractor.getText(); - } catch (IllegalStateException e) { - // expected due to shared strings expansion - } - } - } - - @Test - public void testZipEntityExpansionSharedStringTableEvents() throws Exception { - boolean before = ExtractorFactory.getThreadPrefersEventExtractors(); - ExtractorFactory.setThreadPrefersEventExtractors(true); - try { - try (POITextExtractor extractor = ExtractorFactory.createExtractor(HSSFTestDataSamples.getSampleFile("poc-shared-strings.xlsx"))) { - assertNotNull(extractor); - - try { - extractor.getText(); - } catch (IllegalStateException e) { - // expected due to shared strings expansion - } - } - } catch (XmlException e) { - assertEntityLimitReached(e); - } finally { - ExtractorFactory.setThreadPrefersEventExtractors(before); - } - } - - @Test - public void unparseableCentralDirectory() throws IOException { - File f = OpenXML4JTestDataSamples.getSampleFile("at.pzp.www_uploads_media_PP_Scheinecker-jdk6error.pptx"); - SlideShow ppt = SlideShowFactory.create(f, null, true); - ppt.close(); - } - - @Test - public void testClosingStreamOnException() throws IOException { - InputStream is = OpenXML4JTestDataSamples.openSampleStream("dcterms_bug_56479.zip"); - File tmp = File.createTempFile("poi-test-truncated-zip", ""); - // create a corrupted zip file by truncating a valid zip file to the first 100 bytes - OutputStream os = new FileOutputStream(tmp); - for (int i = 0; i < 100; i++) { - os.write(is.read()); - } - os.flush(); - os.close(); - is.close(); - - // feed the corrupted zip file to OPCPackage - try { - OPCPackage.open(tmp, PackageAccess.READ); - } catch (Exception e) { - // expected: the zip file is invalid - // this test does not care if open() throws an exception or not. - } - // If the stream is not closed on exception, it will keep a file descriptor to tmp, - // and requests to the OS to delete the file will fail. - assertTrue("Can't delete tmp file", tmp.delete()); - } - - /** - * If ZipPackage is passed an invalid file, a call to close - * (eg from the OPCPackage open method) should tidy up the - * stream / file the broken file is being read from. - * See bug #60128 for more - */ - @Test - public void testTidyStreamOnInvalidFile() throws Exception { - // Spreadsheet has a good mix of alternate file types - POIDataSamples files = POIDataSamples.getSpreadSheetInstance(); - - File[] notValidF = new File[] { - files.getFile("SampleSS.ods"), files.getFile("SampleSS.txt") - }; - InputStream[] notValidS = new InputStream[] { - files.openResourceAsStream("SampleSS.ods"), files.openResourceAsStream("SampleSS.txt") - }; - - for (File notValid : notValidF) { - ZipPackage pkg = new ZipPackage(notValid, PackageAccess.READ); - assertNotNull(pkg.getZipArchive()); - assertFalse(pkg.getZipArchive().isClosed()); - try { - pkg.getParts(); - fail("Shouldn't work"); - } catch (NotOfficeXmlFileException e) { - // expected here - } - pkg.close(); - - assertNotNull(pkg.getZipArchive()); - assertTrue(pkg.getZipArchive().isClosed()); - } - for (InputStream notValid : notValidS) { - ZipPackage pkg = new ZipPackage(notValid, PackageAccess.READ); - assertNotNull(pkg.getZipArchive()); - assertFalse(pkg.getZipArchive().isClosed()); - try { - pkg.getParts(); - fail("Shouldn't work"); - } catch (NotOfficeXmlFileException e) { - // expected here - } - pkg.close(); - - assertNotNull(pkg.getZipArchive()); - assertTrue(pkg.getZipArchive().isClosed()); - } - } -} diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/ZipFileAssert.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/ZipFileAssert.java index 330720fd30..3706085c19 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/ZipFileAssert.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/ZipFileAssert.java @@ -18,6 +18,7 @@ package org.apache.poi.openxml4j.opc; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -31,20 +32,26 @@ import java.util.TreeMap; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; -import org.junit.Assert; - import junit.framework.AssertionFailedError; +import org.apache.poi.util.IOUtils; +import org.junit.Assert; +import org.xmlunit.builder.DiffBuilder; +import org.xmlunit.builder.Input; +import org.xmlunit.diff.Comparison; +import org.xmlunit.diff.ComparisonResult; +import org.xmlunit.diff.DefaultNodeMatcher; +import org.xmlunit.diff.Diff; +import org.xmlunit.diff.DifferenceEvaluator; +import org.xmlunit.diff.ElementSelectors; /** * Compare the contents of 2 zip files. */ -public class ZipFileAssert { +public final class ZipFileAssert { private ZipFileAssert() { } - static final int BUFFER_SIZE = 2048; - - protected static void equals( + private static void equals( TreeMap file1, TreeMap file2) { Set listFile1 = file1.keySet(); @@ -52,32 +59,37 @@ public class ZipFileAssert { for (String fileName : listFile1) { // extract the contents for both - ByteArrayOutputStream contain2 = file2.get(fileName); ByteArrayOutputStream contain1 = file1.get(fileName); + ByteArrayOutputStream contain2 = file2.get(fileName); assertNotNull(fileName + " not found in 2nd zip", contain2); // no need to check for contain1. The key come from it - if ((fileName.endsWith(".xml")) || fileName.endsWith(".rels")) { + if (fileName.matches(".*\\.(xml|rels)$")) { // we have a xml file - // TODO - // YK: the original OpenXML4J version attempted to compare xml using xmlunit (http://xmlunit.sourceforge.net), - // but POI does not depend on this library + final Diff diff = DiffBuilder. + compare(Input.fromByteArray(contain1.toByteArray())). + withTest(Input.fromByteArray(contain2.toByteArray())). + ignoreWhitespace(). + checkForSimilar(). + withDifferenceEvaluator(new IgnoreXMLDeclEvaluator()). + withNodeMatcher(new DefaultNodeMatcher(ElementSelectors.byNameAndAllAttributes, ElementSelectors.byNameAndText)). + build(); + assertFalse(fileName+": "+diff.toString(), diff.hasDifferences()); } else { // not xml, may be an image or other binary format - Assert.assertEquals(fileName + " does not have the same size in both zip:", contain2.size(), contain1.size()); + Assert.assertEquals(fileName + " does not have the same size in both zip:", contain1.size(), contain2.size()); assertArrayEquals("contents differ", contain1.toByteArray(), contain2.toByteArray()); } } } - protected static TreeMap decompress( + private static TreeMap decompress( File filename) throws IOException { // store the zip content in memory // let s assume it is not Go ;-) TreeMap zipContent = new TreeMap<>(); - byte data[] = new byte[BUFFER_SIZE]; /* Open file to decompress */ FileInputStream file_decompress = new FileInputStream(filename); @@ -89,20 +101,12 @@ public class ZipFileAssert { /* Processing entries of the zip file */ ZipEntry entree; - int count; while ((entree = zis.getNextEntry()) != null) { /* Create a array for the current entry */ ByteArrayOutputStream byteArray = new ByteArrayOutputStream(); + IOUtils.copy(zis, byteArray); zipContent.put(entree.getName(), byteArray); - - /* copy in memory */ - while ((count = zis.read(data, 0, BUFFER_SIZE)) != -1) { - byteArray.write(data, 0, count); - } - /* Flush the buffer */ - byteArray.flush(); - byteArray.close(); } zis.close(); @@ -136,4 +140,29 @@ public class ZipFileAssert { throw new AssertionFailedError(e.toString()); } } + + private static class IgnoreXMLDeclEvaluator implements DifferenceEvaluator { + public ComparisonResult evaluate(final Comparison comparison, final ComparisonResult outcome) { + if (outcome != ComparisonResult.EQUAL) { + // only evaluate differences + switch (comparison.getType()) { + case CHILD_NODELIST_SEQUENCE: + case XML_STANDALONE: + case NAMESPACE_PREFIX: + return ComparisonResult.SIMILAR; + case TEXT_VALUE: + switch (comparison.getControlDetails().getTarget().getParentNode().getNodeName()) { + case "dcterms:created": + case "dc:creator": + return ComparisonResult.SIMILAR; + } + break; + default: + break; + } + } + + return outcome; + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/marshallers/TestZipPackagePropertiesMarshaller.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/marshallers/TestZipPackagePropertiesMarshaller.java index 4ff321e370..f9abb2fe8b 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/marshallers/TestZipPackagePropertiesMarshaller.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/marshallers/TestZipPackagePropertiesMarshaller.java @@ -17,11 +17,8 @@ package org.apache.poi.openxml4j.opc.internal.marshallers; -import org.apache.poi.openxml4j.exceptions.OpenXML4JException; -import org.apache.poi.openxml4j.opc.PackagingURIHelper; -import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart; -import org.apache.poi.openxml4j.opc.internal.PartMarshaller; -import org.junit.Test; +import static org.apache.poi.openxml4j.opc.PackagingURIHelper.PACKAGE_RELATIONSHIPS_ROOT_URI; +import static org.junit.Assert.assertTrue; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -29,8 +26,11 @@ import java.io.OutputStream; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; -import static org.apache.poi.openxml4j.opc.PackagingURIHelper.PACKAGE_RELATIONSHIPS_ROOT_URI; -import static org.junit.Assert.assertTrue; +import org.apache.poi.openxml4j.exceptions.OpenXML4JException; +import org.apache.poi.openxml4j.opc.PackagingURIHelper; +import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart; +import org.apache.poi.openxml4j.opc.internal.PartMarshaller; +import org.junit.Test; public class TestZipPackagePropertiesMarshaller { private PartMarshaller marshaller = new ZipPackagePropertiesMarshaller(); @@ -58,7 +58,7 @@ public class TestZipPackagePropertiesMarshaller { marshaller.marshall(new PackagePropertiesPart(null, PackagingURIHelper.createPartName(PACKAGE_RELATIONSHIPS_ROOT_URI)), new ZipOutputStream(new ByteArrayOutputStream()) { @Override - public void putNextEntry(ZipEntry e) throws IOException { + public void putNextEntry(final ZipEntry archiveEntry) throws IOException { throw new IOException("TestException"); } }); diff --git a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java index c6c8f959e7..027dd0e884 100644 --- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java +++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java @@ -17,7 +17,9 @@ package org.apache.poi.poifs.crypt; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -26,6 +28,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.security.GeneralSecurityException; +import java.util.stream.IntStream; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -146,21 +149,27 @@ public class TestDecryptor { // the test file contains a wrong ole entry size, produced by extenxls // the fix limits the available size and tries to read all entries File f = POIDataSamples.getPOIFSInstance().getFile("extenxls_pwd123.xlsx"); - NPOIFSFileSystem fs = new NPOIFSFileSystem(f, true); - EncryptionInfo info = new EncryptionInfo(fs); - Decryptor d = Decryptor.getInstance(info); - d.verifyPassword("pwd123"); - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - ZipInputStream zis = new ZipInputStream(d.getDataStream(fs)); - ZipEntry ze; - while ((ze = zis.getNextEntry()) != null) { - bos.reset(); - IOUtils.copy(zis, bos); - assertEquals(ze.getSize(), bos.size()); + + try (NPOIFSFileSystem fs = new NPOIFSFileSystem(f, true)) { + EncryptionInfo info = new EncryptionInfo(fs); + Decryptor d = Decryptor.getInstance(info); + d.verifyPassword("pwd123"); + + final ByteArrayOutputStream bos = new ByteArrayOutputStream(10000); + try (final ZipInputStream zis = new ZipInputStream(d.getDataStream(fs))) { + IntStream.of(3711, 1155, 445, 9376, 450, 588, 1337, 2593, 304, 7910).forEach(size -> { + try { + final ZipEntry ze = zis.getNextEntry(); + assertNotNull(ze); + IOUtils.copy(zis, bos); + assertEquals(size, bos.size()); + bos.reset(); + } catch (IOException e) { + fail(e.getMessage()); + } + }); + } } - - zis.close(); - fs.close(); } @Test diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java index 96bc0654e6..a25d26b6cc 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java @@ -24,6 +24,7 @@ import static org.apache.poi.POITestCase.assertEndsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -245,7 +246,7 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { SXSSFWorkbook wb = new SXSSFWorkbook(); SXSSFSheet sh = wb.createSheet(); SheetDataWriter wr = sh.getSheetDataWriter(); - assertTrue(wr.getClass() == SheetDataWriter.class); + assertSame(wr.getClass(), SheetDataWriter.class); File tmp = wr.getTempFile(); assertStartsWith(tmp.getName(), "poi-sxssf-sheet"); assertEndsWith(tmp.getName(), ".xml"); @@ -256,7 +257,7 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { wb.setCompressTempFiles(true); sh = wb.createSheet(); wr = sh.getSheetDataWriter(); - assertTrue(wr.getClass() == GZIPSheetDataWriter.class); + assertSame(wr.getClass(), GZIPSheetDataWriter.class); tmp = wr.getTempFile(); assertStartsWith(tmp.getName(), "poi-sxssf-sheet-xml"); assertEndsWith(tmp.getName(), ".gz"); @@ -279,22 +280,10 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { public void gzipSheetdataWriter() throws IOException { SXSSFWorkbook wb = new SXSSFWorkbook(); wb.setCompressTempFiles(true); - int rowNum = 1000; - int sheetNum = 5; - for(int i = 0; i < sheetNum; i++){ - Sheet sh = wb.createSheet("sheet" + i); - for(int j = 0; j < rowNum; j++){ - Row row = sh.createRow(j); - Cell cell1 = row.createCell(0); - cell1.setCellValue(new CellReference(cell1).formatAsString()); - - Cell cell2 = row.createCell(1); - cell2.setCellValue(i); - Cell cell3 = row.createCell(2); - cell3.setCellValue(j); - } - } + final int rowNum = 1000; + final int sheetNum = 5; + populateData(wb, 1000, 5); XSSFWorkbook xwb = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); for(int i = 0; i < sheetNum; i++){ @@ -319,10 +308,24 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { wb.close(); } - protected static void assertWorkbookDispose(SXSSFWorkbook wb) + private static void assertWorkbookDispose(SXSSFWorkbook wb) { - int rowNum = 1000; - int sheetNum = 5; + populateData(wb, 1000, 5); + + for (Sheet sheet : wb) { + SXSSFSheet sxSheet = (SXSSFSheet) sheet; + assertTrue(sxSheet.getSheetDataWriter().getTempFile().exists()); + } + + assertTrue(wb.dispose()); + + for (Sheet sheet : wb) { + SXSSFSheet sxSheet = (SXSSFSheet) sheet; + assertFalse(sxSheet.getSheetDataWriter().getTempFile().exists()); + } + } + + private static void populateData(Workbook wb, final int rowNum, final int sheetNum) { for(int i = 0; i < sheetNum; i++){ Sheet sh = wb.createSheet("sheet" + i); for(int j = 0; j < rowNum; j++){ @@ -337,18 +340,6 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { cell3.setCellValue(j); } } - - for (Sheet sheet : wb) { - SXSSFSheet sxSheet = (SXSSFSheet) sheet; - assertTrue(sxSheet.getSheetDataWriter().getTempFile().exists()); - } - - assertTrue(wb.dispose()); - - for (Sheet sheet : wb) { - SXSSFSheet sxSheet = (SXSSFSheet) sheet; - assertFalse(sxSheet.getSheetDataWriter().getTempFile().exists()); - } } @Test @@ -440,43 +431,11 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { } } - @Ignore("Just a local test for http://stackoverflow.com/questions/33627329/apache-poi-streaming-api-using-xssf-template") @Test - public void testTemplateFile() throws IOException { - XSSFWorkbook workBook = XSSFTestDataSamples.openSampleWorkbook("sample.xlsx"); - SXSSFWorkbook streamingWorkBook = new SXSSFWorkbook(workBook,10); - Sheet sheet = streamingWorkBook.getSheet("Sheet1"); - for(int rowNum = 10;rowNum < 1000000;rowNum++) { - Row row = sheet.createRow(rowNum); - for(int cellNum = 0;cellNum < 700;cellNum++) { - Cell cell = row.createCell(cellNum); - cell.setCellValue("somevalue"); - } - - if(rowNum % 100 == 0) { - System.out.print("."); - if(rowNum % 10000 == 0) { - System.out.println(rowNum); - } - } - } - - FileOutputStream fos = new FileOutputStream("C:\\temp\\streaming.xlsx"); - streamingWorkBook.write(fos); - fos.close(); - - streamingWorkBook.close(); - workBook.close(); - } - - - @Test - public void closeDoesNotModifyWorkbook() throws IOException, InvalidFormatException { + public void closeDoesNotModifyWorkbook() throws IOException { final String filename = "SampleSS.xlsx"; final File file = POIDataSamples.getSpreadSheetInstance().getFile(filename); - SXSSFWorkbook wb = null; - XSSFWorkbook xwb = null; - + // Some tests commented out because close() modifies the file // See bug 58779 @@ -489,19 +448,11 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { //assertCloseDoesNotModifyFile(filename, wb); // InputStream - FileInputStream fis = new FileInputStream(file); - try { - xwb = new XSSFWorkbook(fis); - wb = new SXSSFWorkbook(xwb); + + try (FileInputStream fis = new FileInputStream(file); + XSSFWorkbook xwb = new XSSFWorkbook(fis); + SXSSFWorkbook wb = new SXSSFWorkbook(xwb)) { assertCloseDoesNotModifyFile(filename, wb); - } finally { - if (xwb != null) { - xwb.close(); - } - if (wb != null) { - wb.close(); - } - fis.close(); } // OPCPackage @@ -531,7 +482,6 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { System.arraycopy(prefix, 0, useless, 0, prefix.length); String ul = new String(useless); r.createCell(col, CellType.STRING).setCellValue(ul); - ul = null; } } @@ -573,12 +523,12 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { xssf = new XSSFWorkbook(new ByteArrayInputStream(bos.toByteArray())); s = xssf.getSheet(sheetName); assertEquals(10, s.getLastRowNum()); - assertEquals(true, s.getRow(0).getCell(0).getBooleanCellValue()); + assertTrue(s.getRow(0).getCell(0).getBooleanCellValue()); assertEquals("Test Row 9", s.getRow(9).getCell(2).getStringCellValue()); } @Test - public void test56557() throws IOException, InvalidFormatException { + public void test56557() throws IOException { Workbook wb = XSSFTestDataSamples.openSampleWorkbook("56557.xlsx"); // Using streaming XSSFWorkbook makes the output file invalid -- 2.39.5