Patch from Josh from bug #44366 - InputStreams passed to POIFSFileSystem are now automatically closed. A warning is generated for people who might've relied on them not being closed before, and a wrapper to restore the old behaviour is supplied

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@629829 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Nick Burch 2008-02-21 15:35:59 +00:00
parent 173f63cd0a
commit 38421b7650
4 changed files with 238 additions and 16 deletions

View File

@ -36,6 +36,7 @@
<!-- Don't forget to update status.xml too! --> <!-- Don't forget to update status.xml too! -->
<release version="3.1-beta1" date="2008-??-??"> <release version="3.1-beta1" date="2008-??-??">
<action dev="POI-DEVELOPERS" type="fix">44366 - InputStreams passed to POIFSFileSystem are now automatically closed. A warning is generated for people who might've relied on them not being closed before, and a wrapper to restore the old behaviour is supplied</action>
<action dev="POI-DEVELOPERS" type="add">44371 - Support for the Offset function</action> <action dev="POI-DEVELOPERS" type="add">44371 - Support for the Offset function</action>
<action dev="POI-DEVELOPERS" type="fix">38921 - Have HSSFPalette.findSimilar() work properly</action> <action dev="POI-DEVELOPERS" type="fix">38921 - Have HSSFPalette.findSimilar() work properly</action>
<action dev="POI-DEVELOPERS" type="fix">44456 - Fix the contrib SViewer / SViewerPanel to not fail on sheets with missing rows</action> <action dev="POI-DEVELOPERS" type="fix">44456 - Fix the contrib SViewer / SViewerPanel to not fail on sheets with missing rows</action>

View File

@ -33,6 +33,7 @@
<!-- Don't forget to update changes.xml too! --> <!-- Don't forget to update changes.xml too! -->
<changes> <changes>
<release version="3.1-beta1" date="2008-??-??"> <release version="3.1-beta1" date="2008-??-??">
<action dev="POI-DEVELOPERS" type="fix">44366 - InputStreams passed to POIFSFileSystem are now automatically closed. A warning is generated for people who might've relied on them not being closed before, and a wrapper to restore the old behaviour is supplied</action>
<action dev="POI-DEVELOPERS" type="add">44371 - Support for the Offset function</action> <action dev="POI-DEVELOPERS" type="add">44371 - Support for the Offset function</action>
<action dev="POI-DEVELOPERS" type="fix">38921 - Have HSSFPalette.findSimilar() work properly</action> <action dev="POI-DEVELOPERS" type="fix">38921 - Have HSSFPalette.findSimilar() work properly</action>
<action dev="POI-DEVELOPERS" type="fix">44456 - Fix the contrib SViewer / SViewerPanel to not fail on sheets with missing rows</action> <action dev="POI-DEVELOPERS" type="fix">44456 - Fix the contrib SViewer / SViewerPanel to not fail on sheets with missing rows</action>

View File

