JDK-8338663 : Deprecate java.util.zip.ZipError for removal
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.util.jar
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 24
  • Submitted: 2024-08-20
  • Updated: 2024-10-03
  • Resolved: 2024-10-03
Related Reports
CSR :  
Description
Summary
-------

Deprecate for removal the class `java.util.zip.ZipError`.

Problem
-------

`ZipError` was introduced via JDK-4615343 in JDK 6 and extends `java.lang.InternalError` to maintain compatibiliy with any existing client code catching `InternalError`. 

Its usage was removed from `ZipFile` in JDK 9 when the use of native code transitioned to Java via JDK-8142508. The only other use was in the original `ZipFS` demo which also was removed when ZipFS was integrated into JDK 9 as a supported Module. The JDK never documented in any API specification that `ZipError` was thrown. 

The ZipError class is easy to confuse with similarly named `ZipException` class in the same package. It is not suitable for general-purpose ZIP exceptions as it extends InternalError, yet there is no warning against its usage.

A corpus search on GitHub and Maven showed only a handful of uses.

Solution
--------

Mark the class `java.util.zip.ZipError` as deprecated, for removal.

Affected client code should be updated to catch or throw `ZipException`. Code base needing to catch `ZipError` on JDK 8 should catch the super class `InternalError` instead.

This specification change will be accompanied by a Release Note in the release it is approved for.

Specification
-------------

```
diff --git a/src/java.base/share/classes/java/util/zip/ZipError.java b/src/java.base/share/classes/java/util/zip/ZipError.java
index 2aa37bef010..933cc447091 100644
--- a/src/java.base/share/classes/java/util/zip/ZipError.java
+++ b/src/java.base/share/classes/java/util/zip/ZipError.java
@@ -28,9 +28,12 @@
 /**
  * Signals that an unrecoverable error has occurred.
  *
+ * @deprecated ZipError is no longer used and is obsolete.
+ * {@link ZipException} should be used instead.
  * @author  Dave Bristor
  * @since   1.6
  */
+@Deprecated(since="24", forRemoval = true)
 public class ZipError extends InternalError {
     @java.io.Serial
     private static final long serialVersionUID = 853973422266861979L;
```

Comments
Thanks [~alanb]. Moving this CSR to Approved.
03-10-2024

I don't have a strong opinion either way on this proposal. Exposing ZipError extending InternalError was a mistake and deprecating ZipError to discourage use outside of the JDK is fine. It's not toxic and using for something outside of the JDK isn't going to lead to unsafe or unpredictable code, it's just weird. If you really want to deprecate it for removal now, or deprecate the constructor for removal, either is fine. The compatibility impact isn't significant. Removing the Error, or the public constructor, is a future release would require further analysis to be sure that nothing important is using it.
03-10-2024

> Anything significant in that small list? I used Github Code Search with the following query: `java.util.zip.ZipError NOT is:fork (language:Java OR language:Scala OR language:Groovy)` A few notable finds: > sbt/zinc, a incremental compiler library for Scala Reported https://github.com/sbt/zinc/issues/1385, which was swiftly merged > eclipse-jdt/eclipse.jdt.core, Eclipse JDT: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/master/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/indexing/AddJarFileToIndex.java#L286 Reported as: https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2856, no response so far > facebook/buck https://github.com/facebook/buck/blob/main/src/com/facebook/buck/util/unarchive/Unzip.java Buck 1 is an abandoned/dead version. Checked that Buck 2 has no usages of ZipError. > scalameta/metals Seems actively maintained, I could file an issue here. https://github.com/scalameta/metals/blob/main/metals/src/main/scala/scala/meta/internal/metals/JarTopLevels.scala Otherwise, it seems to be used in Minecraft installers and in various code bases which contains lists of JDK APIs. Edit: Also found: > apache/netbeans Reported as issue https://github.com/apache/netbeans/issues/7818
01-10-2024

> Anything significant in that small list? From a previous search, I find this is mostly used in code that maintains JDK 8 compatibility when reading zip, such as Minecraft mod loaders' usage of Zip file system. There's no known scenario using the constructor.
01-10-2024

"A handful of uses". Anything significant in that small list? If not then I think it should be okay to deprecate ZipError. That would discourage usage. It could be deprecated for removal in a future release.
01-10-2024

Moving to Finalized for CSR lead to consider the last responses from me and [~liach]
01-10-2024

