jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8328995: Launcher can't open jar files where the offset of the manifest is >4GB

Open cushon opened this issue 1 year ago • 13 comments
trafficstars

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

Link to Webrev Comment

cushon avatar Mar 25 '24 21:03 cushon

: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.

bridgekeeper[bot] avatar Mar 25 '24 21:03 bridgekeeper[bot]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Mar 25 '24 21:03 openjdk[bot]

@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.

openjdk[bot] avatar Mar 25 '24 21:03 openjdk[bot]

/reviewers 2

AlanBateman avatar Mar 26 '24 22:03 AlanBateman

@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).

openjdk[bot] avatar Mar 26 '24 22:03 openjdk[bot]

@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[bot] avatar May 02 '24 17:05 bridgekeeper[bot]

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

cushon avatar May 02 '24 17:05 cushon

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.

AlanBateman avatar May 02 '24 17:05 AlanBateman

@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[bot] avatar May 30 '24 20:05 bridgekeeper[bot]

@bridgekeeper please keep this open

cushon avatar May 30 '24 20:05 cushon

@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[bot] avatar Jul 02 '24 21:07 bridgekeeper[bot]

Hello Liam @cushon, please keep this open. Some of us have this on our list to review.

jaikiran avatar Jul 03 '24 10:07 jaikiran

bridgekeeper please keep open

cushon avatar Jul 08 '24 17:07 cushon

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.

cushon avatar Jul 18 '24 21:07 cushon

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.

jaikiran avatar Jul 29 '24 07:07 jaikiran

@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[bot] avatar Aug 26 '24 12:08 bridgekeeper[bot]

bridgekeeper please keep this open for now

cushon avatar Aug 26 '24 15:08 cushon

@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[bot] avatar Sep 23 '24 16:09 bridgekeeper[bot]

@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.

bridgekeeper[bot] avatar Oct 21 '24 19:10 bridgekeeper[bot]