Load legacy ZIP64 JARs leniently#365
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return FileSystems.newFileSystem(uri, Collections.emptyMap()); | ||
| } catch (ZipException e) { | ||
| // Rewrite entries through ZipInputStream to discard malformed CEN metadata. | ||
| Path sanitizedArchive = Files.createTempFile("jd-gui-duo-", getExtension(file)); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 194c9a5beb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| return FileSystems.newFileSystem(sanitizedArchive, (ClassLoader)null); |
There was a problem hiding this comment.
When this ZipException fallback is used, the returned zipfs is registered for sanitizedArchive, not for the original uri that load() checks with FileSystems.getFileSystem(uri). Reopening or reloading the same legacy JAR therefore never reuses the existing file system and rewrites another temp archive that is kept open until JVM exit, so repeated opens leak file handles and temp files; cache the fallback by original URI or otherwise make the original lookup hit the sanitized file system.
Useful? React with 👍 / 👎.
194c9a5 to
06d5772
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses ZIP/JAR import failures caused by strict ZIP64/CEN parsing in the JDK ZipFS provider by adding a fallback that rewrites malformed archives into a sanitized temporary ZIP/JAR before opening them as a FileSystem. It also adds a regression test and build plumbing to fetch a known legacy/malformed JAR.
Changes:
- Add a
ZipExceptionfallback inZipFileLoaderProviderthat rewrites entries viaZipInputStream/ZipOutputStreamand opens the sanitized archive with ZipFS. - Add a JUnit test that loads
aspectjweaver-1.8.13.jarand asserts expected JAR structure/container detection. - Update the
servicesMaven module to download the test JAR duringgenerate-test-resourcesand add a test-scoped coordinate reference.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| services/src/main/java/org/jd/gui/service/fileloader/ZipFileLoaderProvider.java | Adds ZipFS fallback that sanitizes malformed ZIP/JARs by rewriting entries into a temporary archive. |
| services/src/test/java/org/jd/gui/service/fileloader/JarFileLoaderProviderTest.java | Adds regression coverage for loading a legacy/malformed JAR and verifying expected structure. |
| services/pom.xml | Adds build step to copy the test JAR into target/test-jars and declares a test dependency coordinate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * This is a Copyleft license that gives the user the right to use, | ||
| * copy and modify the code freely for non-commercial purposes. | ||
| */ |
| import org.apache.commons.io.FilenameUtils; | ||
| import org.jd.core.v1.service.converter.classfiletojavasyntax.util.ExceptionUtil; |
| private String getExtension(File file) { | ||
| String extension = FilenameUtils.getExtension(file.getName()); | ||
| return extension.isEmpty() ? ".zip" : '.' + extension; | ||
| } |
| private FileSystem newFileSystem(URI uri, File file) throws IOException { | ||
| try { | ||
| return FileSystems.newFileSystem(uri, Collections.emptyMap()); | ||
| } catch (ZipException e) { | ||
| // Rewrite entries through ZipInputStream to discard malformed CEN metadata. | ||
| Path sanitizedArchive = Files.createTempFile("jd-gui-duo-", getExtension(file)); | ||
| sanitizedArchive.toFile().deleteOnExit(); | ||
|
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d5772875
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| return FileSystems.newFileSystem(sanitizedArchive, (ClassLoader)null); |
There was a problem hiding this comment.
In the fallback path for a .jar/.zip that is not actually a ZIP, or is corrupt before the first local header, ZipInputStream.getNextEntry() returns null rather than throwing, so the code writes a brand-new empty ZIP and then returns a valid FileSystem for it. That makes load() report success and show an empty archive instead of rejecting the bad input; guard that at least one entry was copied, or rethrow the original ZipException, before opening the sanitized archive.
Useful? React with 👍 / 👎.
Fixes #363