> Hmm. I'd be more comfortable if ZipError was deprecated and its sole constructor was deprecated for removal (which would mean making the constructor private). > That would seem to cause less churn to clients dutifully trying to handle the obsolete type. > What do you think? Sorry for letting this linger for so long. While Joe's proposal looks like a reasonable conservative approach, it also feels a bit like a missed opportunity to get rid of this class which was arguably a mistake from the beginning: * It is the sole class in the JDK extending InternalError * As such, it is documented to signal "that an unrecoverable error has occurred", so catching this seems suspicious * Its use has never been documented in any JDK API * Its name is deceptively similar to `ZipException`, the exception class actually used in `java.util.zip` Digging into the implementation in JDK 8 I find: * ZipError is only thrown from ZipEntryIterator::next, use from the public methods ZipFile:entries and ZipFile::stream. See https://github.com/openjdk/jdk8u-dev/blob/f2d72b71880b739d006cd6fadc92018bcc705c15/jdk/src/share/classes/java/util/zip/ZipFile.java#L538 * The error happens on Windows only, when a ZIP file is being overwriten after the ZipFile was constructed. This observation is supported by searching for stack trackes, the only instances found were on Windows. My understanding is that Windows file system / memory mapping semantics are different from Linux in that Linux keeps reading from the old inode, while Windows does not. Also, the OpenJDK test for ZipError only runs on Windows. * The behavior of enumerating ZIP file entries while its backing ZIP is being overwritten does not seem like something user code can reasonably recover from, some form of external file locking should be used to prevent this from happening. Perhaps the above may help sway the CSR lead to support the removal of ZipError. If not, I can go ahead and update the PR/CSR with the proposed removal of the constructor only. Would also be interested in an update from [~lancea] following this last part of the discussion. Thanks!
01-10-2024

I believe it is safe for direct removal instead of simple deprecation. [Per](https://github.com/openjdk/jdk/pull/20642#issuecomment-2308845720) [~lancea]: > The overall usage of ZipError is extremely minimal in the wild at best, from what I can tell was in a catch statement and was not documented to be thrown by any public API, though was thrown by ZipFile's internal ZipEntryIterator::next > If this was a more heavily used Exception, it would probably warrant further consideration for additional documentation, I am not convinced it is needed here The risk of the removal of `ZipError` is probably smaller than that of removal of `sun.misc.Unsafe`.
30-09-2024

Moving to Provisional, not Approved. Hmm. I'd be more comfortable if ZipError was deprecated and its sole constructor was deprecated for removal (which would mean making the constructor private). That would seem to cause less churn to clients dutifully trying to handle the obsolete type. What do you think?
31-08-2024

This CSR has been though the initial round of review and seems ready for consideration by the CSR Lead [~darcy]
30-08-2024

[~lancea] I've updated the Problem, Solution and Compatibility Risk sections to accommodate your feedback. Could you make another pass over the text and see if this takes care of your concerns? Thanks :)
28-08-2024

I would suggest updating the CSR to indicate that ZipError was introduced in JDK 6 via JDK-4615343 and its usage was removed from ZipFile in JDK 9 when the use of native code transitioned to Java via JDK-8142508. The only other use was in the original ZipFS demo which also was removed when ZipFS was integrated into JDK 9 as a supported Module The compatibility risk section should also be updated to indicate that the JDK never documented that this Error was thrown by ZipFile or any other API which also reduces the risk significantly. In addition the corpus search of Maven, showed only a handful of uses. The paragraph "The actual removal of this class..." is not needed I would indicate that an RN will accompany this proposed change
28-08-2024

I have updated the Specification section and moved this to the Proposed state to allow for an initial round of reviews.
28-08-2024

[~liach] > The actual removal of this class may take longer, as there may be Class Files compiled against release 8 catching ZipError, which will fail to run on the version ZipError is removed. We will most likely remove ZipError when release 8 is no longer supported by javac. Can you confirm that you would still like to include this paragraph in the Solution section? The first sentence seems to refer to a now deleted preceding paragraph. The second paragraph seems to commit to plans of the actual removal, which is perhaps a bit premature for this deprecation? Thanks, Eirik :)
28-08-2024

I added the label release-note=yes. (Thanks to Joe for moving it to the parent issue!). I also created a draft release note as a subtask on the main issue, we can review that once the review of the deprecation itself reaches consensus.
27-08-2024

[~liach], anyone can add the release-note=yes label to the parent issue. Without necessarily commenting on the specifics of the deprecation path here, it is reasonable to give some guidance to the users, as long as there is wording to indicate flexibility in the plan changing too.
26-08-2024

Note to reviewer: this RFE certainly needs a release note. Please add the release-note=yes label on the main issue when the CSR is approve.
24-08-2024

[~darcy] Should we include such an excerpt in the `@deprecated` section of the specification, about the steps of deprecation? ``` * In a future release, implementation of {@code ZipError} will be removed before this class itself * is removed; at that point, {@code ZipError} can no longer be constructed or used, but can still * be compiled against as an aid in migration. ```
24-08-2024