Bug 60713 - (S)XSSFWorkbook/POIXMLDocument.write(OutputStream) closes the OutputStream
Summary: (S)XSSFWorkbook/POIXMLDocument.write(OutputStream) closes the OutputStream
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: OPC (show other bugs)
Version: 4.0.x-dev
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-09 06:35 UTC by ddchzyj9
Modified: 2018-06-14 22:27 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ddchzyj9 2017-02-09 06:35:34 UTC
SXSSFWorkbook.write(OutputStream) will close OutputStream. if i want to write multi workbooks to one OutputStream, that's impossible. should the OutputStream be closed by creator rather than user?
Comment 1 Javen O'Neal 2017-02-09 17:02:02 UTC
That's a good point--whatever opens a stream should be responsible for closing it.

However, you may have problems with corrupt workbooks if you cat two workbooks together, so I don't see this as a real use case.
Comment 2 Nick Burch 2017-02-10 11:16:22 UTC
I believe that all the write methods should be closing the streams, because once POI has finished writing to the File / Stream, there's nothing more that can be done to them. All the file formats are write-once only, none of them support subsequent appending.
Comment 3 Javen O'Neal 2017-02-17 05:02:04 UTC
There should only be a problem if the OutputStream's close method follows the AutoCloseable interface and is not idempotent, where subsequent calls to close have visible side effects.

At the creation of these classes and prior to Java 7, OutputStream implemented the Closeable interface, which requires subsequent calls to close to have no effect.

We must balance that with users forgetting to close their own input streams and leaking resources (though that's on them). If we change POI to no longer close user-opened OutputStreams, we should document this changed behavior in both the Javadocs and changelog, making sure to update all of our POI example code and website to close the provided stream (that'd be a good idea anyways).

https://docs.oracle.com/javase/7/docs/api/java/io/OutputStream.html#close()
Comment 4 Javen O'Neal 2017-02-17 05:12:04 UTC
ddchzyj99, what kind of OutputStream are you using that has problems being closed twice?

I think we may want to err on the side of no change unless someone has a nonhypothetical scenario where this impacts them.
Comment 5 Dominik Stadler 2017-02-17 05:56:32 UTC
I agree, it sounds like a theoretical issue and changing this very likely would have bad side-effects for existing users.
Comment 6 Nick Burch 2017-02-17 11:08:42 UTC
We already have a helper method on NPOIFS for people who don't want the auto-close behaviour when opening from an InputStream - https://poi.apache.org/apidocs/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.html#createNonClosingInputStream(java.io.InputStream)

Maybe we could add a similar one for OutputStream, to help people wrap their problematic ones if needed?
Comment 7 Greg Woolsey 2017-02-17 17:34:33 UTC
I'm also in favor of a helper method, to avoid a dramatic change to the API.  Big breaks in backward compatibility mean longer delays before downstream projects taken an update, and often I want newer functionality available to my projects that use those downstream projects sooner rather than later.  Maintaining backward compatibility speeds that up, or lets me update POI independent of other libraries.
Comment 8 Andreas Beeker 2018-06-13 17:34:22 UTC
This closing habbit already annoyed me several times and I've actually added a fix via r1832746

Just have a look at [1] this closing is simply confusing.
Furthermore if you write a file backed OPC-Document to a different file, the original file gets also modified [2].

We should really fix this for 4.0.0.


[1] https://stackoverflow.com/questions/50646538/stream-close-exception-occures
[2] https://stackoverflow.com/a/50830215/2066598
Comment 9 Andreas Beeker 2018-06-14 22:27:52 UTC
Applied the SXSSFWorkbook specific patch in r1833566
The EncryptedTempData needs to be closed now.