Bug 66584 - ZipPackage can fail to handle excepions.
Summary: ZipPackage can fail to handle excepions.
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks: 67579
  Show dependency tree
 
Reported: 2023-04-28 08:59 UTC by zhonghao
Modified: 2023-10-03 10:01 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description zhonghao 2023-04-28 08:59:59 UTC
ZipPackage has a constructor:

ZipPackage(InputStream in, PackageAccess access) throws IOException {
        super(access);
        ZipArchiveThresholdInputStream zis = ZipHelper.openZipStream(in); // NOSONAR
        try {
            this.zipArchive = new ZipInputStreamZipEntrySource(zis);
        } catch (final IOException | RuntimeException e) {
            IOUtils.closeQuietly(zis);
            throw e;
        }
    }

Here, ZipHelper.openZipStream(in) can throw IOException, but it is not inside the try statement. It can be put inside the try statement to avoid exceptions. BTW, I notice that in the same class, IOException is caught and InvalidOperationException is rethrown:

 private static ZipEntrySource openZipEntrySourceStream(FileInputStream fis) throws InvalidOperationException {
 try {
            // open the zip input stream
            zis = ZipHelper.openZipStream(fis); // NOSONAR
        } catch (final IOException e) {
            // If the source cannot be acquired, abort (no resources to free at this level)
            throw new InvalidOperationException("Could not open the file input stream", e);
        }
        ...
}
Comment 1 PJ Fanning 2023-04-28 09:13:50 UTC
thanks - I made a change with r1909467
Comment 2 PJ Fanning 2023-10-03 10:01:08 UTC
This change has been largely reverted as part of https://bz.apache.org/bugzilla/show_bug.cgi?id=67579

The original change is in POI 5.2.4 but it is causing a regression will be undone in future releases.

It is the responsibility of users who have InputStreams that they feed into POI APIs to close those streams themselves.

Something like:

try(
    InputStream stream = ...;
    XSSFWorkbook workbook = new XSSFWorkbook(stream)
) {
    // use workbook
}

The try-with-resources syntax will ensure that stream and workbook are closed after the try block completes (regardless of whether there is an exception or not).