RFR: JDK-8319626: Override toString() for ZipFile [v7]

[Jaikiran Pai]

On Fri, 1 Dec 2023 21:32:57 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8319982) which overrides and provides an implementation of `toString()` in _java.util.zip.ZipFile_ (and by extension, _java.util.jar.JarFile_).
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   drop additional specification

src/java.base/share/classes/java/util/zip/ZipFile.java line 494:

> 492:     @Override
> 493:     public String toString() {
> 494:         return res.zsrc.key.file.getName()

Hello Justin, relying on `res.zsrc.key.file` to get to the file name can cause troubles. For example, consider this usage (which I ran in jshell) with this proposed change in this PR:


jshell> import java.util.zip.*

jshell> ZipFile zf = new ZipFile("/tmp/workdir.zip")
zf ==> workdir.zip at 34c45dca

jshell> zf.close()

jshell> zf.toString()
|  Exception java.lang.NullPointerException: Cannot read field "key" because "this.res.zsrc" is null
|        at ZipFile.toString (ZipFile.java:494)
|        at (#4:1)



What I do here is that I call `ZipFile.toString()` after I (intentionally) close the `ZipFile`. The `toString()` call leads to `NullPointerException` because on closing the `ZipFile` instance we clean up the resource it holds on to.

I think what we can do here is, something like (the following diff was generated on top of the current state of this PR):


diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java
index 67a5e65089f..2b6996294e1 100644
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java
@@ -95,7 +95,8 @@
  */
 public class ZipFile implements ZipConstants, Closeable {
 
-    private final String name;     // zip file name
+    private final String filePath;     // zip file path
+    private final String fileName;     // name of the file
     private volatile boolean closeRequested;
 
     // The "resource" used by this zip file that needs to be
@@ -245,7 +246,8 @@ public ZipFile(File file, int mode, Charset charset) throws IOException
         }
         Objects.requireNonNull(charset, "charset");
 
-        this.name = name;
+        this.filePath = name;
+        this.fileName = file.getName();
         long t0 = System.nanoTime();
 
         this.res = new CleanableResource(this, ZipCoder.get(charset), file, mode);
@@ -483,7 +485,7 @@ public int available() throws IOException {
      * @return the path name of the ZIP file
      */
     public String getName() {
-        return name;
+        return filePath;
     }
 
     /**
@@ -491,7 +493,7 @@ public String getName() {
      */
     @Override
     public String toString() {
-        return res.zsrc.key.file.getName()
+        return this.fileName
                 + "@" + Integer.toHexString(System.identityHashCode(this));
     }
 



The above example usage now works even after I close the ZipFile. I haven't run other tests, but given that this doesn't change any APIs or their implementation, I think the tests would run fine too.

Do you see any issues with this?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16643#discussion_r1420232060