jdk
jdk copied to clipboard
8328995: Launcher can't open jar files where the offset of the manifest is >4GB
This change fixes a zip64 bug in the launcher that is prevent it from reading the manifest of jars where the 'relative offset of local header' field in the central directory entry is >4GB. As described in APPNOTE.TXT 4.5.3, the offset is too large to be stored in the central directory it is stored in a 'Zip64 Extended Information Extra Field'.
Progress
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)
Issue
- JDK-8328995: Launcher can't open jar files where the offset of the manifest is >4GB (Bug - P3)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18479/head:pull/18479
$ git checkout pull/18479
Update a local copy of the PR:
$ git checkout pull/18479
$ git pull https://git.openjdk.org/jdk.git pull/18479/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18479
View PR using the GUI difftool:
$ git pr show -t 18479
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18479.diff
Webrev
:wave: Welcome back cushon! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
@cushon The following label will be automatically applied to this pull request:
core-libs
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 08: Full - Incremental (18661092)
- 07: Full - Incremental (172609a3)
- 06: Full - Incremental (da36c059)
- 05: Full - Incremental (140e13b9)
- 04: Full - Incremental (c2e16e12)
- 03: Full - Incremental (d475de76)
- 02: Full - Incremental (7b599c9c)
- 01: Full - Incremental (6b13ecc2)
- 00: Full (8f908a0e)
/reviewers 2
@AlanBateman The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).
@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Please keep this open, assistance on progressing this pull request is welcome
Please keep this open, assistance on progressing this pull request is welcome
ACK. There was a lengthy bug tail when zip64 support was added. We've got away without needing this for executable JAR files for a long time so it's great to see effort on it. I think it will take time to review closely.
@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@bridgekeeper please keep this open
@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Hello Liam @cushon, please keep this open. Some of us have this on our list to review.
bridgekeeper please keep open
For what it's worth I have been testing the current version of the PR at Google, and have verified it allows jars with >4GB offsets to be launched, and we haven't observed any regressions using it.
Hello Liam,
At that point there were three including pack200, and now we're down to two, but consolidating zip_util.c and parse_manifest.c still seems like a big job.
In a different context, I was experimenting with some changes in the launcher code (one part of which involves this parsing of the manifest). While at it, we realized that some of the changes I am looking into will allow us to move away from having to parse the manifest of the jar in this code path. What that then means is that we won't have to do any of this jar parsing within the C code of the launcher and can rely on existing java code to do the necessary jar parsing. That should then allow us to support zip64 jars without this added complexity and maintenance in the C code. I have some prototype change which I'm experimenting with. Please give me some more days so that I can bring it to a state where it can be reviewed. As such, I request you to put on hold, for a few more days, the changes that are being proposed in this current PR.
@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
bridgekeeper please keep this open for now
@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@cushon This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.