From a59ed12ecf23da72e783742e84c0b428defb31c3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alain=20B=C3=A9arez?= Date: Tue, 21 May 2019 00:13:51 +0000 Subject: [PATCH] fix potential output resource leaks (LGTM) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1859592 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/record/FilePassRecord.java | 67 +++++++++-------- .../org/apache/poi/hssf/record/ObjRecord.java | 67 +++++++++-------- .../poi/hssf/record/StandardRecord.java | 73 +++++++++++-------- .../hssf/record/cont/ContinuableRecord.java | 22 ++++-- .../poi/ss/util/CellRangeAddressList.java | 26 ++++--- 5 files changed, 147 insertions(+), 108 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/FilePassRecord.java b/src/java/org/apache/poi/hssf/record/FilePassRecord.java index 3b54d058d2..8489447325 100644 --- a/src/java/org/apache/poi/hssf/record/FilePassRecord.java +++ b/src/java/org/apache/poi/hssf/record/FilePassRecord.java @@ -43,10 +43,10 @@ public final class FilePassRecord extends StandardRecord implements Cloneable { public static final short sid = 0x002F; private static final int ENCRYPTION_XOR = 0; private static final int ENCRYPTION_OTHER = 1; - + private final int encryptionType; private EncryptionInfo encryptionInfo; - + private FilePassRecord(FilePassRecord other) { encryptionType = other.encryptionType; try { @@ -55,15 +55,15 @@ public final class FilePassRecord extends StandardRecord implements Cloneable { throw new EncryptedDocumentException(e); } } - + public FilePassRecord(EncryptionMode encryptionMode) { encryptionType = (encryptionMode == EncryptionMode.xor) ? ENCRYPTION_XOR : ENCRYPTION_OTHER; encryptionInfo = new EncryptionInfo(encryptionMode); } - + public FilePassRecord(RecordInputStream in) { encryptionType = in.readUShort(); - + EncryptionMode preferredMode; switch (encryptionType) { case ENCRYPTION_XOR: @@ -75,7 +75,7 @@ public final class FilePassRecord extends StandardRecord implements Cloneable { default: throw new EncryptedDocumentException("invalid encryption type"); } - + try { encryptionInfo = new EncryptionInfo(in, preferredMode); } catch (IOException e) { @@ -88,32 +88,37 @@ public final class FilePassRecord extends StandardRecord implements Cloneable { public void serialize(LittleEndianOutput out) { out.writeShort(encryptionType); - byte[] data = new byte[1024]; - LittleEndianByteArrayOutputStream bos = new LittleEndianByteArrayOutputStream(data, 0); // NOSONAR + byte data[] = new byte[1024]; + try (LittleEndianByteArrayOutputStream bos = + new LittleEndianByteArrayOutputStream(data, 0)) { // NOSONAR - switch (encryptionInfo.getEncryptionMode()) { - case xor: - ((XOREncryptionHeader)encryptionInfo.getHeader()).write(bos); - ((XOREncryptionVerifier)encryptionInfo.getVerifier()).write(bos); - break; - case binaryRC4: - out.writeShort(encryptionInfo.getVersionMajor()); - out.writeShort(encryptionInfo.getVersionMinor()); - ((BinaryRC4EncryptionHeader)encryptionInfo.getHeader()).write(bos); - ((BinaryRC4EncryptionVerifier)encryptionInfo.getVerifier()).write(bos); - break; - case cryptoAPI: - out.writeShort(encryptionInfo.getVersionMajor()); - out.writeShort(encryptionInfo.getVersionMinor()); - out.writeInt(encryptionInfo.getEncryptionFlags()); - ((CryptoAPIEncryptionHeader)encryptionInfo.getHeader()).write(bos); - ((CryptoAPIEncryptionVerifier)encryptionInfo.getVerifier()).write(bos); - break; - default: - throw new EncryptedDocumentException("not supported"); - } + switch (encryptionInfo.getEncryptionMode()) { + case xor: + ((XOREncryptionHeader)encryptionInfo.getHeader()).write(bos); + ((XOREncryptionVerifier)encryptionInfo.getVerifier()).write(bos); + break; + case binaryRC4: + out.writeShort(encryptionInfo.getVersionMajor()); + out.writeShort(encryptionInfo.getVersionMinor()); + ((BinaryRC4EncryptionHeader)encryptionInfo.getHeader()).write(bos); + ((BinaryRC4EncryptionVerifier)encryptionInfo.getVerifier()).write(bos); + break; + case cryptoAPI: + out.writeShort(encryptionInfo.getVersionMajor()); + out.writeShort(encryptionInfo.getVersionMinor()); + out.writeInt(encryptionInfo.getEncryptionFlags()); + ((CryptoAPIEncryptionHeader)encryptionInfo.getHeader()).write(bos); + ((CryptoAPIEncryptionVerifier)encryptionInfo.getVerifier()).write(bos); + break; + default: + throw new EncryptedDocumentException("not supported"); + } - out.write(data, 0, bos.getWriteIndex()); + out.write(data, 0, bos.getWriteIndex()); + } catch (IOException ioe) { + // should never happen in practice + throw new IllegalStateException(ioe); + } } @Override @@ -132,7 +137,7 @@ public final class FilePassRecord extends StandardRecord implements Cloneable { public short getSid() { return sid; } - + @Override public FilePassRecord clone() { return new FilePassRecord(this); diff --git a/src/java/org/apache/poi/hssf/record/ObjRecord.java b/src/java/org/apache/poi/hssf/record/ObjRecord.java index d1b4ab4d11..bddd89ce2c 100644 --- a/src/java/org/apache/poi/hssf/record/ObjRecord.java +++ b/src/java/org/apache/poi/hssf/record/ObjRecord.java @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - + package org.apache.poi.hssf.record; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -28,7 +29,7 @@ import org.apache.poi.util.RecordFormatException; /** * OBJRECORD (0x005D)

- * + * * The obj record is used to hold various graphic objects and controls. */ public final class ObjRecord extends Record implements Cloneable { @@ -36,7 +37,7 @@ public final class ObjRecord extends Record implements Cloneable { private static final int NORMAL_PAD_ALIGNMENT = 2; private static int MAX_PAD_ALIGNMENT = 4; - + private List subrecords; /** used when POI has no idea what is going on */ private final byte[] _uninterpretedData; @@ -100,7 +101,7 @@ public final class ObjRecord extends Record implements Cloneable { _isPaddedToQuadByteMultiple = subRecordData.length % MAX_PAD_ALIGNMENT == 0; if (nRemainingBytes >= (_isPaddedToQuadByteMultiple ? MAX_PAD_ALIGNMENT : NORMAL_PAD_ALIGNMENT)) { if (!canPaddingBeDiscarded(subRecordData, nRemainingBytes)) { - String msg = "Leftover " + nRemainingBytes + String msg = "Leftover " + nRemainingBytes + " bytes in subrecord data " + HexDump.toHex(subRecordData); throw new RecordFormatException(msg); } @@ -118,7 +119,7 @@ public final class ObjRecord extends Record implements Cloneable { * written by a version of POI (around 3.1) which incorrectly interpreted the second short of * the ftLbs subrecord (0x1FEE) as a length, and read that many bytes as padding (other bugs * helped allow this to occur). - * + * * Excel reads files with this excessive padding OK, truncating the over-sized ObjRecord back * to the its proper size. POI does the same. */ @@ -145,7 +146,7 @@ public final class ObjRecord extends Record implements Cloneable { sb.append("[/OBJ]\n"); return sb.toString(); } - + @Override public int getRecordSize() { if (_uninterpretedData != null) { @@ -167,31 +168,35 @@ public final class ObjRecord extends Record implements Cloneable { return size + 4; } - @Override - public int serialize(int offset, byte[] data) { - int recSize = getRecordSize(); - int dataSize = recSize - 4; - LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, offset, recSize); // NOSONAR - - out.writeShort(sid); - out.writeShort(dataSize); - - if (_uninterpretedData == null) { - - for (int i = 0; i < subrecords.size(); i++) { - SubRecord record = subrecords.get(i); - record.serialize(out); - } - int expectedEndIx = offset+dataSize; - // padding - while (out.getWriteIndex() < expectedEndIx) { - out.writeByte(0); - } - } else { - out.write(_uninterpretedData); - } - return recSize; - } + @Override + public int serialize(int offset, byte[] data) { + int recSize = getRecordSize(); + int dataSize = recSize - 4; + + try (LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, offset, recSize)) { // NOSONAR + out.writeShort(sid); + out.writeShort(dataSize); + + if (_uninterpretedData == null) { + + for (int i = 0; i < subrecords.size(); i++) { + SubRecord record = subrecords.get(i); + record.serialize(out); + } + int expectedEndIx = offset + dataSize; + // padding + while (out.getWriteIndex() < expectedEndIx) { + out.writeByte(0); + } + } else { + out.write(_uninterpretedData); + } + } catch (IOException ioe) { + // should never happen in practice + throw new IllegalStateException(ioe); + } + return recSize; + } @Override public short getSid() { diff --git a/src/java/org/apache/poi/hssf/record/StandardRecord.java b/src/java/org/apache/poi/hssf/record/StandardRecord.java index 54886d7e26..610c57b835 100644 --- a/src/java/org/apache/poi/hssf/record/StandardRecord.java +++ b/src/java/org/apache/poi/hssf/record/StandardRecord.java @@ -17,51 +17,62 @@ package org.apache.poi.hssf.record; +import java.io.IOException; + import org.apache.poi.util.LittleEndianByteArrayOutputStream; import org.apache.poi.util.LittleEndianOutput; /** - * Subclasses of this class (the majority of BIFF records) are non-continuable. This allows for - * some simplification of serialization logic + * Subclasses of this class (the majority of BIFF records) are non-continuable. + * This allows for some simplification of serialization logic */ public abstract class StandardRecord extends Record { - protected abstract int getDataSize(); - public final int getRecordSize() { - return 4 + getDataSize(); - } + protected abstract int getDataSize(); + + @Override + public final int getRecordSize() { + return 4 + getDataSize(); + } /** - * Write the data content of this BIFF record including the sid and record length. + * Write the data content of this BIFF record including the sid and record + * length. *

* The subclass must write the exact number of bytes as reported by - * {@link org.apache.poi.hssf.record.Record#getRecordSize()}} + * {@link org.apache.poi.hssf.record.Record#getRecordSize()}} */ - @Override - public final int serialize(int offset, byte[] data) { - int dataSize = getDataSize(); - int recSize = 4 + dataSize; - LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, offset, recSize); - out.writeShort(getSid()); - out.writeShort(dataSize); - serialize(out); - if (out.getWriteIndex() - offset != recSize) { - throw new IllegalStateException("Error in serialization of (" + getClass().getName() + "): " - + "Incorrect number of bytes written - expected " - + recSize + " but got " + (out.getWriteIndex() - offset)); - } - return recSize; - } + @Override + public final int serialize(int offset, byte[] data) { + int dataSize = getDataSize(); + int recSize = 4 + dataSize; + try (LittleEndianByteArrayOutputStream out = + new LittleEndianByteArrayOutputStream(data, offset, recSize)) { + out.writeShort(getSid()); + out.writeShort(dataSize); + serialize(out); + if (out.getWriteIndex() - offset != recSize) { + throw new IllegalStateException("Error in serialization of (" + getClass().getName() + "): " + + "Incorrect number of bytes written - expected " + recSize + " but got " + + (out.getWriteIndex() - offset)); + } + } catch (IOException ioe) { + // should never happen in practice + throw new IllegalStateException(ioe); + } + return recSize; + } /** - * Write the data content of this BIFF record. The 'ushort sid' and 'ushort size' header fields - * have already been written by the superclass. + * Write the data content of this BIFF record. The 'ushort sid' and 'ushort + * size' header fields have already been written by the superclass. *

* The number of bytes written must equal the record size reported by - * {@link org.apache.poi.hssf.record.Record#getRecordSize()}} minus four - * ( record header consisting of a 'ushort sid' and 'ushort reclength' has already been written - * by their superclass). - * - * @param out the output object + * {@link org.apache.poi.hssf.record.Record#getRecordSize()}} minus four ( + * record header consisting of a 'ushort sid' and 'ushort reclength' has + * already been written by their superclass). + * + * @param out + * the output object */ - protected abstract void serialize(LittleEndianOutput out); + protected abstract void serialize(LittleEndianOutput out); } diff --git a/src/java/org/apache/poi/hssf/record/cont/ContinuableRecord.java b/src/java/org/apache/poi/hssf/record/cont/ContinuableRecord.java index 88399074a4..8c170aa3af 100644 --- a/src/java/org/apache/poi/hssf/record/cont/ContinuableRecord.java +++ b/src/java/org/apache/poi/hssf/record/cont/ContinuableRecord.java @@ -17,6 +17,8 @@ package org.apache.poi.hssf.record.cont; +import java.io.IOException; + import org.apache.poi.hssf.record.ContinueRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.util.LittleEndianByteArrayOutputStream; @@ -49,6 +51,7 @@ public abstract class ContinuableRecord extends Record { * (Note - if any {@link ContinueRecord} is required, this result includes the * size of those too) */ + @Override public final int getRecordSize() { ContinuableRecordOutput out = ContinuableRecordOutput.createForCountingOnly(); serialize(out); @@ -56,12 +59,19 @@ public abstract class ContinuableRecord extends Record { return out.getTotalSize(); } + @Override public final int serialize(int offset, byte[] data) { - - LittleEndianOutput leo = new LittleEndianByteArrayOutputStream(data, offset); - ContinuableRecordOutput out = new ContinuableRecordOutput(leo, getSid()); - serialize(out); - out.terminate(); - return out.getTotalSize(); + int totalSize = 0; + try (LittleEndianByteArrayOutputStream leo = + new LittleEndianByteArrayOutputStream(data, offset)) { + ContinuableRecordOutput out = new ContinuableRecordOutput(leo, getSid()); + serialize(out); + out.terminate(); + totalSize = out.getTotalSize(); + } catch (IOException ioe) { + // should never happen in practice + throw new IllegalStateException(ioe); + } + return totalSize; } } diff --git a/src/java/org/apache/poi/ss/util/CellRangeAddressList.java b/src/java/org/apache/poi/ss/util/CellRangeAddressList.java index 7dad2a763d..06c3f84e3f 100644 --- a/src/java/org/apache/poi/ss/util/CellRangeAddressList.java +++ b/src/java/org/apache/poi/ss/util/CellRangeAddressList.java @@ -17,6 +17,7 @@ package org.apache.poi.ss.util; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -28,13 +29,13 @@ import org.apache.poi.util.LittleEndianOutput; * Implementation of the cell range address lists,like is described * in OpenOffice.org's Excel Documentation: excelfileformat.pdf sec 2.5.14 - * 'Cell Range Address List' - * + * * In BIFF8 there is a common way to store absolute cell range address lists in * several records (not formulas). A cell range address list consists of a field * with the number of ranges and the list of the range addresses. Each cell * range address (called an ADDR structure) contains 4 16-bit-values. *

- * + * * @author Dragos Buleandra (dragos.buleandra@trade2b.ro) */ public class CellRangeAddressList { @@ -48,7 +49,7 @@ public class CellRangeAddressList { _list = new ArrayList<>(); } /** - * Convenience constructor for creating a CellRangeAddressList with a single + * Convenience constructor for creating a CellRangeAddressList with a single * CellRangeAddress. Other CellRangeAddresses may be added later. */ public CellRangeAddressList(int firstRow, int lastRow, int firstCol, int lastCol) { @@ -71,7 +72,7 @@ public class CellRangeAddressList { * structures is automatically set when reading an Excel file and/or * increased when you manually add a new ADDR structure . This is the reason * there isn't a set method for this field . - * + * * @return number of ADDR structures */ public int countRanges() { @@ -80,7 +81,7 @@ public class CellRangeAddressList { /** * Add a cell range structure. - * + * * @param firstRow - the upper left hand corner's row * @param firstCol - the upper left hand corner's col * @param lastRow - the lower right hand corner's row @@ -98,7 +99,7 @@ public class CellRangeAddressList { throw new RuntimeException("List is empty"); } if (rangeIndex < 0 || rangeIndex >= _list.size()) { - throw new RuntimeException("Range index (" + rangeIndex + throw new RuntimeException("Range index (" + rangeIndex + ") is outside allowable range (0.." + (_list.size()-1) + ")"); } return _list.remove(rangeIndex); @@ -124,9 +125,17 @@ public class CellRangeAddressList { public int serialize(int offset, byte[] data) { int totalSize = getSize(); - serialize(new LittleEndianByteArrayOutputStream(data, offset, totalSize)); + try (LittleEndianByteArrayOutputStream lebaos = + new LittleEndianByteArrayOutputStream(data, offset, totalSize)) { + serialize(lebaos); + } + catch (IOException ioe) { + // should never happen in practice + throw new IllegalStateException(ioe); + } return totalSize; } + public void serialize(LittleEndianOutput out) { int nItems = _list.size(); out.writeShort(nItems); @@ -135,11 +144,10 @@ public class CellRangeAddressList { region.serialize(out); } } - public CellRangeAddressList copy() { CellRangeAddressList result = new CellRangeAddressList(); - + int nItems = _list.size(); for (int k = 0; k < nItems; k++) { CellRangeAddress region = _list.get(k); -- 2.39.5