@ -19,6 +19,7 @@
package org.apache.poi.poifs.filesystem; package org.apache.poi.poifs.filesystem;
import java.io.ByteArrayInputStream;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
@ -30,6 +31,8 @@ import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.poi.poifs.dev.POIFSViewable; import org.apache.poi.poifs.dev.POIFSViewable;
import org.apache.poi.poifs.property.DirectoryProperty; import org.apache.poi.poifs.property.DirectoryProperty;
import org.apache.poi.poifs.property.Property; import org.apache.poi.poifs.property.Property;
@ -58,6 +61,33 @@ import org.apache.poi.util.LongField;
public class POIFSFileSystem public class POIFSFileSystem
implements POIFSViewable implements POIFSViewable
{ {
private static final Log _logger = LogFactory.getLog(POIFSFileSystem.class);
private static final class CloseIgnoringInputStream extends InputStream {
private final InputStream _is;
public CloseIgnoringInputStream(InputStream is) {
_is = is;
}
public int read() throws IOException {
return _is.read();
}
public int read(byte[] b, int off, int len) throws IOException {
return _is.read(b, off, len);
}
public void close() {
// do nothing
}
}
/**
* Convenience method for clients that want to avoid the auto-close behaviour of the constructor.
*/
public static InputStream createNonClosingInputStream(InputStream is) {
return new CloseIgnoringInputStream(is);
}
private PropertyTable _property_table; private PropertyTable _property_table;
private List _documents; private List _documents;
private DirectoryNode _root; private DirectoryNode _root;
@ -74,23 +104,52 @@ public class POIFSFileSystem
} }
/** /**
* Create a POIFSFileSystem from an InputStream * Create a POIFSFileSystem from an <tt>InputStream</tt>. Normally the stream is read until
* EOF. The stream is always closed.<p/>
*
* Some streams are usable after reaching EOF (typically those that return <code>true</code>
* for <tt>markSupported()</tt>). In the unlikely case that the caller has such a stream
* <i>and</i> needs to use it after this constructor completes, a work around is to wrap the
* stream in order to trap the <tt>close()</tt> call. A convenience method (
* <tt>createNonClosingInputStream()</tt>) has been provided for this purpose:
* <pre>
* InputStream wrappedStream = POIFSFileSystem.createNonClosingInputStream(is);
* HSSFWorkbook wb = new HSSFWorkbook(wrappedStream);
* is.reset();
* doSomethingElse(is);
* </pre>
* Note also the special case of <tt>ByteArrayInputStream</tt> for which the <tt>close()</tt>
* method does nothing.
* <pre>
* ByteArrayInputStream bais = ...
* HSSFWorkbook wb = new HSSFWorkbook(bais); // calls bais.close() !
* bais.reset(); // no problem
* doSomethingElse(bais);
* </pre>
* *
* @param stream the InputStream from which to read the data * @param stream the InputStream from which to read the data
* *
* @exception IOException on errors reading, or on invalid data * @exception IOException on errors reading, or on invalid data
*/ */
public POIFSFileSystem(final InputStream stream) public POIFSFileSystem(InputStream stream)
throws IOException throws IOException
{ {
this(); this();
boolean success = false;
// read the header block from the stream // read the header block from the stream
HeaderBlockReader header_block_reader = new HeaderBlockReader(stream); HeaderBlockReader header_block_reader;
// read the rest of the stream into blocks // read the rest of the stream into blocks
RawDataBlockList data_blocks = new RawDataBlockList(stream); RawDataBlockList data_blocks;
try {
header_block_reader = new HeaderBlockReader(stream);
data_blocks = new RawDataBlockList(stream);
success = true;
} finally {
closeInputStream(stream, success);
}
// set up the block allocation table (necessary for the // set up the block allocation table (necessary for the
// data_blocks to be manageable // data_blocks to be manageable
@ -112,6 +171,31 @@ public class POIFSFileSystem
.getSBATStart()), data_blocks, properties.getRoot() .getSBATStart()), data_blocks, properties.getRoot()
.getChildren(), null); .getChildren(), null);
} }
/**
* @param stream the stream to be closed
* @param success <code>false</code> if an exception is currently being thrown in the calling method
*/
private void closeInputStream(InputStream stream, boolean success) {
if(stream.markSupported() && !(stream instanceof ByteArrayInputStream)) {
String msg = "POIFS is closing the supplied input stream of type ("
+ stream.getClass().getName() + ") which supports mark/reset. "
+ "This will be a problem for the caller if the stream will still be used. "
+ "If that is the case the caller should wrap the input stream to avoid this close logic. "
+ "This warning is only temporary and will not be present in future versions of POI.";
_logger.warn(msg);
}
try {
stream.close();
} catch (IOException e) {
if(success) {
throw new RuntimeException(e);
}
// else not success? Try block did not complete normally
// just print stack trace and leave original ex to be thrown
e.printStackTrace();
}
}
/** /**
* Checks that the supplied InputStream (which MUST * Checks that the supplied InputStream (which MUST

View File

@ -0,0 +1,136 @@
/* ====================================================================
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==================================================================== */
package org.apache.poi.poifs.filesystem;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import junit.framework.TestCase;
/**
* Tests for POIFSFileSystem
*
* @author Josh Micich
*/
public final class TestPOIFSFileSystem extends TestCase {
public TestPOIFSFileSystem(String testName) {
super(testName);
}
/**
* Mock exception used to ensure correct error handling
*/
private static final class MyEx extends RuntimeException {
public MyEx() {
// no fields to initialise
}
}
/**
* Helps facilitate testing. Keeps track of whether close() was called.
* Also can throw an exception at a specific point in the stream.
*/
private static final class TestIS extends InputStream {
private final InputStream _is;
private final int _failIndex;
private int _currentIx;
private boolean _isClosed;
public TestIS(InputStream is, int failIndex) {
_is = is;
_failIndex = failIndex;
_currentIx = 0;
_isClosed = false;
}
public int read() throws IOException {
int result = _is.read();
if(result >=0) {
checkRead(1);
}
return result;
}
public int read(byte[] b, int off, int len) throws IOException {
int result = _is.read(b, off, len);
checkRead(result);
return result;
}
private void checkRead(int nBytes) {
_currentIx += nBytes;
if(_failIndex > 0 && _currentIx > _failIndex) {
throw new MyEx();
}
}
public void close() throws IOException {
_isClosed = true;
_is.close();
}
public boolean isClosed() {
return _isClosed;
}
}
/**
* Test for undesired behaviour observable as of svn revision 618865 (5-Feb-2008).
* POIFSFileSystem was not closing the input stream.
*/
public void testAlwaysClose() {
TestIS testIS;
// Normal case - read until EOF and close
testIS = new TestIS(openSampleStream("13224.xls"), -1);
try {
new POIFSFileSystem(testIS);
} catch (IOException e) {
throw new RuntimeException(e);
}
assertTrue("input stream was not closed", testIS.isClosed());
// intended to crash after reading 10000 bytes
testIS = new TestIS(openSampleStream("13224.xls"), 10000);
try {
new POIFSFileSystem(testIS);
fail("ex should have been thrown");
} catch (IOException e) {
throw new RuntimeException(e);
} catch (MyEx e) {
// expected
}
assertTrue("input stream was not closed", testIS.isClosed()); // but still should close
}
private static InputStream openSampleStream(String sampleName) {
String dataDirName = System.getProperty("HSSF.testdata.path");
if(dataDirName == null) {
throw new RuntimeException("Missing system property '" + "HSSF.testdata.path" + "'");
}
File f = new File(dataDirName + "/" + sampleName);
try {
return new FileInputStream(f);
} catch (FileNotFoundException e) {
throw new RuntimeException("Sample file '" + f.getAbsolutePath() + "' not found");
}
}
}