From e0f70145037e2f8789e420df76d926ff67585a89 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Fri, 27 Apr 2018 21:38:19 +0000 Subject: [PATCH] #59893 - Forbid calls to InputStream.available git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1830400 13f79535-47bb-0310-9956-ffa450edef68 --- .../poifs/poibrowser/DocumentDescriptor.java | 30 +- .../org/apache/poi/hpsf/VariantSupport.java | 10 +- .../org/apache/poi/hssf/record/ObjRecord.java | 8 +- .../record/crypto/Biff8DecryptingStream.java | 8 +- .../poifs/crypt/ChunkedCipherInputStream.java | 14 +- .../poifs/eventfilesystem/POIFSReader.java | 16 +- .../filesystem/ODocumentInputStream.java | 2 +- .../poi/sl/draw/BitmapImageRenderer.java | 40 +- src/java/org/apache/poi/util/IOUtils.java | 74 ++- .../org/apache/poi/TestPOIXMLProperties.java | 7 +- .../poi/openxml4j/util/TestZipSecureFile.java | 6 +- .../apache/poi/poifs/crypt/TestDecryptor.java | 167 +++--- .../apache/poi/poifs/crypt/TestEncryptor.java | 505 +++++++++--------- .../devtools/forbidden-signatures.txt | 3 + .../poi/hslf/record/TextSpecInfoAtom.java | 2 +- .../poi/hwmf/usermodel/HwmfPicture.java | 99 ++-- .../org/apache/poi/hwmf/TestHwmfParsing.java | 27 +- .../poi/poifs/filesystem/ReaderWriter.java | 10 +- .../filesystem/TestDocumentInputStream.java | 103 ++-- .../poifs/filesystem/TestPOIFSFileSystem.java | 10 +- .../org/apache/poi/util/TestIOUtils.java | 23 + .../org/apache/poi/util/TestLittleEndian.java | 62 ++- 22 files changed, 646 insertions(+), 580 deletions(-) diff --git a/src/examples/src/org/apache/poi/poifs/poibrowser/DocumentDescriptor.java b/src/examples/src/org/apache/poi/poifs/poibrowser/DocumentDescriptor.java index d3c837a44..e93c23b79 100644 --- a/src/examples/src/org/apache/poi/poifs/poibrowser/DocumentDescriptor.java +++ b/src/examples/src/org/apache/poi/poifs/poibrowser/DocumentDescriptor.java @@ -17,15 +17,17 @@ package org.apache.poi.poifs.poibrowser; -import java.io.*; -import org.apache.poi.poifs.filesystem.*; +import java.io.IOException; + +import org.apache.poi.poifs.filesystem.DocumentInputStream; +import org.apache.poi.poifs.filesystem.POIFSDocumentPath; import org.apache.poi.util.IOUtils; /** *

Describes the most important (whatever that is) features of a * {@link POIFSDocumentPath}.

