(poi) branch trunk updated: Fix resource leak in ZipSecureFile constructors and correct path logging in ZipPackage (#1123)

[fa...@apache.org]

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 9bea74998a Fix resource leak in ZipSecureFile constructors and correct path logging in ZipPackage (#1123)
9bea74998a is described below

commit 9bea74998aee3242a1dd57bfcbf659fb8a67f54e
Author: KALI 834X <ka...@gmail.com>
AuthorDate: Tue Jun 9 00:13:23 2026 +0530

    Fix resource leak in ZipSecureFile constructors and correct path logging in ZipPackage (#1123)
    
    Fix file descriptor leak in ZipSecureFile and correct ZipPackage temp file log
    
    Co-authored-by: “sahvx655-wq” <“sahvx655@gmail.com”>
---
 .../org/apache/poi/openxml4j/opc/ZipPackage.java   |  2 +-
 .../apache/poi/openxml4j/util/ZipSecureFile.java   | 14 ++++++++--
 .../poi/openxml4j/util/TestZipSecureFile.java      | 30 ++++++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
index 1fe78faca8..6869d02a6e 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
@@ -591,7 +591,7 @@ public final class ZipPackage extends OPCPackage {
             } finally {
                 // Either the save operation succeed or not, we delete the temporary file
                 if (!tempFile.delete()) {
-                    LOG.atWarn().log("The temporary file: '{}' cannot be deleted ! Make sure that no other application use it.", targetFile.getAbsolutePath());
+                    LOG.atWarn().log("The temporary file: '{}' cannot be deleted ! Make sure that no other application use it.", tempFile.getAbsolutePath());
                 }
             }
         }
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
index 395e4fa5e5..4ccb069f81 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
@@ -210,7 +210,12 @@ public class ZipSecureFile extends ZipFile {
     public ZipSecureFile(File file) throws IOException {
         super(file);
         this.fileName = file.getAbsolutePath();
-        validateEntryNames();
+        try {
+            validateEntryNames();
+        } catch (IOException | RuntimeException | Error e) {
+            super.close();
+            throw e;
+        }
     }
 
     /**
@@ -220,7 +225,12 @@ public class ZipSecureFile extends ZipFile {
     public ZipSecureFile(String name) throws IOException {
         super(name);
         this.fileName = new File(name).getAbsolutePath();
-        validateEntryNames();
+        try {
+            validateEntryNames();
+        } catch (IOException | RuntimeException | Error e) {
+            super.close();
+            throw e;
+        }
     }
 
     /**
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 5d81c23091..261495f434 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,11 +19,19 @@ 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.TempFile;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.junit.jupiter.api.Test;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.Enumeration;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -101,4 +109,26 @@ class TestZipSecureFile {
             ZipSecureFile.setMaxFileCount(ZipSecureFile.DEFAULT_MAX_FILE_COUNT);
         }
     }
+
+    @Test
+    void testConstructorExceptionReleasesFileHandle() throws Exception {
+        File tempFile = TempFile.createTempFile("duplicate-entries", ".zip");
+        try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(tempFile)) {
+            ZipArchiveEntry entry1 = new ZipArchiveEntry("test.txt");
+            zos.putArchiveEntry(entry1);
+            zos.write("hello".getBytes(StandardCharsets.UTF_8));
+            zos.closeArchiveEntry();
+
+            ZipArchiveEntry entry2 = new ZipArchiveEntry("test.txt");
+            zos.putArchiveEntry(entry2);
+            zos.write("world".getBytes(StandardCharsets.UTF_8));
+            zos.closeArchiveEntry();
+        }
+
+        // The constructor should throw an IOException (specifically InvalidZipException)
+        assertThrows(IOException.class, () -> new ZipSecureFile(tempFile));
+
+        // 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");
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org