moparisthebest
/
poi
1
0
Fork 0
Code Issues Pull Requests Releases 1 Wiki Activity

Use IOUtils.closeQuietly() in more places 

Avoid two possible file-handle leaks when opening files fails with an exception
Make tests close resources properly to not spam the output when running with file-leak-detector

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1763485 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2016-10-05 20:00:07 +00:00
parent 89f68a6a47
commit cdef5dc16c
5 changed files with 96 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
View File

@ -54,6 +54,7 @@ import org.apache.poi.openxml4j.opc.internal.unmarshallers.PackagePropertiesUnma
import org.apache.poi.openxml4j.opc.internal.unmarshallers.UnmarshallContext;
import org.apache.poi.openxml4j.util.Nullable;
import org.apache.poi.openxml4j.util.ZipEntrySource;
import org.apache.poi.util.IOUtils;
import org.apache.poi.util.NotImplemented;
import org.apache.poi.util.POILogFactory;
import org.apache.poi.util.POILogger;
@ -221,18 +222,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
// pack.originalPackagePath = file.getAbsolutePath();
return pack;
} catch (InvalidFormatException e) {
try {
pack.close();
} catch (IOException e1) {
throw new IllegalStateException(e);
}
IOUtils.closeQuietly(pack);
throw e;
} catch (RuntimeException e) {
try {
pack.close();
} catch (IOException e1) {
throw new IllegalStateException(e);
}
IOUtils.closeQuietly(pack);
throw e;
}
}
@ -277,6 +270,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
}
}
}
pack.originalPackagePath = new File(path).getAbsolutePath();
return pack;
}
@ -310,18 +304,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
pack.originalPackagePath = file.getAbsolutePath();
return pack;
} catch (InvalidFormatException e) {
try {
pack.close();
} catch (IOException e1) {
throw new IllegalStateException(e);
}
IOUtils.closeQuietly(pack);
throw e;
} catch (RuntimeException e) {
try {
pack.close();
} catch (IOException e1) {
throw new IllegalStateException(e);
}
IOUtils.closeQuietly(pack);
throw e;
}
}
@ -340,8 +326,16 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
public static OPCPackage open(InputStream in) throws InvalidFormatException,
IOException {
OPCPackage pack = new ZipPackage(in, PackageAccess.READ_WRITE);
if (pack.partList == null) {
pack.getParts();
try {
if (pack.partList == null) {
pack.getParts();
}
} catch (InvalidFormatException e) {
IOUtils.closeQuietly(pack);
throw e;
} catch (RuntimeException e) {
IOUtils.closeQuietly(pack);
throw e;
}
return pack;
}
@ -357,13 +351,11 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
* Throws if the specified file exist and is not valid.
*/
public static OPCPackage openOrCreate(File file) throws InvalidFormatException {
OPCPackage retPackage = null;
if (file.exists()) {
retPackage = open(file.getAbsolutePath());
return open(file.getAbsolutePath());
} else {
retPackage = create(file);
return create(file);
}
return retPackage;
}
/**

38
src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java
View File

@ -17,11 +17,7 @@
package org.apache.poi.hwpf.usermodel;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.*;
import org.apache.poi.POIDataSamples;
import org.apache.poi.hwpf.HWPFDocument;
@ -90,10 +86,17 @@ public final class TestHWPFWrite extends HWPFTestCase {
public void testInPlaceWrite() throws Exception {
// Setup as a copy of a known-good file
final File file = TempFile.createTempFile("TestDocument", ".doc");
IOUtils.copy(
POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc"),
new FileOutputStream(file)
);
InputStream inputStream = POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc");
try {
FileOutputStream outputStream = new FileOutputStream(file);
try {
IOUtils.copy(inputStream, outputStream);
} finally {
outputStream.close();
}
} finally {
inputStream.close();
}
// Open from the temp file in read-write mode
HWPFDocument doc = new HWPFDocument(new NPOIFSFileSystem(file, false).getRoot());
@ -108,7 +111,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
doc.close();
doc = new HWPFDocument(new NPOIFSFileSystem(file).getRoot());
r = doc.getRange();
assertEquals("X XX a test document\r", r.getParagraph(0).text());
doc.close();
}
@SuppressWarnings("resource")
@ -121,7 +126,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
try {
doc.write();
fail("Shouldn't work for InputStream");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
doc.close();
// Can't work for OPOIFS
OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@ -130,7 +138,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
try {
doc.write();
fail("Shouldn't work for OPOIFSFileSystem");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
doc.close();
// Can't work for Read-Only files
NPOIFSFileSystem fs = new NPOIFSFileSystem(
@ -139,6 +150,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
try {
doc.write();
fail("Shouldn't work for Read Only");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
doc.close();
}
}

37
src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
View File

@ -1222,7 +1222,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
try {
wb.write();
fail("Shouldn't work for new files");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
wb.close();
// Can't work for InputStream opened files
wb = new HSSFWorkbook(
@ -1230,7 +1233,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
try {
wb.write();
fail("Shouldn't work for InputStream");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
wb.close();
// Can't work for OPOIFS
OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@ -1239,7 +1245,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
try {
wb.write();
fail("Shouldn't work for OPOIFSFileSystem");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
wb.close();
// Can't work for Read-Only files
NPOIFSFileSystem fs = new NPOIFSFileSystem(
@ -1248,17 +1257,27 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
try {
wb.write();
fail("Shouldn't work for Read Only");
} catch (IllegalStateException e) {}
} catch (IllegalStateException e) {
// expected here
}
wb.close();
}
@Test
public void inPlaceWrite() throws Exception {
// Setup as a copy of a known-good file
final File file = TempFile.createTempFile("TestHSSFWorkbook", ".xls");
IOUtils.copy(
POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls"),
new FileOutputStream(file)
);
InputStream inputStream = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls");
try {
FileOutputStream outputStream = new FileOutputStream(file);
try {
IOUtils.copy(inputStream, outputStream);
} finally {
outputStream.close();
}
} finally {
inputStream.close();
}
// Open from the temp file in read-write mode
HSSFWorkbook wb = new HSSFWorkbook(new NPOIFSFileSystem(file, false));
@ -1276,6 +1295,8 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
wb = new HSSFWorkbook(new NPOIFSFileSystem(file));
assertEquals(1, wb.getNumberOfSheets());
assertEquals("Changed!", wb.getSheetAt(0).getRow(0).getCell(0).toString());
wb.close();
}
@Test

39
src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
View File

@ -24,11 +24,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.*;
import java.nio.ByteBuffer;
import java.util.Iterator;
@ -96,20 +92,25 @@ public final class TestNPOIFSFileSystem {
HeaderBlock header = new HeaderBlock(new ByteArrayInputStream(baos.toByteArray()));
return header;
}
protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
original.writeFilesystem(baos);
original.close();
return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
}
protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
final FileOutputStream fout = new FileOutputStream(file);
original.writeFilesystem(fout);
original.close();
return new NPOIFSFileSystem(file, false);
}
protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
original.writeFilesystem(baos);
original.close();
return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
}
protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
final OutputStream fout = new FileOutputStream(file);
try {
original.writeFilesystem(fout);
} finally {
fout.close();
}
original.close();
return new NPOIFSFileSystem(file, false);
}
@Test
public void basicOpen() throws Exception {

4
src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
View File

@ -1017,7 +1017,7 @@ public abstract class BaseTestCell {
}
@Test
public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() {
public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() throws IOException {
// bug 59836
// until we have changes POI from working on primitives (int) to enums,
// we should make sure we minimize backwards compatibility breakages.
@ -1046,5 +1046,7 @@ public abstract class BaseTestCell {
default:
fail("unexpected cell type: " + cell.getCellType());
}
wb.close();
}
}