*/ -public class DocumentDescriptor +class DocumentDescriptor { //arbitrarily selected; may need to increase @@ -54,26 +56,20 @@ public class DocumentDescriptor public DocumentDescriptor(final String name, final POIFSDocumentPath path, final DocumentInputStream stream, - final int nrOfBytes) - { + final int nrOfBytes) { this.name = name; this.path = path; this.stream = stream; - try - { - size = stream.available(); - if (stream.markSupported()) - { + try { + if (stream.markSupported()) { stream.mark(nrOfBytes); - final byte[] b = IOUtils.safelyAllocate(nrOfBytes, MAX_RECORD_LENGTH); - final int read = stream.read(b, 0, Math.min(size, b.length)); - bytes = new byte[read]; - System.arraycopy(b, 0, bytes, 0, read); + bytes = IOUtils.toByteArray(stream, nrOfBytes, MAX_RECORD_LENGTH); stream.reset(); + } else { + bytes = new byte[0]; } - } - catch (IOException ex) - { + size = bytes.length + stream.available(); + } catch (IOException ex) { System.out.println(ex); } } diff --git a/src/java/org/apache/poi/hpsf/VariantSupport.java b/src/java/org/apache/poi/hpsf/VariantSupport.java index 7d57cbb5a..e60679027 100644 --- a/src/java/org/apache/poi/hpsf/VariantSupport.java +++ b/src/java/org/apache/poi/hpsf/VariantSupport.java @@ -175,10 +175,12 @@ public class VariantSupport extends Variant { try { typedPropertyValue.readValue(lei); } catch ( UnsupportedOperationException exc ) { - int propLength = Math.min( length, lei.available() ); - final byte[] v = IOUtils.safelyAllocate(propLength, MAX_RECORD_LENGTH); - lei.readFully(v, 0, propLength); - throw new ReadingNotSupportedException( type, v ); + try { + final byte[] v = IOUtils.toByteArray(lei, length, MAX_RECORD_LENGTH); + throw new ReadingNotSupportedException( type, v ); + } catch (IOException e) { + throw new RuntimeException(e); + } } switch ( (int) type ) { diff --git a/src/java/org/apache/poi/hssf/record/ObjRecord.java b/src/java/org/apache/poi/hssf/record/ObjRecord.java index e41e6cbc2..d1b4ab4d1 100644 --- a/src/java/org/apache/poi/hssf/record/ObjRecord.java +++ b/src/java/org/apache/poi/hssf/record/ObjRecord.java @@ -17,14 +17,13 @@ package org.apache.poi.hssf.record; -import java.io.ByteArrayInputStream; import java.util.ArrayList; import java.util.List; import org.apache.poi.util.HexDump; import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.LittleEndianByteArrayInputStream; import org.apache.poi.util.LittleEndianByteArrayOutputStream; -import org.apache.poi.util.LittleEndianInputStream; import org.apache.poi.util.RecordFormatException; /** @@ -85,8 +84,7 @@ public final class ObjRecord extends Record implements Cloneable { */ subrecords = new ArrayList<>(); - ByteArrayInputStream bais = new ByteArrayInputStream(subRecordData); - LittleEndianInputStream subRecStream = new LittleEndianInputStream(bais); + LittleEndianByteArrayInputStream subRecStream = new LittleEndianByteArrayInputStream(subRecordData); CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord)SubRecord.createSubRecord(subRecStream, 0); subrecords.add(cmo); while (true) { @@ -96,7 +94,7 @@ public final class ObjRecord extends Record implements Cloneable { break; } } - int nRemainingBytes = bais.available(); + final int nRemainingBytes = subRecordData.length-subRecStream.getReadIndex(); if (nRemainingBytes > 0) { // At present (Oct-2008), most unit test samples have (subRecordData.length % 2 == 0) _isPaddedToQuadByteMultiple = subRecordData.length % MAX_PAD_ALIGNMENT == 0; diff --git a/src/java/org/apache/poi/hssf/record/crypto/Biff8DecryptingStream.java b/src/java/org/apache/poi/hssf/record/crypto/Biff8DecryptingStream.java index 3c5be4267..99c5d46dc 100644 --- a/src/java/org/apache/poi/hssf/record/crypto/Biff8DecryptingStream.java +++ b/src/java/org/apache/poi/hssf/record/crypto/Biff8DecryptingStream.java @@ -32,6 +32,7 @@ import org.apache.poi.util.LittleEndian; import org.apache.poi.util.LittleEndianConsts; import org.apache.poi.util.LittleEndianInput; import org.apache.poi.util.RecordFormatException; +import org.apache.poi.util.SuppressForbidden; public final class Biff8DecryptingStream implements BiffHeaderInput, LittleEndianInput { @@ -39,7 +40,6 @@ public final class Biff8DecryptingStream implements BiffHeaderInput, LittleEndia //arbitrarily selected; may need to increase private static final int MAX_RECORD_LENGTH = 100_000; - private final EncryptionInfo info; private ChunkedCipherInputStream ccis; private final byte buffer[] = new byte[LittleEndianConsts.LONG_SIZE]; private boolean shouldSkipEncryptionOnCurrentRecord; @@ -54,9 +54,8 @@ public final class Biff8DecryptingStream implements BiffHeaderInput, LittleEndia stream = new PushbackInputStream(in, initialOffset); ((PushbackInputStream)stream).unread(initialBuf); } - - this.info = info; - Decryptor dec = this.info.getDecryptor(); + + Decryptor dec = info.getDecryptor(); dec.setChunkSize(RC4_REKEYING_INTERVAL); ccis = (ChunkedCipherInputStream)dec.getDataStream(stream, Integer.MAX_VALUE, 0); @@ -69,6 +68,7 @@ public final class Biff8DecryptingStream implements BiffHeaderInput, LittleEndia } @Override + @SuppressForbidden("just delegating") public int available() { return ccis.available(); } diff --git a/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java b/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java index 77a967d1d..0b8436267 100644 --- a/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java +++ b/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java @@ -97,7 +97,7 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { private int read(byte[] b, int off, int len, boolean readPlain) throws IOException { int total = 0; - if (available() <= 0) { + if (remainingBytes() <= 0) { return -1; } @@ -112,7 +112,7 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { } } int count = (int)(chunk.length - (pos & chunkMask)); - int avail = available(); + int avail = remainingBytes(); if (avail == 0) { return total; } @@ -133,7 +133,7 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { } @Override - public long skip(final long n) throws IOException { + public long skip(final long n) { long start = pos; long skip = Math.min(remainingBytes(), n); @@ -169,7 +169,7 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { } @Override - public synchronized void reset() throws IOException { + public synchronized void reset() { throw new UnsupportedOperationException(); } @@ -193,7 +193,7 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { } final int todo = (int)Math.min(size, chunk.length); - int readBytes = 0, totalBytes = 0; + int readBytes, totalBytes = 0; do { readBytes = super.read(plain, totalBytes, todo-totalBytes); totalBytes += Math.max(0, readBytes); @@ -211,10 +211,6 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { /** * Helper function for overriding the cipher invocation, i.e. XOR doesn't use a cipher * and uses it's own implementation - * - * @throws BadPaddingException - * @throws IllegalBlockSizeException - * @throws ShortBufferException */ protected int invokeCipher(int totalBytes, boolean doFinal) throws GeneralSecurityException { if (doFinal) { diff --git a/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java b/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java index c4aaecfe0..e61039360 100644 --- a/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java +++ b/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java @@ -184,15 +184,15 @@ public class POIFSReader /** * Activates the notification of empty directories.

* If this flag is activated, the {@link POIFSReaderListener listener} receives - * {@link POIFSReaderEvent POIFSReaderEvents} with nulled {@code name} and {@code stream} + * {@link POIFSReaderEvent POIFSReaderEvents} with nulled {@code name} and {@code stream} * * @param notifyEmptyDirectories */ public void setNotifyEmptyDirectories(boolean notifyEmptyDirectories) { this.notifyEmptyDirectories = notifyEmptyDirectories; } - - + + /** * read in files * @@ -239,7 +239,7 @@ public class POIFSReader } return; } - + while (properties.hasNext()) { Property property = properties.next(); @@ -273,11 +273,9 @@ public class POIFSReader while (listeners.hasNext()) { POIFSReaderListener listener = listeners.next(); - - listener.processPOIFSReaderEvent( - new POIFSReaderEvent( - new DocumentInputStream(document), path, - name)); + try (DocumentInputStream dis = new DocumentInputStream(document)) { + listener.processPOIFSReaderEvent(new POIFSReaderEvent(dis, path, name)); + } } } else diff --git a/src/java/org/apache/poi/poifs/filesystem/ODocumentInputStream.java b/src/java/org/apache/poi/poifs/filesystem/ODocumentInputStream.java index cc280390e..97225805a 100644 --- a/src/java/org/apache/poi/poifs/filesystem/ODocumentInputStream.java +++ b/src/java/org/apache/poi/poifs/filesystem/ODocumentInputStream.java @@ -138,7 +138,7 @@ public final class ODocumentInputStream extends DocumentInputStream { if (atEOD()) { return EOF; } - int limit = Math.min(available(), len); + int limit = Math.min(_document_size - _current_offset, len); readFully(b, off, limit); return limit; } diff --git a/src/java/org/apache/poi/sl/draw/BitmapImageRenderer.java b/src/java/org/apache/poi/sl/draw/BitmapImageRenderer.java index e09054479..a92c8dc59 100644 --- a/src/java/org/apache/poi/sl/draw/BitmapImageRenderer.java +++ b/src/java/org/apache/poi/sl/draw/BitmapImageRenderer.java @@ -28,6 +28,7 @@ import java.awt.image.AffineTransformOp; import java.awt.image.BufferedImage; import java.awt.image.RescaleOp; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.util.Iterator; @@ -39,6 +40,7 @@ import javax.imageio.ImageTypeSpecifier; import javax.imageio.stream.ImageInputStream; import javax.imageio.stream.MemoryCacheImageInputStream; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -69,20 +71,24 @@ public class BitmapImageRenderer implements ImageRenderer { * @return the bufferedImage or null, if there was no image reader for this content type * @throws IOException thrown if there was an error while processing the image */ - private static BufferedImage readImage(InputStream data, String contentType) throws IOException { + private static BufferedImage readImage(final InputStream data, final String contentType) throws IOException { IOException lastException = null; BufferedImage img = null; - if (data.markSupported()) { - data.mark(data.available()); + + final ByteArrayInputStream bis; + if (data instanceof ByteArrayInputStream) { + bis = (ByteArrayInputStream)data; + } else { + ByteArrayOutputStream bos = new ByteArrayOutputStream(0x3FFFF); + IOUtils.copy(data, bos); + bis = new ByteArrayInputStream(bos.toByteArray()); } - + + // currently don't use FileCacheImageInputStream, // because of the risk of filling the file handles (see #59166) - ImageInputStream iis = new MemoryCacheImageInputStream(data); + ImageInputStream iis = new MemoryCacheImageInputStream(bis); try { - iis = new MemoryCacheImageInputStream(data); - iis.mark(); - Iterator iter = ImageIO.getImageReaders(iis); while (img==null && iter.hasNext()) { ImageReader reader = iter.next(); @@ -90,21 +96,11 @@ public class BitmapImageRenderer implements ImageRenderer { // 0:default mode, 1:fallback mode for (int mode=0; img==null && mode<3; mode++) { lastException = null; - try { - iis.reset(); - } catch (IOException e) { - if (data.markSupported()) { - data.reset(); - data.mark(data.available()); - iis.close(); - iis = new MemoryCacheImageInputStream(data); - } else { - // can't restore the input stream, so we need to stop processing here - lastException = e; - break; - } + if (mode > 0) { + bis.reset(); + iis.close(); + iis = new MemoryCacheImageInputStream(bis); } - iis.mark(); try { diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java index 0fbe0df46..c6bf8bd2e 100644 --- a/src/java/org/apache/poi/util/IOUtils.java +++ b/src/java/org/apache/poi/util/IOUtils.java @@ -68,7 +68,7 @@ public final class IOUtils { /** * Peeks at the first N bytes of the stream. Returns those bytes, but * with the stream unaffected. Requires a stream that supports mark/reset, - * or a PushbackInputStream. If the stream has >0 but <N bytes, + * or a PushbackInputStream. If the stream has >0 but <N bytes, * remaining bytes will be zero. * @throws EmptyFileException if the stream is empty */ @@ -81,7 +81,7 @@ public final class IOUtils { if (readBytes == 0) { throw new EmptyFileException(); } - + if (readBytes < limit) { bos.write(new byte[limit-readBytes]); } @@ -116,23 +116,59 @@ public final class IOUtils { * @return A byte array with the read bytes. * @throws IOException If reading data fails or EOF is encountered too early for the given length. */ - public static byte[] toByteArray(InputStream stream, int length) throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(length == Integer.MAX_VALUE ? 4096 : length); + public static byte[] toByteArray(InputStream stream, final int length) throws IOException { + return toByteArray(stream, length, Integer.MAX_VALUE); + } + + + /** + * Reads up to {@code length} bytes from the input stream, and returns the bytes read. + * + * @param stream The byte stream of data to read. + * @param length The maximum length to read, use {@link Integer#MAX_VALUE} to read the stream + * until EOF + * @param maxLength if the input is equal to/longer than {@code maxLength} bytes, + * then throw an {@link IOException} complaining about the length. +* use {@link Integer#MAX_VALUE} to disable the check + * @return A byte array with the read bytes. + * @throws IOException If reading data fails or EOF is encountered too early for the given length. + */ + public static byte[] toByteArray(InputStream stream, final long length, final int maxLength) throws IOException { + if (length < 0L || maxLength < 0L) { + throw new RecordFormatException("Can't allocate an array of length < 0"); + } + if (length > (long)Integer.MAX_VALUE) { + throw new RecordFormatException("Can't allocate an array > "+Integer.MAX_VALUE); + } + if (BYTE_ARRAY_MAX_OVERRIDE > 0) { + if (length > BYTE_ARRAY_MAX_OVERRIDE) { + throwRFE(length, BYTE_ARRAY_MAX_OVERRIDE); + } + } else if (length > maxLength) { + throwRFE(length, maxLength); + } + + final int len = Math.min((int)length, maxLength); + ByteArrayOutputStream baos = new ByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len); byte[] buffer = new byte[4096]; int totalBytes = 0, readBytes; do { - readBytes = stream.read(buffer, 0, Math.min(buffer.length, length-totalBytes)); + readBytes = stream.read(buffer, 0, Math.min(buffer.length, len-totalBytes)); totalBytes += Math.max(readBytes,0); if (readBytes > 0) { baos.write(buffer, 0, readBytes); } - } while (totalBytes < length && readBytes > -1); + } while (totalBytes < len && readBytes > -1); - if (length != Integer.MAX_VALUE && totalBytes < length) { - throw new IOException("unexpected EOF"); + if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { + throw new IOException("MaxLength ("+maxLength+") reached - stream seems to be invalid."); } - + + if (len != Integer.MAX_VALUE && totalBytes < len) { + throw new EOFException("unexpected EOF"); + } + return baos.toByteArray(); } @@ -350,19 +386,19 @@ public final class IOUtils { * * @param inp The {@link InputStream} which provides the data * @param out The {@link OutputStream} to write the data to + * @return the amount of bytes copied + * * @throws IOException If copying the data fails. */ - public static void copy(InputStream inp, OutputStream out) throws IOException { - byte[] buff = new byte[4096]; - int count; - while ((count = inp.read(buff)) != -1) { - if (count < -1) { - throw new RecordFormatException("Can't have read < -1 bytes"); - } + public static long copy(InputStream inp, OutputStream out) throws IOException { + final byte[] buff = new byte[4096]; + long totalCount = 0; + for (int count; (count = inp.read(buff)) != -1; totalCount += count) { if (count > 0) { out.write(buff, 0, count); } } + return totalCount; } /** @@ -370,16 +406,18 @@ public final class IOUtils { * * @param srcStream The {@link InputStream} which provides the data * @param destFile The file where the data should be stored + * @return the amount of bytes copied + * * @throws IOException If the target directory does not exist and cannot be created * or if copying the data fails. */ - public static void copy(InputStream srcStream, File destFile) throws IOException { + public static long copy(InputStream srcStream, File destFile) throws IOException { File destDirectory = destFile.getParentFile(); if (!(destDirectory.exists() || destDirectory.mkdirs())) { throw new RuntimeException("Can't create destination directory: "+destDirectory); } try (OutputStream destStream = new FileOutputStream(destFile)) { - IOUtils.copy(srcStream, destStream); + return IOUtils.copy(srcStream, destStream); } } diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLProperties.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLProperties.java index 2c89085f2..8ecb65131 100644 --- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLProperties.java +++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLProperties.java @@ -30,6 +30,7 @@ import java.util.Date; import org.apache.poi.POIXMLProperties.CoreProperties; import org.apache.poi.openxml4j.util.Nullable; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.LocaleUtil; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFWorkbook; @@ -249,20 +250,18 @@ public final class TestPOIXMLProperties { // Adding / changing ByteArrayInputStream imageData = new ByteArrayInputStream(new byte[1]); - assertEquals(1, imageData.available()); noThumbProps.setThumbnail("Testing.png", imageData); assertNotNull(noThumbProps.getThumbnailPart()); assertEquals("/Testing.png", noThumbProps.getThumbnailFilename()); assertNotNull(noThumbProps.getThumbnailImage()); - assertEquals(1, noThumbProps.getThumbnailImage().available()); + assertEquals(1, IOUtils.toByteArray(noThumbProps.getThumbnailImage()).length); imageData = new ByteArrayInputStream(new byte[2]); - assertEquals(2, imageData.available()); noThumbProps.setThumbnail("Testing2.png", imageData); assertNotNull(noThumbProps.getThumbnailPart()); assertEquals("/Testing.png", noThumbProps.getThumbnailFilename()); assertNotNull(noThumbProps.getThumbnailImage()); - assertEquals(2, noThumbProps.getThumbnailImage().available()); + assertEquals(2, IOUtils.toByteArray(noThumbProps.getThumbnailImage()).length); } private static String zeroPad(long i) { diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/util/TestZipSecureFile.java b/src/ooxml/testcases/org/apache/poi/openxml4j/util/TestZipSecureFile.java index eb19cecb3..89f144c1a 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/util/TestZipSecureFile.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/util/TestZipSecureFile.java @@ -16,6 +16,7 @@ ==================================================================== */ package org.apache.poi.openxml4j.util; +import org.apache.poi.util.IOUtils; import org.apache.poi.xssf.XSSFTestDataSamples; import org.junit.Test; @@ -39,8 +40,9 @@ public class TestZipSecureFile { while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); - InputStream inputStream = secureFile.getInputStream(entry); - assertTrue(inputStream.available() > 0); + try (InputStream inputStream = secureFile.getInputStream(entry)) { + assertTrue(IOUtils.toByteArray(inputStream).length > 0); + } } } } 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 027dd0e88..1fb4946e4 100644 --- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java +++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestDecryptor.java @@ -24,7 +24,6 @@ import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.security.GeneralSecurityException; @@ -39,107 +38,92 @@ import org.apache.poi.poifs.filesystem.DirectoryNode; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.IOUtils; -import org.apache.poi.xssf.XSSFTestDataSamples; import org.junit.Assume; import org.junit.Test; public class TestDecryptor { + private static final POIDataSamples samples = POIDataSamples.getPOIFSInstance(); + @Test public void passwordVerification() throws IOException, GeneralSecurityException { - POIFSFileSystem fs = new POIFSFileSystem(POIDataSamples.getPOIFSInstance().openResourceAsStream("protect.xlsx")); - - EncryptionInfo info = new EncryptionInfo(fs); - - Decryptor d = Decryptor.getInstance(info); - - assertTrue(d.verifyPassword(Decryptor.DEFAULT_PASSWORD)); - - fs.close(); + try (InputStream is = samples.openResourceAsStream("protect.xlsx"); + POIFSFileSystem fs = new POIFSFileSystem(is)) { + EncryptionInfo info = new EncryptionInfo(fs); + Decryptor d = Decryptor.getInstance(info); + assertTrue(d.verifyPassword(Decryptor.DEFAULT_PASSWORD)); + } } @Test public void decrypt() throws IOException, GeneralSecurityException { - POIFSFileSystem fs = new POIFSFileSystem(POIDataSamples.getPOIFSInstance().openResourceAsStream("protect.xlsx")); - - EncryptionInfo info = new EncryptionInfo(fs); - - Decryptor d = Decryptor.getInstance(info); - - d.verifyPassword(Decryptor.DEFAULT_PASSWORD); - - zipOk(fs.getRoot(), d); - - fs.close(); + try (InputStream is = samples.openResourceAsStream("protect.xlsx"); + POIFSFileSystem fs = new POIFSFileSystem(is)) { + EncryptionInfo info = new EncryptionInfo(fs); + Decryptor d = Decryptor.getInstance(info); + d.verifyPassword(Decryptor.DEFAULT_PASSWORD); + zipOk(fs.getRoot(), d); + } } @Test public void agile() throws IOException, GeneralSecurityException { - POIFSFileSystem fs = new POIFSFileSystem(POIDataSamples.getPOIFSInstance().openResourceAsStream("protected_agile.docx")); - - EncryptionInfo info = new EncryptionInfo(fs); - - assertTrue(info.getVersionMajor() == 4 && info.getVersionMinor() == 4); - - Decryptor d = Decryptor.getInstance(info); - - assertTrue(d.verifyPassword(Decryptor.DEFAULT_PASSWORD)); - - zipOk(fs.getRoot(), d); - - fs.close(); + try (InputStream is = samples.openResourceAsStream("protected_agile.docx"); + POIFSFileSystem fs = new POIFSFileSystem(is)) { + EncryptionInfo info = new EncryptionInfo(fs); + assertTrue(info.getVersionMajor() == 4 && info.getVersionMinor() == 4); + Decryptor d = Decryptor.getInstance(info); + assertTrue(d.verifyPassword(Decryptor.DEFAULT_PASSWORD)); + zipOk(fs.getRoot(), d); + } } private void zipOk(DirectoryNode root, Decryptor d) throws IOException, GeneralSecurityException { - ZipInputStream zin = new ZipInputStream(d.getDataStream(root)); + try (ZipInputStream zin = new ZipInputStream(d.getDataStream(root))) { - while (true) { - ZipEntry entry = zin.getNextEntry(); - if (entry==null) { - break; + while (true) { + ZipEntry entry = zin.getNextEntry(); + if (entry == null) { + break; + } + // crc32 is checked within zip-stream + if (entry.isDirectory()) { + continue; + } + assertEquals(entry.getSize() - 1, zin.skip(entry.getSize() - 1)); + byte buf[] = new byte[10]; + int readBytes = zin.read(buf); + // zin.available() doesn't work for entries + assertEquals("size failed for " + entry.getName(), 1, readBytes); } - // crc32 is checked within zip-stream - if (entry.isDirectory()) { - continue; - } - zin.skip(entry.getSize()); - byte buf[] = new byte[10]; - int readBytes = zin.read(buf); - // zin.available() doesn't work for entries - assertEquals("size failed for "+entry.getName(), -1, readBytes); } - - zin.close(); } @Test public void dataLength() throws Exception { - POIFSFileSystem fs = new POIFSFileSystem(POIDataSamples.getPOIFSInstance().openResourceAsStream("protected_agile.docx")); + try (InputStream fsIs = samples.openResourceAsStream("protected_agile.docx"); + POIFSFileSystem fs = new POIFSFileSystem(fsIs)) { + EncryptionInfo info = new EncryptionInfo(fs); + Decryptor d = Decryptor.getInstance(info); + d.verifyPassword(Decryptor.DEFAULT_PASSWORD); - EncryptionInfo info = new EncryptionInfo(fs); + try (InputStream is = d.getDataStream(fs)) { - Decryptor d = Decryptor.getInstance(info); + long len = d.getLength(); + assertEquals(12810, len); - d.verifyPassword(Decryptor.DEFAULT_PASSWORD); + byte[] buf = new byte[(int) len]; + assertEquals(12810, is.read(buf)); - InputStream is = d.getDataStream(fs); + ZipInputStream zin = new ZipInputStream(new ByteArrayInputStream(buf)); - long len = d.getLength(); - assertEquals(12810, len); + while (true) { + ZipEntry entry = zin.getNextEntry(); + if (entry == null) { + break; + } - byte[] buf = new byte[(int)len]; - - is.read(buf); - - ZipInputStream zin = new ZipInputStream(new ByteArrayInputStream(buf)); - - while (true) { - ZipEntry entry = zin.getNextEntry(); - if (entry==null) { - break; - } - - while (zin.available()>0) { - zin.skip(zin.available()); + IOUtils.toByteArray(zin); + } } } } @@ -147,8 +131,8 @@ public class TestDecryptor { @Test public void bug57080() throws Exception { // 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"); + // the fix limits the available size and tries to read all entries + File f = samples.getFile("extenxls_pwd123.xlsx"); try (NPOIFSFileSystem fs = new NPOIFSFileSystem(f, true)) { EncryptionInfo info = new EncryptionInfo(fs); @@ -174,14 +158,12 @@ public class TestDecryptor { @Test public void test58616() throws IOException, GeneralSecurityException { - FileInputStream fis = new FileInputStream(XSSFTestDataSamples.getSampleFile("58616.xlsx")); - POIFSFileSystem pfs = new POIFSFileSystem(fis); - EncryptionInfo info = new EncryptionInfo(pfs); - Decryptor dec = Decryptor.getInstance(info); - //dec.verifyPassword(null); - dec.getDataStream(pfs); - pfs.close(); - fis.close(); + try (InputStream is = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("58616.xlsx"); + POIFSFileSystem pfs = new POIFSFileSystem(is)) { + EncryptionInfo info = new EncryptionInfo(pfs); + Decryptor dec = Decryptor.getInstance(info); + dec.getDataStream(pfs).close(); + } } @Test @@ -189,19 +171,12 @@ public class TestDecryptor { int maxKeyLen = Cipher.getMaxAllowedKeyLength("AES"); Assume.assumeTrue("Please install JCE Unlimited Strength Jurisdiction Policy files for AES 256", maxKeyLen == 2147483647); - InputStream is = POIDataSamples.getPOIFSInstance().openResourceAsStream("60320-protected.xlsx"); - POIFSFileSystem fs = new POIFSFileSystem(is); - is.close(); - - EncryptionInfo info = new EncryptionInfo(fs); - - Decryptor d = Decryptor.getInstance(info); - - boolean b = d.verifyPassword("Test001!!"); - assertTrue(b); - - zipOk(fs.getRoot(), d); - - fs.close(); + try (InputStream is = samples.openResourceAsStream("60320-protected.xlsx"); + POIFSFileSystem fs = new POIFSFileSystem(is)) { + EncryptionInfo info = new EncryptionInfo(fs); + Decryptor d = Decryptor.getInstance(info); + assertTrue(d.verifyPassword("Test001!!")); + zipOk(fs.getRoot(), d); + } } } diff --git a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestEncryptor.java b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestEncryptor.java index 2e95fa59d..a6244b9d6 100644 --- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestEncryptor.java +++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestEncryptor.java @@ -36,18 +36,20 @@ import javax.crypto.Cipher; import org.apache.poi.POIDataSamples; import org.apache.poi.openxml4j.opc.ContentTypes; import org.apache.poi.openxml4j.opc.OPCPackage; +import org.apache.poi.openxml4j.opc.PackageAccess; import org.apache.poi.poifs.crypt.agile.AgileDecryptor; import org.apache.poi.poifs.crypt.agile.AgileEncryptionHeader; import org.apache.poi.poifs.crypt.agile.AgileEncryptionVerifier; import org.apache.poi.poifs.filesystem.DirectoryNode; +import org.apache.poi.poifs.filesystem.DocumentEntry; import org.apache.poi.poifs.filesystem.DocumentNode; import org.apache.poi.poifs.filesystem.Entry; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.poifs.filesystem.POIFSFileSystem; -import org.apache.poi.util.BoundedInputStream; import org.apache.poi.util.IOUtils; import org.apache.poi.util.TempFile; import org.apache.poi.xwpf.usermodel.XWPFDocument; +import org.apache.poi.xwpf.usermodel.XWPFParagraph; import org.junit.Assume; import org.junit.Ignore; import org.junit.Test; @@ -59,35 +61,37 @@ public class TestEncryptor { // ... at least the output can be opened in Excel Viewer String password = "pass"; - InputStream is = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SimpleMultiCell.xlsx"); - ByteArrayOutputStream payloadExpected = new ByteArrayOutputStream(); - IOUtils.copy(is, payloadExpected); - is.close(); - - POIFSFileSystem fs = new POIFSFileSystem(); - EncryptionInfo ei = new EncryptionInfo(EncryptionMode.binaryRC4); - Encryptor enc = ei.getEncryptor(); - enc.confirmPassword(password); - - OutputStream os = enc.getDataStream(fs.getRoot()); - payloadExpected.writeTo(os); - os.close(); - + final byte[] payloadExpected; + try (InputStream is = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SimpleMultiCell.xlsx")) { + payloadExpected = IOUtils.toByteArray(is); + } + ByteArrayOutputStream bos = new ByteArrayOutputStream(); - fs.writeFilesystem(bos); + try (POIFSFileSystem fs = new POIFSFileSystem()) { + EncryptionInfo ei = new EncryptionInfo(EncryptionMode.binaryRC4); + Encryptor enc = ei.getEncryptor(); + enc.confirmPassword(password); + + try (OutputStream os = enc.getDataStream(fs.getRoot())) { + os.write(payloadExpected); + } + + fs.writeFilesystem(bos); + } + + final byte[] payloadActual; + try (POIFSFileSystem fs = new POIFSFileSystem(new ByteArrayInputStream(bos.toByteArray()))) { + EncryptionInfo ei = new EncryptionInfo(fs); + Decryptor dec = ei.getDecryptor(); + boolean b = dec.verifyPassword(password); + assertTrue(b); + + try (InputStream is = dec.getDataStream(fs.getRoot())) { + payloadActual = IOUtils.toByteArray(is); + } + } - fs = new POIFSFileSystem(new ByteArrayInputStream(bos.toByteArray())); - ei = new EncryptionInfo(fs); - Decryptor dec = ei.getDecryptor(); - boolean b = dec.verifyPassword(password); - assertTrue(b); - - ByteArrayOutputStream payloadActual = new ByteArrayOutputStream(); - is = dec.getDataStream(fs.getRoot()); - IOUtils.copy(is,payloadActual); - is.close(); - - assertArrayEquals(payloadExpected.toByteArray(), payloadActual.toByteArray()); + assertArrayEquals(payloadExpected, payloadActual); } @Test @@ -97,83 +101,89 @@ public class TestEncryptor { File file = POIDataSamples.getDocumentInstance().getFile("bug53475-password-is-pass.docx"); String pass = "pass"; - NPOIFSFileSystem nfs = new NPOIFSFileSystem(file); - // Check the encryption details - EncryptionInfo infoExpected = new EncryptionInfo(nfs); - Decryptor decExpected = Decryptor.getInstance(infoExpected); - boolean passed = decExpected.verifyPassword(pass); - assertTrue("Unable to process: document is encrypted", passed); - - // extract the payload - InputStream is = decExpected.getDataStream(nfs); - byte payloadExpected[] = IOUtils.toByteArray(is); - is.close(); + final byte[] payloadExpected, encPackExpected; + final long decPackLenExpected; + final EncryptionInfo infoExpected; + final Decryptor decExpected; - long decPackLenExpected = decExpected.getLength(); - assertEquals(decPackLenExpected, payloadExpected.length); + try (NPOIFSFileSystem nfs = new NPOIFSFileSystem(file, true)) { - is = nfs.getRoot().createDocumentInputStream(Decryptor.DEFAULT_POIFS_ENTRY); - is = new BoundedInputStream(is, is.available()-16); // ignore padding block - byte encPackExpected[] = IOUtils.toByteArray(is); - is.close(); - - // listDir(nfs.getRoot(), "orig", ""); - - nfs.close(); + // Check the encryption details + infoExpected = new EncryptionInfo(nfs); + decExpected = Decryptor.getInstance(infoExpected); + boolean passed = decExpected.verifyPassword(pass); + assertTrue("Unable to process: document is encrypted", passed); + + // extract the payload + try (InputStream is = decExpected.getDataStream(nfs)) { + payloadExpected = IOUtils.toByteArray(is); + } + + decPackLenExpected = decExpected.getLength(); + assertEquals(decPackLenExpected, payloadExpected.length); + + final DirectoryNode root = nfs.getRoot(); + final DocumentEntry entry = (DocumentEntry)root.getEntry(Decryptor.DEFAULT_POIFS_ENTRY); + try (InputStream is = root.createDocumentInputStream(entry)) { + // ignore padding block + encPackExpected = IOUtils.toByteArray(is, entry.getSize()-16); + } + } // check that same verifier/salt lead to same hashes - byte verifierSaltExpected[] = infoExpected.getVerifier().getSalt(); - byte verifierExpected[] = decExpected.getVerifier(); - byte keySalt[] = infoExpected.getHeader().getKeySalt(); - byte keySpec[] = decExpected.getSecretKey().getEncoded(); - byte integritySalt[] = decExpected.getIntegrityHmacKey(); + final byte verifierSaltExpected[] = infoExpected.getVerifier().getSalt(); + final byte verifierExpected[] = decExpected.getVerifier(); + final byte keySalt[] = infoExpected.getHeader().getKeySalt(); + final byte keySpec[] = decExpected.getSecretKey().getEncoded(); + final byte integritySalt[] = decExpected.getIntegrityHmacKey(); // the hmacs of the file always differ, as we use PKCS5-padding to pad the bytes // whereas office just uses random bytes // byte integrityHash[] = d.getIntegrityHmacValue(); - POIFSFileSystem fs = new POIFSFileSystem(); - EncryptionInfo infoActual = new EncryptionInfo( + final EncryptionInfo infoActual = new EncryptionInfo( EncryptionMode.agile , infoExpected.getVerifier().getCipherAlgorithm() , infoExpected.getVerifier().getHashAlgorithm() , infoExpected.getHeader().getKeySize() , infoExpected.getHeader().getBlockSize() , infoExpected.getVerifier().getChainingMode() - ); + ); Encryptor e = Encryptor.getInstance(infoActual); e.confirmPassword(pass, keySpec, keySalt, verifierExpected, verifierSaltExpected, integritySalt); - - OutputStream os = e.getDataStream(fs); - IOUtils.copy(new ByteArrayInputStream(payloadExpected), os); - os.close(); - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - fs.writeFilesystem(bos); - fs.close(); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (POIFSFileSystem fs = new POIFSFileSystem()) { + try (OutputStream os = e.getDataStream(fs)) { + os.write(payloadExpected); + } + fs.writeFilesystem(bos); + } - nfs = new NPOIFSFileSystem(new ByteArrayInputStream(bos.toByteArray())); - infoActual = new EncryptionInfo(nfs.getRoot()); - Decryptor decActual = Decryptor.getInstance(infoActual); - passed = decActual.verifyPassword(pass); - assertTrue("Unable to process: document is encrypted", passed); - - // extract the payload - is = decActual.getDataStream(nfs); - byte payloadActual[] = IOUtils.toByteArray(is); - is.close(); - - long decPackLenActual = decActual.getLength(); - - is = nfs.getRoot().createDocumentInputStream(Decryptor.DEFAULT_POIFS_ENTRY); - is = new BoundedInputStream(is, is.available()-16); // ignore padding block - byte encPackActual[] = IOUtils.toByteArray(is); - is.close(); - - // listDir(nfs.getRoot(), "copy", ""); - - nfs.close(); + final EncryptionInfo infoActual2; + final byte[] payloadActual, encPackActual; + final long decPackLenActual; + try (NPOIFSFileSystem nfs = new NPOIFSFileSystem(new ByteArrayInputStream(bos.toByteArray()))) { + infoActual2 = new EncryptionInfo(nfs.getRoot()); + Decryptor decActual = Decryptor.getInstance(infoActual2); + boolean passed = decActual.verifyPassword(pass); + assertTrue("Unable to process: document is encrypted", passed); + + // extract the payload + try (InputStream is = decActual.getDataStream(nfs)) { + payloadActual = IOUtils.toByteArray(is); + } + + decPackLenActual = decActual.getLength(); + + final DirectoryNode root = nfs.getRoot(); + final DocumentEntry entry = (DocumentEntry)root.getEntry(Decryptor.DEFAULT_POIFS_ENTRY); + try (InputStream is = root.createDocumentInputStream(entry)) { + // ignore padding block + encPackActual = IOUtils.toByteArray(is, entry.getSize()-16); + } + } AgileEncryptionHeader aehExpected = (AgileEncryptionHeader)infoExpected.getHeader(); AgileEncryptionHeader aehActual = (AgileEncryptionHeader)infoActual.getHeader(); @@ -186,32 +196,33 @@ public class TestEncryptor { @Test public void standardEncryption() throws Exception { File file = POIDataSamples.getDocumentInstance().getFile("bug53475-password-is-solrcell.docx"); - String pass = "solrcell"; - - NPOIFSFileSystem nfs = new NPOIFSFileSystem(file); + final String pass = "solrcell"; - // Check the encryption details - EncryptionInfo infoExpected = new EncryptionInfo(nfs); - Decryptor d = Decryptor.getInstance(infoExpected); - boolean passed = d.verifyPassword(pass); - assertTrue("Unable to process: document is encrypted", passed); + final byte[] payloadExpected; + final EncryptionInfo infoExpected; + final Decryptor d; + try (NPOIFSFileSystem nfs = new NPOIFSFileSystem(file, true)) { + + // Check the encryption details + infoExpected = new EncryptionInfo(nfs); + d = Decryptor.getInstance(infoExpected); + boolean passed = d.verifyPassword(pass); + assertTrue("Unable to process: document is encrypted", passed); + + // extract the payload + try (InputStream is = d.getDataStream(nfs)) { + payloadExpected = IOUtils.toByteArray(is); + } + } - // extract the payload - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - InputStream is = d.getDataStream(nfs); - IOUtils.copy(is, bos); - is.close(); - nfs.close(); - byte payloadExpected[] = bos.toByteArray(); - // check that same verifier/salt lead to same hashes - byte verifierSaltExpected[] = infoExpected.getVerifier().getSalt(); - byte verifierExpected[] = d.getVerifier(); - byte keySpec[] = d.getSecretKey().getEncoded(); - byte keySalt[] = infoExpected.getHeader().getKeySalt(); + final byte verifierSaltExpected[] = infoExpected.getVerifier().getSalt(); + final byte verifierExpected[] = d.getVerifier(); + final byte keySpec[] = d.getSecretKey().getEncoded(); + final byte keySalt[] = infoExpected.getHeader().getKeySalt(); - EncryptionInfo infoActual = new EncryptionInfo( + final EncryptionInfo infoActual = new EncryptionInfo( EncryptionMode.standard , infoExpected.getVerifier().getCipherAlgorithm() , infoExpected.getVerifier().getHashAlgorithm() @@ -220,55 +231,50 @@ public class TestEncryptor { , infoExpected.getVerifier().getChainingMode() ); - Encryptor e = Encryptor.getInstance(infoActual); + final Encryptor e = Encryptor.getInstance(infoActual); e.confirmPassword(pass, keySpec, keySalt, verifierExpected, verifierSaltExpected, null); - + assertArrayEquals(infoExpected.getVerifier().getEncryptedVerifier(), infoActual.getVerifier().getEncryptedVerifier()); assertArrayEquals(infoExpected.getVerifier().getEncryptedVerifierHash(), infoActual.getVerifier().getEncryptedVerifierHash()); // now we use a newly generated salt/verifier and check - // if the file content is still the same + // if the file content is still the same - infoActual = new EncryptionInfo( - EncryptionMode.standard - , infoExpected.getVerifier().getCipherAlgorithm() - , infoExpected.getVerifier().getHashAlgorithm() - , infoExpected.getHeader().getKeySize() - , infoExpected.getHeader().getBlockSize() - , infoExpected.getVerifier().getChainingMode() - ); - - e = Encryptor.getInstance(infoActual); - e.confirmPassword(pass); + final byte[] encBytes; + try (POIFSFileSystem fs = new POIFSFileSystem()) { - POIFSFileSystem fs = new POIFSFileSystem(); - OutputStream os = e.getDataStream(fs); - IOUtils.copy(new ByteArrayInputStream(payloadExpected), os); - os.close(); - - bos.reset(); - fs.writeFilesystem(bos); + final EncryptionInfo infoActual2 = new EncryptionInfo( + EncryptionMode.standard + , infoExpected.getVerifier().getCipherAlgorithm() + , infoExpected.getVerifier().getHashAlgorithm() + , infoExpected.getHeader().getKeySize() + , infoExpected.getHeader().getBlockSize() + , infoExpected.getVerifier().getChainingMode() + ); - ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray()); - - // FileOutputStream fos = new FileOutputStream("encrypted.docx"); - // IOUtils.copy(bis, fos); - // fos.close(); - // bis.reset(); - - nfs = new NPOIFSFileSystem(bis); - infoExpected = new EncryptionInfo(nfs); - d = Decryptor.getInstance(infoExpected); - passed = d.verifyPassword(pass); - assertTrue("Unable to process: document is encrypted", passed); + final Encryptor e2 = Encryptor.getInstance(infoActual2); + e2.confirmPassword(pass); + + try (OutputStream os = e2.getDataStream(fs)) { + os.write(payloadExpected); + } + + final ByteArrayOutputStream bos = new ByteArrayOutputStream(50000); + fs.writeFilesystem(bos); + encBytes = bos.toByteArray(); + } + + final byte[] payloadActual; + try (NPOIFSFileSystem nfs = new NPOIFSFileSystem(new ByteArrayInputStream(encBytes))) { + final EncryptionInfo ei = new EncryptionInfo(nfs); + Decryptor d2 = Decryptor.getInstance(ei); + assertTrue("Unable to process: document is encrypted", d2.verifyPassword(pass)); + + try (InputStream is = d2.getDataStream(nfs)) { + payloadActual = IOUtils.toByteArray(is); + } + } - bos.reset(); - is = d.getDataStream(nfs); - IOUtils.copy(is, bos); - is.close(); - nfs.close(); - byte payloadActual[] = bos.toByteArray(); - assertArrayEquals(payloadExpected, payloadActual); } @@ -281,85 +287,87 @@ public class TestEncryptor { @Test public void encryptPackageWithoutCoreProperties() throws Exception { // Open our file without core properties - File inp = POIDataSamples.getOpenXML4JInstance().getFile("OPCCompliance_NoCoreProperties.xlsx"); - OPCPackage pkg = OPCPackage.open(inp.getPath()); - - // It doesn't have any core properties yet - assertEquals(0, pkg.getPartsByContentType(ContentTypes.CORE_PROPERTIES_PART).size()); - assertNotNull(pkg.getPackageProperties()); - assertNotNull(pkg.getPackageProperties().getLanguageProperty()); - assertNull(pkg.getPackageProperties().getLanguageProperty().getValue()); - - // Encrypt it - EncryptionInfo info = new EncryptionInfo(EncryptionMode.agile); - NPOIFSFileSystem fs = new NPOIFSFileSystem(); - - Encryptor enc = info.getEncryptor(); - enc.confirmPassword("password"); - OutputStream os = enc.getDataStream(fs); - pkg.save(os); - os.close(); - pkg.revert(); - - // Save the resulting OLE2 document, and re-open it - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - fs.writeFilesystem(baos); - fs.close(); - - ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - NPOIFSFileSystem inpFS = new NPOIFSFileSystem(bais); - - // Check we can decrypt it - info = new EncryptionInfo(inpFS); - Decryptor d = Decryptor.getInstance(info); - assertEquals(true, d.verifyPassword("password")); - - OPCPackage inpPkg = OPCPackage.open(d.getDataStream(inpFS)); - - // Check it now has empty core properties - assertEquals(1, inpPkg.getPartsByContentType(ContentTypes.CORE_PROPERTIES_PART).size()); - assertNotNull(inpPkg.getPackageProperties()); - assertNotNull(inpPkg.getPackageProperties().getLanguageProperty()); - assertNull(inpPkg.getPackageProperties().getLanguageProperty().getValue()); + final byte[] encBytes; + try (InputStream is = POIDataSamples.getOpenXML4JInstance().openResourceAsStream("OPCCompliance_NoCoreProperties.xlsx"); + OPCPackage pkg = OPCPackage.open(is)) { - inpPkg.close(); - inpFS.close(); + // It doesn't have any core properties yet + assertEquals(0, pkg.getPartsByContentType(ContentTypes.CORE_PROPERTIES_PART).size()); + assertNotNull(pkg.getPackageProperties()); + assertNotNull(pkg.getPackageProperties().getLanguageProperty()); + assertNull(pkg.getPackageProperties().getLanguageProperty().getValue()); + + // Encrypt it + EncryptionInfo info = new EncryptionInfo(EncryptionMode.agile); + Encryptor enc = info.getEncryptor(); + enc.confirmPassword("password"); + + try (NPOIFSFileSystem fs = new NPOIFSFileSystem()) { + + try (OutputStream os = enc.getDataStream(fs)) { + pkg.save(os); + } + + // Save the resulting OLE2 document, and re-open it + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + fs.writeFilesystem(baos); + encBytes = baos.toByteArray(); + } + } + + + try (NPOIFSFileSystem inpFS = new NPOIFSFileSystem(new ByteArrayInputStream(encBytes))) { + // Check we can decrypt it + EncryptionInfo info = new EncryptionInfo(inpFS); + Decryptor d = Decryptor.getInstance(info); + assertEquals(true, d.verifyPassword("password")); + + try (OPCPackage inpPkg = OPCPackage.open(d.getDataStream(inpFS))) { + // Check it now has empty core properties + assertEquals(1, inpPkg.getPartsByContentType(ContentTypes.CORE_PROPERTIES_PART).size()); + assertNotNull(inpPkg.getPackageProperties()); + assertNotNull(inpPkg.getPackageProperties().getLanguageProperty()); + assertNull(inpPkg.getPackageProperties().getLanguageProperty().getValue()); + + } + } } @Test @Ignore public void inPlaceRewrite() throws Exception { File f = TempFile.createTempFile("protected_agile", ".docx"); - // File f = new File("protected_agile.docx"); - FileOutputStream fos = new FileOutputStream(f); - InputStream fis = POIDataSamples.getPOIFSInstance().openResourceAsStream("protected_agile.docx"); - IOUtils.copy(fis, fos); - fis.close(); - fos.close(); + + try (FileOutputStream fos = new FileOutputStream(f); + InputStream fis = POIDataSamples.getPOIFSInstance().openResourceAsStream("protected_agile.docx")) { + IOUtils.copy(fis, fos); + } - NPOIFSFileSystem fs = new NPOIFSFileSystem(f, false); + try (NPOIFSFileSystem fs = new NPOIFSFileSystem(f, false)) { - // decrypt the protected file - in this case it was encrypted with the default password - EncryptionInfo encInfo = new EncryptionInfo(fs); - Decryptor d = encInfo.getDecryptor(); - boolean b = d.verifyPassword(Decryptor.DEFAULT_PASSWORD); - assertTrue(b); + // decrypt the protected file - in this case it was encrypted with the default password + EncryptionInfo encInfo = new EncryptionInfo(fs); + Decryptor d = encInfo.getDecryptor(); + boolean b = d.verifyPassword(Decryptor.DEFAULT_PASSWORD); + assertTrue(b); - // do some strange things with it ;) - InputStream docIS = d.getDataStream(fs); - XWPFDocument docx = new XWPFDocument(docIS); - docx.getParagraphArray(0).insertNewRun(0).setText("POI was here! All your base are belong to us!"); - docx.getParagraphArray(0).insertNewRun(1).addBreak(); + try (InputStream docIS = d.getDataStream(fs); + XWPFDocument docx = new XWPFDocument(docIS)) { - // and encrypt it again - Encryptor e = encInfo.getEncryptor(); - e.confirmPassword("AYBABTU"); - docx.write(e.getDataStream(fs)); - docx.close(); - docIS.close(); - - docx.close(); - fs.close(); + // do some strange things with it ;) + XWPFParagraph p = docx.getParagraphArray(0); + p.insertNewRun(0).setText("POI was here! All your base are belong to us!"); + p.insertNewRun(1).addBreak(); + + // and encrypt it again + Encryptor e = encInfo.getEncryptor(); + e.confirmPassword("AYBABTU"); + + try (OutputStream os = e.getDataStream(fs)) { + docx.write(os); + } + } + } } @@ -437,22 +445,24 @@ public class TestEncryptor { // existing = getCipher(skey, header.getCipherAlgorithm(), header.getChainingMode(), header.getKeySalt(), encryptionMode, padding); // } - InputStream is = POIDataSamples.getPOIFSInstance().openResourceAsStream("60320-protected.xlsx"); - POIFSFileSystem fsOrig = new POIFSFileSystem(is); - is.close(); - EncryptionInfo infoOrig = new EncryptionInfo(fsOrig); - Decryptor decOrig = infoOrig.getDecryptor(); - boolean b = decOrig.verifyPassword("Test001!!"); - assertTrue(b); - InputStream decIn = decOrig.getDataStream(fsOrig); - byte[] zipInput = IOUtils.toByteArray(decIn); - decIn.close(); + final EncryptionInfo infoOrig; + final byte[] zipInput, epOrigBytes; + try (InputStream is = POIDataSamples.getPOIFSInstance().openResourceAsStream("60320-protected.xlsx"); + POIFSFileSystem fsOrig = new POIFSFileSystem(is)) { + infoOrig = new EncryptionInfo(fsOrig); + Decryptor decOrig = infoOrig.getDecryptor(); + boolean b = decOrig.verifyPassword("Test001!!"); + assertTrue(b); + try (InputStream decIn = decOrig.getDataStream(fsOrig)) { + zipInput = IOUtils.toByteArray(decIn); + } + + try (InputStream epOrig = fsOrig.getRoot().createDocumentInputStream("EncryptedPackage")) { + // ignore the 16 padding bytes + epOrigBytes = IOUtils.toByteArray(epOrig, 9400); + } + } - InputStream epOrig = fsOrig.getRoot().createDocumentInputStream("EncryptedPackage"); - // ignore the 16 padding bytes - byte[] epOrigBytes = IOUtils.toByteArray(epOrig, 9400); - epOrig.close(); - EncryptionInfo eiNew = new EncryptionInfo(EncryptionMode.agile); AgileEncryptionHeader aehHeader = (AgileEncryptionHeader)eiNew.getHeader(); aehHeader.setCipherAlgorithm(CipherAlgorithm.aes128); @@ -472,26 +482,29 @@ public class TestEncryptor { infoOrig.getVerifier().getSalt(), infoOrig.getDecryptor().getIntegrityHmacKey() ); - NPOIFSFileSystem fsNew = new NPOIFSFileSystem(); - OutputStream os = enc.getDataStream(fsNew); - os.write(zipInput); - os.close(); - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - fsNew.writeFilesystem(bos); - fsNew.close(); - - NPOIFSFileSystem fsReload = new NPOIFSFileSystem(new ByteArrayInputStream(bos.toByteArray())); - InputStream epReload = fsReload.getRoot().createDocumentInputStream("EncryptedPackage"); - byte[] epNewBytes = IOUtils.toByteArray(epReload, 9400); - epReload.close(); - + final byte[] epNewBytes; + final EncryptionInfo infoReload; + try (NPOIFSFileSystem fsNew = new NPOIFSFileSystem()) { + try (OutputStream os = enc.getDataStream(fsNew)) { + os.write(zipInput); + } + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + fsNew.writeFilesystem(bos); + + try (NPOIFSFileSystem fsReload = new NPOIFSFileSystem(new ByteArrayInputStream(bos.toByteArray()))) { + infoReload = new EncryptionInfo(fsReload); + try (InputStream epReload = fsReload.getRoot().createDocumentInputStream("EncryptedPackage")) { + epNewBytes = IOUtils.toByteArray(epReload, 9400); + } + } + } + assertArrayEquals(epOrigBytes, epNewBytes); - EncryptionInfo infoReload = new EncryptionInfo(fsOrig); Decryptor decReload = infoReload.getDecryptor(); - b = decReload.verifyPassword("Test001!!"); - assertTrue(b); + assertTrue(decReload.verifyPassword("Test001!!")); AgileEncryptionHeader aehOrig = (AgileEncryptionHeader)infoOrig.getHeader(); AgileEncryptionHeader aehReload = (AgileEncryptionHeader)infoReload.getHeader(); @@ -529,7 +542,5 @@ public class TestEncryptor { // assertArrayEquals(adOrig.getIntegrityHmacValue(), adReload.getIntegrityHmacValue()); assertArrayEquals(adOrig.getSecretKey().getEncoded(), adReload.getSecretKey().getEncoded()); assertArrayEquals(adOrig.getVerifier(), adReload.getVerifier()); - - fsReload.close(); } } diff --git a/src/resources/devtools/forbidden-signatures.txt b/src/resources/devtools/forbidden-signatures.txt index c96b08b6c..2ea708740 100644 --- a/src/resources/devtools/forbidden-signatures.txt +++ b/src/resources/devtools/forbidden-signatures.txt @@ -119,6 +119,9 @@ java.lang.Object#notifyAll() @defaultMessage Don't interrupt threads use FutureUtils#cancel(Future) instead java.util.concurrent.Future#cancel(boolean) +@defaultMessage Don't use ...InputStream.available() as it gives wrong result for certain streams - use IOUtils.toByteArray to read the stream fully and then count the available bytes +java.io.InputStream#available() + @defaultMessage Unnecessary, inefficient, and confusing conversion of String.toString java.lang.String#toString() diff --git a/src/scratchpad/src/org/apache/poi/hslf/record/TextSpecInfoAtom.java b/src/scratchpad/src/org/apache/poi/hslf/record/TextSpecInfoAtom.java index db3eb5ab7..c732f39c7 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/record/TextSpecInfoAtom.java +++ b/src/scratchpad/src/org/apache/poi/hslf/record/TextSpecInfoAtom.java @@ -171,7 +171,7 @@ public final class TextSpecInfoAtom extends RecordAtom { public TextSpecInfoRun[] getTextSpecInfoRuns(){ LittleEndianByteArrayInputStream bis = new LittleEndianByteArrayInputStream(_data); // NOSONAR List lst = new ArrayList<>(); - while (bis.available() > 0) { + while (bis.getReadIndex() < _data.length) { lst.add(new TextSpecInfoRun(bis)); } return lst.toArray(new TextSpecInfoRun[lst.size()]); diff --git a/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java b/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java index 07e7d16cd..fa59f4bc3 100644 --- a/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java +++ b/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java @@ -50,52 +50,59 @@ public class HwmfPicture { final HwmfHeader header; public HwmfPicture(InputStream inputStream) throws IOException { - BufferedInputStream bis = new BufferedInputStream(inputStream, 10000); - LittleEndianInputStream leis = new LittleEndianInputStream(bis); - placeableHeader = HwmfPlaceableHeader.readHeader(leis); - header = new HwmfHeader(leis); - - for (;;) { - if (leis.available() < 6) { - logger.log(POILogger.ERROR, "unexpected eof - wmf file was truncated"); - break; - } - // recordSize in DWORDs - long recordSizeLong = leis.readUInt()*2; - if (recordSizeLong > Integer.MAX_VALUE) { - throw new RecordFormatException("record size can't be > "+Integer.MAX_VALUE); - } else if (recordSizeLong < 0L) { - throw new RecordFormatException("record size can't be < 0"); - } - int recordSize = (int)recordSizeLong; - int recordFunction = leis.readShort(); - // 4 bytes (recordSize) + 2 bytes (recordFunction) - int consumedSize = 6; - HwmfRecordType wrt = HwmfRecordType.getById(recordFunction); - if (wrt == null) { - throw new IOException("unexpected record type: "+recordFunction); - } - if (wrt == HwmfRecordType.eof) break; - if (wrt.clazz == null) { - throw new IOException("unsupported record type: "+recordFunction); - } - - HwmfRecord wr; - try { - wr = wrt.clazz.newInstance(); - records.add(wr); - } catch (Exception e) { - throw (IOException)new IOException("can't create wmf record").initCause(e); - } - - consumedSize += wr.init(leis, recordSize, recordFunction); - int remainingSize = recordSize - consumedSize; - if (remainingSize < 0) { - throw new RecordFormatException("read too many bytes. record size: "+recordSize + "; comsumed size: "+consumedSize); - } else if(remainingSize > 0) { - long skipped = IOUtils.skipFully(leis, remainingSize); - if (skipped != (long)remainingSize) { - throw new RecordFormatException("Tried to skip "+remainingSize + " but skipped: "+skipped); + + try (BufferedInputStream bis = new BufferedInputStream(inputStream, 10000); + LittleEndianInputStream leis = new LittleEndianInputStream(bis)) { + placeableHeader = HwmfPlaceableHeader.readHeader(leis); + header = new HwmfHeader(leis); + + for (;;) { + long recordSize; + int recordFunction; + try { + // recordSize in DWORDs + long recordSizeLong = leis.readUInt()*2; + if (recordSizeLong > Integer.MAX_VALUE) { + throw new RecordFormatException("record size can't be > "+Integer.MAX_VALUE); + } else if (recordSizeLong < 0L) { + throw new RecordFormatException("record size can't be < 0"); + } + recordSize = (int)recordSizeLong; + recordFunction = leis.readShort(); + } catch (Exception e) { + logger.log(POILogger.ERROR, "unexpected eof - wmf file was truncated"); + break; + } + // 4 bytes (recordSize) + 2 bytes (recordFunction) + int consumedSize = 6; + HwmfRecordType wrt = HwmfRecordType.getById(recordFunction); + if (wrt == null) { + throw new IOException("unexpected record type: "+recordFunction); + } + if (wrt == HwmfRecordType.eof) { + break; + } + if (wrt.clazz == null) { + throw new IOException("unsupported record type: "+recordFunction); + } + + HwmfRecord wr; + try { + wr = wrt.clazz.newInstance(); + records.add(wr); + } catch (Exception e) { + throw (IOException)new IOException("can't create wmf record").initCause(e); + } + + consumedSize += wr.init(leis, recordSize, recordFunction); + int remainingSize = (int)(recordSize - consumedSize); + if (remainingSize < 0) { + throw new RecordFormatException("read too many bytes. record size: "+recordSize + "; comsumed size: "+consumedSize); + } else if(remainingSize > 0) { + long skipped = IOUtils.skipFully(leis, remainingSize); + if (skipped != (long)remainingSize) { + throw new RecordFormatException("Tried to skip "+remainingSize + " but skipped: "+skipped); + } } } } diff --git a/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java b/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java index 97a042c76..1667b67b4 100644 --- a/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java +++ b/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java @@ -32,6 +32,7 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.FilterInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.nio.charset.Charset; import java.util.List; @@ -57,32 +58,30 @@ import org.junit.Ignore; import org.junit.Test; public class TestHwmfParsing { + + private static final POIDataSamples samples = POIDataSamples.getSlideShowInstance(); + + @Test public void parse() throws IOException { - File f = POIDataSamples.getSlideShowInstance().getFile("santa.wmf"); - FileInputStream fis = new FileInputStream(f); - HwmfPicture wmf = new HwmfPicture(fis); - fis.close(); - List records = wmf.getRecords(); - assertEquals(581, records.size()); + try (InputStream fis = samples.openResourceAsStream("santa.wmf")) { + HwmfPicture wmf = new HwmfPicture(fis); + List records = wmf.getRecords(); + assertEquals(581, records.size()); + } } @Test(expected = RecordFormatException.class) public void testInfiniteLoop() throws Exception { - File f = POIDataSamples.getSlideShowInstance().getFile("61338.wmf"); - FileInputStream fis = null; - try { - fis = new FileInputStream(f); - HwmfPicture wmf = new HwmfPicture(fis); - } finally { - fis.close(); + try (InputStream is = samples.openResourceAsStream("61338.wmf")) { + new HwmfPicture(is); } } @Test @Ignore("This is work-in-progress and not a real unit test ...") public void paint() throws IOException { - File f = POIDataSamples.getSlideShowInstance().getFile("santa.wmf"); + File f = samples.getFile("santa.wmf"); // File f = new File("bla.wmf"); FileInputStream fis = new FileInputStream(f); HwmfPicture wmf = new HwmfPicture(fis); diff --git a/src/testcases/org/apache/poi/poifs/filesystem/ReaderWriter.java b/src/testcases/org/apache/poi/poifs/filesystem/ReaderWriter.java index 54257598b..240e945a5 100644 --- a/src/testcases/org/apache/poi/poifs/filesystem/ReaderWriter.java +++ b/src/testcases/org/apache/poi/poifs/filesystem/ReaderWriter.java @@ -29,6 +29,7 @@ import java.util.Map; import org.apache.poi.poifs.eventfilesystem.POIFSReader; import org.apache.poi.poifs.eventfilesystem.POIFSReaderEvent; import org.apache.poi.poifs.eventfilesystem.POIFSReaderListener; +import org.apache.poi.util.IOUtils; /** * Test (Proof of concept) program that employs the @@ -110,16 +111,15 @@ public class ReaderWriter @Override public void processPOIFSReaderEvent(final POIFSReaderEvent event) { + @SuppressWarnings("resource") DocumentInputStream istream = event.getStream(); POIFSDocumentPath path = event.getPath(); String name = event.getName(); - try - { - int size = istream.available(); - byte[] data = new byte[ istream.available() ]; + try { + byte[] data = IOUtils.toByteArray(istream); + int size = data.length; - istream.read(data); DocumentDescriptor descriptor = new DocumentDescriptor(path, name); diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestDocumentInputStream.java b/src/testcases/org/apache/poi/poifs/filesystem/TestDocumentInputStream.java index bbe1e39f6..a4b64c56e 100644 --- a/src/testcases/org/apache/poi/poifs/filesystem/TestDocumentInputStream.java +++ b/src/testcases/org/apache/poi/poifs/filesystem/TestDocumentInputStream.java @@ -17,22 +17,28 @@ package org.apache.poi.poifs.filesystem; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.util.Arrays; -import junit.framework.TestCase; - import org.apache.poi.POIDataSamples; import org.apache.poi.poifs.property.DirectoryProperty; import org.apache.poi.poifs.storage.RawDataBlock; +import org.apache.poi.util.SuppressForbidden; +import org.junit.Before; +import org.junit.Test; /** * Class to test DocumentInputStream functionality */ -public final class TestDocumentInputStream extends TestCase { +public final class TestDocumentInputStream { private DocumentNode _workbook_n; private DocumentNode _workbook_o; private byte[] _workbook_data; @@ -42,8 +48,8 @@ public final class TestDocumentInputStream extends TestCase { // any block size private static final int _buffer_size = 6; - @Override - protected void setUp() throws Exception { + @Before + public void setUp() throws Exception { int blocks = (_workbook_size + 511) / 512; _workbook_data = new byte[ 512 * blocks ]; @@ -92,6 +98,7 @@ public final class TestDocumentInputStream extends TestCase { /** * test constructor */ + @Test public void testConstructor() throws IOException { DocumentInputStream ostream = new ODocumentInputStream(_workbook_o); DocumentInputStream nstream = new NDocumentInputStream(_workbook_n); @@ -99,8 +106,8 @@ public final class TestDocumentInputStream extends TestCase { assertEquals(_workbook_size, _workbook_o.getSize()); assertEquals(_workbook_size, _workbook_n.getSize()); - assertEquals(_workbook_size, ostream.available()); - assertEquals(_workbook_size, nstream.available()); + assertEquals(_workbook_size, available(ostream)); + assertEquals(_workbook_size, available(nstream)); ostream.close(); nstream.close(); @@ -109,23 +116,24 @@ public final class TestDocumentInputStream extends TestCase { /** * test available() behavior */ + @Test public void testAvailable() throws IOException { DocumentInputStream ostream = new DocumentInputStream(_workbook_o); DocumentInputStream nstream = new NDocumentInputStream(_workbook_n); - assertEquals(_workbook_size, ostream.available()); - assertEquals(_workbook_size, nstream.available()); + assertEquals(_workbook_size, available(ostream)); + assertEquals(_workbook_size, available(nstream)); ostream.close(); nstream.close(); try { - ostream.available(); + available(ostream); fail("Should have caught IOException"); } catch (IllegalStateException ignored) { // as expected } try { - nstream.available(); + available(nstream); fail("Should have caught IOException"); } catch (IllegalStateException ignored) { // as expected @@ -135,6 +143,7 @@ public final class TestDocumentInputStream extends TestCase { /** * test mark/reset/markSupported. */ + @Test public void testMarkFunctions() throws IOException { byte[] buffer = new byte[ _workbook_size / 5 ]; byte[] small_buffer = new byte[212]; @@ -152,12 +161,12 @@ public final class TestDocumentInputStream extends TestCase { _workbook_data[ j ], buffer[ j ] ); } - assertEquals(_workbook_size - buffer.length, stream.available()); + assertEquals(_workbook_size - buffer.length, available(stream)); // Reset, and check the available goes back to being the // whole of the stream stream.reset(); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); // Read part of a block @@ -168,7 +177,7 @@ public final class TestDocumentInputStream extends TestCase { _workbook_data[ j ], small_buffer[ j ] ); } - assertEquals(_workbook_size - small_buffer.length, stream.available()); + assertEquals(_workbook_size - small_buffer.length, available(stream)); stream.mark(0); // Read the next part @@ -179,11 +188,11 @@ public final class TestDocumentInputStream extends TestCase { _workbook_data[ j+small_buffer.length ], small_buffer[ j ] ); } - assertEquals(_workbook_size - 2*small_buffer.length, stream.available()); + assertEquals(_workbook_size - 2*small_buffer.length, available(stream)); // Reset, check it goes back to where it was stream.reset(); - assertEquals(_workbook_size - small_buffer.length, stream.available()); + assertEquals(_workbook_size - small_buffer.length, available(stream)); // Read stream.read(small_buffer); @@ -193,7 +202,7 @@ public final class TestDocumentInputStream extends TestCase { _workbook_data[ j+small_buffer.length ], small_buffer[ j ] ); } - assertEquals(_workbook_size - 2*small_buffer.length, stream.available()); + assertEquals(_workbook_size - 2*small_buffer.length, available(stream)); // Now read at various points @@ -236,11 +245,11 @@ public final class TestDocumentInputStream extends TestCase { _workbook_data[ j ], buffer[ j ] ); } - assertEquals(_workbook_size - buffer.length, stream.available()); + assertEquals(_workbook_size - buffer.length, available(stream)); // Read all of it again, check it began at the start again stream.reset(); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); stream.read(buffer); for (int j = 0; j < buffer.length; j++) { @@ -254,7 +263,7 @@ public final class TestDocumentInputStream extends TestCase { stream.mark(12); stream.read(buffer); assertEquals(_workbook_size - (2 * buffer.length), - stream.available()); + available(stream)); for (int j = buffer.length; j < (2 * buffer.length); j++) { assertEquals("checking byte " + j, _workbook_data[ j ], @@ -263,12 +272,12 @@ public final class TestDocumentInputStream extends TestCase { // Reset, should go back to only one buffer full read stream.reset(); - assertEquals(_workbook_size - buffer.length, stream.available()); + assertEquals(_workbook_size - buffer.length, available(stream)); // Read the buffer again stream.read(buffer); assertEquals(_workbook_size - (2 * buffer.length), - stream.available()); + available(stream)); for (int j = buffer.length; j < (2 * buffer.length); j++) { assertEquals("checking byte " + j, _workbook_data[ j ], @@ -281,6 +290,7 @@ public final class TestDocumentInputStream extends TestCase { /** * test simple read method */ + @Test public void testReadSingleByte() throws IOException { DocumentInputStream[] streams = new DocumentInputStream[] { new DocumentInputStream(_workbook_o), @@ -297,7 +307,7 @@ public final class TestDocumentInputStream extends TestCase { ( byte ) b); remaining--; assertEquals("checking remaining after reading byte " + j, - remaining, stream.available()); + remaining, available(stream)); } // Ensure we fell off the end @@ -317,6 +327,7 @@ public final class TestDocumentInputStream extends TestCase { /** * Test buffered read */ + @Test public void testBufferRead() throws IOException { DocumentInputStream[] streams = new DocumentInputStream[] { new DocumentInputStream(_workbook_o), @@ -333,22 +344,22 @@ public final class TestDocumentInputStream extends TestCase { // test reading zero length buffer assertEquals(0, stream.read(new byte[ 0 ])); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); byte[] buffer = new byte[ _buffer_size ]; int offset = 0; - while (stream.available() >= buffer.length) + while (available(stream) >= buffer.length) { assertEquals(_buffer_size, stream.read(buffer)); - for (byte data : buffer) { + for (byte element : buffer) { assertEquals("in main loop, byte " + offset, - _workbook_data[ offset ], data); + _workbook_data[ offset ], element); offset++; } assertEquals("offset " + offset, _workbook_size - offset, - stream.available()); + available(stream)); } - assertEquals(_workbook_size % _buffer_size, stream.available()); + assertEquals(_workbook_size % _buffer_size, available(stream)); Arrays.fill(buffer, ( byte ) 0); int count = stream.read(buffer); @@ -378,6 +389,7 @@ public final class TestDocumentInputStream extends TestCase { /** * Test complex buffered read */ + @Test public void testComplexBufferRead() throws IOException { DocumentInputStream[] streams = new DocumentInputStream[] { new DocumentInputStream(_workbook_o), @@ -413,11 +425,11 @@ public final class TestDocumentInputStream extends TestCase { // test reading zero assertEquals(0, stream.read(new byte[ 5 ], 0, 0)); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); byte[] buffer = new byte[ _workbook_size ]; int offset = 0; - while (stream.available() >= _buffer_size) + while (available(stream) >= _buffer_size) { Arrays.fill(buffer, ( byte ) 0); assertEquals(_buffer_size, @@ -437,9 +449,9 @@ public final class TestDocumentInputStream extends TestCase { } offset += _buffer_size; assertEquals("offset " + offset, _workbook_size - offset, - stream.available()); + available(stream)); } - assertEquals(_workbook_size % _buffer_size, stream.available()); + assertEquals(_workbook_size % _buffer_size, available(stream)); Arrays.fill(buffer, ( byte ) 0); int count = stream.read(buffer, offset, _workbook_size % _buffer_size); @@ -474,38 +486,40 @@ public final class TestDocumentInputStream extends TestCase { /** * Tests that we can skip within the stream */ + @Test public void testSkip() throws IOException { DocumentInputStream[] streams = new DocumentInputStream[] { new DocumentInputStream(_workbook_o), new NDocumentInputStream(_workbook_n) }; for(DocumentInputStream stream : streams) { - assertEquals(_workbook_size, stream.available()); - int count = stream.available(); + assertEquals(_workbook_size, available(stream)); + int count = available(stream); - while (stream.available() >= _buffer_size) { + while (available(stream) >= _buffer_size) { assertEquals(_buffer_size, stream.skip(_buffer_size)); count -= _buffer_size; - assertEquals(count, stream.available()); + assertEquals(count, available(stream)); } assertEquals(_workbook_size % _buffer_size, stream.skip(_buffer_size)); - assertEquals(0, stream.available()); + assertEquals(0, available(stream)); stream.reset(); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); assertEquals(_workbook_size, stream.skip(_workbook_size * 2)); - assertEquals(0, stream.available()); + assertEquals(0, available(stream)); stream.reset(); - assertEquals(_workbook_size, stream.available()); + assertEquals(_workbook_size, available(stream)); assertEquals(_workbook_size, stream.skip(2 + ( long ) Integer.MAX_VALUE)); - assertEquals(0, stream.available()); + assertEquals(0, available(stream)); } } /** * Test that we can read files at multiple levels down the tree */ + @Test public void testReadMultipleTreeLevels() throws Exception { final POIDataSamples _samples = POIDataSamples.getPublisherInstance(); File sample = _samples.getFile("Sample.pub"); @@ -551,4 +565,9 @@ public final class TestDocumentInputStream extends TestCase { npoifs.close(); } } + + @SuppressForbidden("just for testing") + private static int available(InputStream is) throws IOException { + return is.available(); + } } diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java b/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java index 9aac65ab4..d1ae24851 100644 --- a/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java +++ b/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java @@ -33,6 +33,7 @@ import org.apache.poi.poifs.storage.BATBlock; import org.apache.poi.poifs.storage.BlockAllocationTableReader; import org.apache.poi.poifs.storage.HeaderBlock; import org.apache.poi.poifs.storage.RawDataBlockList; +import org.apache.poi.util.IOUtils; /** * Tests for the older OPOIFS-based POIFSFileSystem @@ -285,13 +286,8 @@ public final class TestPOIFSFileSystem extends TestCase { checkAllDirectoryContents((DirectoryEntry)entry); } else { DocumentNode doc = (DocumentNode) entry; - DocumentInputStream dis = new DocumentInputStream(doc); - try { - int numBytes = dis.available(); - byte[] data = new byte [numBytes]; - dis.read(data); - } finally { - dis.close(); + try (DocumentInputStream dis = new DocumentInputStream(doc)) { + IOUtils.toByteArray(dis); } } } diff --git a/src/testcases/org/apache/poi/util/TestIOUtils.java b/src/testcases/org/apache/poi/util/TestIOUtils.java index efda71779..b3ebeac4f 100644 --- a/src/testcases/org/apache/poi/util/TestIOUtils.java +++ b/src/testcases/org/apache/poi/util/TestIOUtils.java @@ -172,6 +172,29 @@ public final class TestIOUtils { } } + @Test(expected = RecordFormatException.class) + public void testMaxLengthTooLong() throws IOException { + try (InputStream is = new FileInputStream(TMP)) { + IOUtils.toByteArray(is, Integer.MAX_VALUE, 100); + } + } + + @Test + public void testMaxLengthIgnored() throws IOException { + try (InputStream is = new FileInputStream(TMP)) { + IOUtils.toByteArray(is, 90, Integer.MAX_VALUE); + IOUtils.toByteArray(is, 90, 100); + IOUtils.toByteArray(is, Integer.MAX_VALUE, Integer.MAX_VALUE); + } + } + + @Test(expected = RecordFormatException.class) + public void testMaxLengthInvalid() throws IOException { + try (InputStream is = new FileInputStream(TMP)) { + IOUtils.toByteArray(is, 90, 80); + } + } + @Test public void testWonkyInputStream() throws IOException { long skipped = IOUtils.skipFully(new WonkyInputStream(), 10000); diff --git a/src/testcases/org/apache/poi/util/TestLittleEndian.java b/src/testcases/org/apache/poi/util/TestLittleEndian.java index e6b7b46c4..67741921a 100644 --- a/src/testcases/org/apache/poi/util/TestLittleEndian.java +++ b/src/testcases/org/apache/poi/util/TestLittleEndian.java @@ -258,10 +258,14 @@ public final class TestLittleEndian { InputStream stream = new ByteArrayInputStream(_good_array); int count = 0; - while (stream.available() > 0) { - short value = LittleEndian.readShort(stream); - assertEquals(value, expected_value); - count++; + while (true) { + try { + short value = LittleEndian.readShort(stream); + assertEquals(value, expected_value); + count++; + } catch (BufferUnderrunException e) { + break; + } } assertEquals(count, _good_array.length / LittleEndianConsts.SHORT_SIZE); @@ -283,10 +287,14 @@ public final class TestLittleEndian { InputStream stream = new ByteArrayInputStream(_good_array); int count = 0; - while (stream.available() > 0) { - int value = LittleEndian.readInt(stream); - assertEquals(value, expected_value); - count++; + while (true) { + try { + int value = LittleEndian.readInt(stream); + assertEquals(value, expected_value); + count++; + } catch (BufferUnderrunException e) { + break; + } } assertEquals(count, _good_array.length / LittleEndianConsts.INT_SIZE); stream = new ByteArrayInputStream(_bad_array); @@ -308,10 +316,14 @@ public final class TestLittleEndian { InputStream stream = new ByteArrayInputStream(_good_array); int count = 0; - while (stream.available() > 0) { - long value = LittleEndian.readLong(stream); - assertEquals(value, expected_value); - count++; + while (true) { + try { + long value = LittleEndian.readLong(stream); + assertEquals(value, expected_value); + count++; + } catch (BufferUnderrunException e) { + break; + } } assertEquals(count, _good_array.length / LittleEndianConsts.LONG_SIZE); @@ -324,21 +336,17 @@ public final class TestLittleEndian { } } -// public void testReadFromStream() throws IOException { -// int actual; -// actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 5, -128, })); -// assertEquals(32773, actual); -// -// actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 1, 2, 3, 4, })); -// assertEquals(513, actual); -// -// try { -// LittleEndian.readInt(new ByteArrayInputStream(new byte[] { 1, 2, 3, })); -// fail("Should have caught BufferUnderrunException"); -// } catch (BufferUnderrunException ignored) { -// // as expected -// } -// } + @Test(expected = BufferUnderrunException.class) + public void testReadFromStream() throws IOException { + int actual; + actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 5, -128, })); + assertEquals(32773, actual); + + actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 1, 2, 3, 4, })); + assertEquals(513, actual); + + LittleEndian.readInt(new ByteArrayInputStream(new byte[] { 1, 2, 3, })); + } @Test public void testUnsignedByteToInt() {