Bug 67579 - POI 5.2.4 is closing the input stream used in the XSSFWorkbook constructor which explicitly specifies closeStream=false
Summary: POI 5.2.4 is closing the input stream used in the XSSFWorkbook constructor w...
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: unspecified
Hardware: PC All
: P2 regression (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on: 66584
Blocks:
  Show dependency tree
 
Reported: 2023-10-02 21:48 UTC by nniesen
Modified: 2023-12-01 14:42 UTC (History)
0 users



Attachments
Zip file with 2 POI XSSFWorkbook exports (7.61 KB, application/zip)
2023-10-02 21:48 UTC, nniesen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description nniesen 2023-10-02 21:48:48 UTC
Created attachment 39088 [details]
Zip file with 2 POI XSSFWorkbook exports

Version 5.2.4 started closing the InputStream for 'new XSSFWorkbook(zipInputStream)' which explicitly specifies closeStream=false

MavenCentral artifact:
implementation 'org.apache.poi:poi-ooxml:5.2.4'

Spock test (works with 5.2.3, fails with 5.2.4):

def 'testExportAll'() {
    given:
    InputStream fileInputStream = new File('src/test/groovy/ExportAll.zip').newInputStream()

    when: 'export all zip unpacked'
    ZipInputStream zipInputStream = new ZipInputStream(fileInputStream)

    then: 'read first file (entry)'
    ZipEntry zipEntry1 = zipInputStream.getNextEntry()
    assert zipEntry1.name == 'testExport1.xlsx'
    Workbook workbook1 = new XSSFWorkbook(zipInputStream)
    assert workbook1.getSheetAt(0).sheetName == 'testExport1'

    and: 'read first file (entry)'
    ZipEntry zipEntry2 = zipInputStream.getNextEntry() // poi-ooxml:5.2.2 throws java.io.IOException: Stream closed

    assert zipEntry2.name == 'testExport2.xlsx'
    Workbook workbook2 = new XSSFWorkbook(zipInputStream)
    assert workbook2.getSheetAt(0).sheetName == 'testExport2'

    and: 'should be no more'
    assert zipInputStream.getNextEntry() == null
}
Comment 1 nniesen 2023-10-02 21:50:22 UTC
Test comment was supposed to read:
// poi-ooxml:5.2.4 throws java.io.IOException: Stream closed
Comment 2 nniesen 2023-10-02 22:20:15 UTC
ZipPackage 5.2.4 changed to a try-with-resources which is : 

130:    ZipPackage(InputStream in, PackageAccess access) throws IOException {
131:        super(access);
132:        try (ZipArchiveThresholdInputStream zis = ZipHelper.openZipStream(in)) {
133:            this.zipArchive = new ZipInputStreamZipEntrySource(zis);


ZipPackage 5.2.3 didn't close the input stream:
128:    ZipPackage(InputStream in, PackageAccess access) throws IOException {
129:        super(access);
130:        ZipArchiveThresholdInputStream zis = ZipHelper.openZipStream(in); // NOSONAR
131:        try {
132:            this.zipArchive = new ZipInputStreamZipEntrySource(zis);
Comment 3 PJ Fanning 2023-10-03 00:07:55 UTC
added r1912700

will wait a week or so to see if other regressions are reported before seeing about a POI 5.2.5 RC
Comment 4 PJ Fanning 2023-10-03 00:10:08 UTC
breaking change was in https://bz.apache.org/bugzilla/show_bug.cgi?id=66584
Comment 5 PJ Fanning 2023-10-26 15:15:22 UTC
@nniesen - we might need to undo the changes I put in. 

If you read the stuff about poi-reproducer in https://lists.apache.org/thread/0l5mrsb097rrwbvk5sgshp725zop6q44 - you'll see that POI 5.2.3 is the outlier. Most POI releases have closed input streams after using them. So the argument is that POI 5.2.3 had the regression and that POI 5.2.4 undid the regression.

I agree that POI shouldn't close user-provided input streams but the truth is that POI has done so and done so for a long time.

Anyone who wants to stop POI closing their InputStreams could wrap their stream to disable the close on their stream.

Example in https://github.com/apache/poi/blob/f64524916d449d37350e0763e4f86edc239bbd34/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/NoCloseInputStream.java
Comment 6 PJ Fanning 2023-10-26 17:39:10 UTC
reverted recent changes with r1913356

Closing as Will Not Be Done. See Comment 5.
Comment 7 PJ Fanning 2023-10-26 18:11:22 UTC
Adding a new XSSFWorkbook that might be useful - r1913359
Comment 8 nniesen 2023-11-16 16:48:54 UTC
You've added a couple of options that will work perfectly fine to prevent the stream from being closed so THANK YOU very much for your help and diligence.
Comment 9 PJ Fanning 2023-11-16 20:09:41 UTC
POI 5.2.4 had a regression where it did not close user-provided InputStreams. In POI 5.2.5, user-provided InputStreams are again closed. There are new constructors that allow you to control whether the user-provided streams are closed.
Comment 10 nniesen 2023-12-01 14:42:42 UTC
Moot now but for posterity the regression was more likely in POI 5.2.0 revision 1896461 Dec 2021. We had been using the same `XSSFWorkbook(InputStream)` constructor since 5.2.1 and it didn't close the stream.