This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new 4531c9c6db Fix temporary file leak in ZipInputStreamZipEntrySource on
constructor failure (#1128)
4531c9c6db is described below
commit 4531c9c6dbdbe2b8436f75ab12e248ba254309c4
Author: KALI 834X <[email protected]>
AuthorDate: Thu Jun 11 14:58:29 2026 +0530
Fix temporary file leak in ZipInputStreamZipEntrySource on constructor
failure (#1128)
* Fix temporary file leak in ZipInputStreamZipEntrySource constructor
* Fix forbidden method invocations by using DefaultTempFileCreationStrategy
in TestZipSecureFile
---------
Co-authored-by: “sahvx655-wq” <“[email protected]”>
---
.../util/ZipInputStreamZipEntrySource.java | 38 ++++++++-----
.../poi/openxml4j/util/TestZipSecureFile.java | 64 ++++++++++++++++++++++
2 files changed, 89 insertions(+), 13 deletions(-)
diff --git
a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java
b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java
index dfb9a1230f..5eb1124fde 100644
---
a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java
+++
b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java
@@ -98,21 +98,33 @@ public class ZipInputStreamZipEntrySource implements
ZipEntrySource {
*/
public ZipInputStreamZipEntrySource(ZipArchiveThresholdInputStream inp)
throws IOException {
final Set<String> filenames = new HashSet<>();
- for (;;) {
- final ZipArchiveEntry zipEntry = inp.getNextEntry();
- if (zipEntry == null) {
- break;
+ try {
+ for (;;) {
+ final ZipArchiveEntry zipEntry = inp.getNextEntry();
+ if (zipEntry == null) {
+ break;
+ }
+ String name = zipEntry.getName();
+ if (name == null || name.isEmpty()) {
+ throw new InvalidZipException("Input file contains an
entry with an empty name");
+ }
+ name = IOUtils.normalizePath(name).toLowerCase(Locale.ROOT);
+ if (filenames.contains(name)) {
+ throw new InvalidZipException("Input file contains more
than 1 entry with the name " + zipEntry.getName());
+ }
+ filenames.add(name);
+ zipEntries.put(name, new ZipArchiveFakeEntry(zipEntry, inp));
}
- String name = zipEntry.getName();
- if (name == null || name.isEmpty()) {
- throw new InvalidZipException("Input file contains an entry
with an empty name");
+ } catch (IOException | RuntimeException | Error e) {
+ for (ZipArchiveFakeEntry entry : zipEntries.values()) {
+ try {
+ entry.close();
+ } catch (IOException ioex) {
+ // ignore
+ }
}
- name = IOUtils.normalizePath(name).toLowerCase(Locale.ROOT);
- if (filenames.contains(name)) {
- throw new InvalidZipException("Input file contains more than 1
entry with the name " + zipEntry.getName());
- }
- filenames.add(name);
- zipEntries.put(name, new ZipArchiveFakeEntry(zipEntry, inp));
+ zipEntries.clear();
+ throw e;
}
streamToClose = inp;
diff --git
a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
index fb2885c0da..41c04fb9be 100644
---
a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
+++
b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
@@ -19,7 +19,9 @@ package org.apache.poi.openxml4j.util;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;
import org.apache.poi.util.IOUtils;
+import org.apache.poi.util.DefaultTempFileCreationStrategy;
import org.apache.poi.util.TempFile;
+import org.apache.poi.util.TempFileCreationStrategy;
import org.apache.poi.xssf.XSSFTestDataSamples;
import org.junit.jupiter.api.Test;
@@ -28,10 +30,14 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.ArrayList;
import java.util.Enumeration;
+import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
+import org.apache.poi.openxml4j.opc.internal.ZipHelper;
import static org.junit.jupiter.api.Assertions.*;
@@ -130,6 +136,64 @@ class TestZipSecureFile {
// If the file descriptor was correctly closed, we should be able to
delete the file
assertTrue(tempFile.delete(), "Temporary file should be successfully
deleted after constructor failure");
+ }
+
+ @Test
+ void testZipInputStreamZipEntrySourceExceptionReleasesResources() throws
Exception {
+ List<File> createdFiles = new ArrayList<>();
+ TempFileCreationStrategy customStrategy = new
TempFileCreationStrategy() {
+ private final TempFileCreationStrategy delegate = new
DefaultTempFileCreationStrategy();
+ @Override
+ public File createTempFile(String prefix, String suffix) throws
IOException {
+ File f = delegate.createTempFile(prefix, suffix);
+ createdFiles.add(f);
+ return f;
+ }
+ @Override
+ public File createTempDirectory(String prefix) throws IOException {
+ return delegate.createTempDirectory(prefix);
+ }
+ };
+
+ TempFile.withStrategy(customStrategy, () -> {
+ try {
+ int oldThreshold =
ZipInputStreamZipEntrySource.getThresholdBytesForTempFiles();
+ ZipInputStreamZipEntrySource.setThresholdBytesForTempFiles(0);
+ try {
+ File tempZipFile = TempFile.createTempFile("test-leak",
".zip");
+ try {
+ try (ZipArchiveOutputStream zos = new
ZipArchiveOutputStream(tempZipFile)) {
+ ZipArchiveEntry entry1 = new
ZipArchiveEntry("first.txt");
+ zos.putArchiveEntry(entry1);
+
zos.write("hello".getBytes(StandardCharsets.UTF_8));
+ zos.closeArchiveEntry();
+
+ ZipArchiveEntry entry2 = new
ZipArchiveEntry("first.txt");
+ zos.putArchiveEntry(entry2);
+
zos.write("world".getBytes(StandardCharsets.UTF_8));
+ zos.closeArchiveEntry();
+ }
+
+ try (InputStream is =
Files.newInputStream(tempZipFile.toPath());
+ ZipArchiveThresholdInputStream zis =
ZipHelper.openZipStream(is)) {
+ assertThrows(IOException.class, () -> new
ZipInputStreamZipEntrySource(zis));
+ }
+ } finally {
+ tempZipFile.delete();
+ }
+ } finally {
+
ZipInputStreamZipEntrySource.setThresholdBytesForTempFiles(oldThreshold);
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ return null;
+ });
+
+ assertFalse(createdFiles.isEmpty(), "At least one temporary file
should have been created for the zip entries");
+ for (File f : createdFiles) {
+ assertFalse(f.exists(), "Temporary file " + f.getAbsolutePath() +
" should have been deleted");
+ }
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
