Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v3]
On Thu, 1 Oct 2020 14:42:21 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for >> https://bugs.openjdk.java.net/browse/JDK-8242882? >> >> As noted in that JBS issue, if the size of the Manifest entry in the jar >> happens to be very large (such that it exceeds >> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >> lead to a `NegativeArraySizeException`. This >> is due to the: if (len != -1 && len <= 65535) block which evaluates to >> `true` when the size of the manifest entry is >> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >> code which can lead to the >> `NegativeArraySizeException`. The commit in this PR fixes that issue by >> changing those `if/else` blocks to prevent >> this issue and instead use a code path that leads to the >> `InputStream#readAllBytes()` which internally has the >> necessary checks to throw the expected `OutOfMemoryError`. This commit also >> includes a jtreg test case which >> reproduces the issue and verifies the fix. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Second round of review comments addressed Hi Jaikiran, Yes I think you are OK to move forward with the integration, - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Wed, 7 Oct 2020 21:40:43 GMT, Brent Christian wrote: >> I decided to slightly change the way this large manifest file was being >> created. I borrowed the idea from >> `Zip64SizeTest`[1] to create the file and set its length to a large value. I >> hope that is OK. If not, let me know, I >> will change this part. [1] >> https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/zip/ZipFile/Zip64SizeTest.java#L121 > > I did some automated test runs, and the duration of this test is sufficiently > improved, IMO. > > While not representative of a real MANIFEST.MF file, I think it works well > enough for this specific test. Thank you for the review Brent. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v3]
On Wed, 7 Oct 2020 21:40:57 GMT, Brent Christian wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Second round of review comments addressed > > Marked as reviewed by bchristi (Reviewer). Hello Lance, does the latest state of this PR look fine to you? If so, shall I trigger a integrate? - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Thu, 1 Oct 2020 14:39:50 GMT, Jaikiran Pai wrote: >> test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 78: >> >>> 76: bw.write("OOM-Test: "); >>> 77: for (long i = 0; i < 2147483648L; i++) { >>> 78: bw.write("a"); >> >> As you probably noticed, this test takes a little while to run. One way to >> speed it up a little would be to write more >> characters at a time. While we're at it, we may as well make the Manifest >> well-formed by breaking it into 72-byte >> lines. See "Line length" under: >> https://docs.oracle.com/en/java/javase/15/docs/specs/jar/jar.html#notes-on-manifest-and-signature-files >> Just write >> enough lines to exceed Integer.MAX_VALUE bytes. > > I decided to slightly change the way this large manifest file was being > created. I borrowed the idea from > `Zip64SizeTest`[1] to create the file and set its length to a large value. I > hope that is OK. If not, let me know, I > will change this part. [1] > https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/zip/ZipFile/Zip64SizeTest.java#L121 I did some automated test runs, and the duration of this test is sufficiently improved, IMO. While not representative of a real MANIFEST.MF file, I think it works well enough for this specific test. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v3]
On Thu, 1 Oct 2020 14:42:21 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for >> https://bugs.openjdk.java.net/browse/JDK-8242882? >> >> As noted in that JBS issue, if the size of the Manifest entry in the jar >> happens to be very large (such that it exceeds >> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >> lead to a `NegativeArraySizeException`. This >> is due to the: if (len != -1 && len <= 65535) block which evaluates to >> `true` when the size of the manifest entry is >> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >> code which can lead to the >> `NegativeArraySizeException`. The commit in this PR fixes that issue by >> changing those `if/else` blocks to prevent >> this issue and instead use a code path that leads to the >> `InputStream#readAllBytes()` which internally has the >> necessary checks to throw the expected `OutOfMemoryError`. This commit also >> includes a jtreg test case which >> reproduces the issue and verifies the fix. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Second round of review comments addressed Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Wed, 30 Sep 2020 17:21:14 GMT, Brent Christian wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address the review comments and introduce an array size check in >> JarFile.getBytes() method itself > > src/java.base/share/classes/java/util/jar/JarFile.java line 161: > >> 159: * OutOfMemoryError: Required array size too large >> 160: */ >> 161: private static final int MAX_BUFFER_SIZE = Integer.MAX_VALUE - 8; > > "/**" comments are generally only used for public documentation. For use > here, probably a single line // comment would > be sufficient to explain what this value is. > This constant is also named, "MAX_ARRAY_SIZE" in various places, which seems > more applicable to this case. Hello Brent, I've updated the PR with your suggested changes for this member variable name and the comment. > src/java.base/share/classes/java/util/jar/JarFile.java line 801: > >> 799: if (len > MAX_BUFFER_SIZE) { >> 800: throw new OutOfMemoryError("Required array size too >> large"); >> 801: } > > I would just add a new `long zeSize` to read and check `ze.getSize()`, and > then (int) cast it into `len`, as before. > Then I think no changes would be needed past L802, `int bytesRead;` Done. Changed it based on your input. > test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 78: > >> 76: bw.write("OOM-Test: "); >> 77: for (long i = 0; i < 2147483648L; i++) { >> 78: bw.write("a"); > > As you probably noticed, this test takes a little while to run. One way to > speed it up a little would be to write more > characters at a time. While we're at it, we may as well make the Manifest > well-formed by breaking it into 72-byte > lines. See "Line length" under: > https://docs.oracle.com/en/java/javase/15/docs/specs/jar/jar.html#notes-on-manifest-and-signature-files > Just write > enough lines to exceed Integer.MAX_VALUE bytes. I decided to slightly change the way this large manifest file was being created. I borrowed the idea from `Zip64SizeTest`[1] to create the file and set its length to a large value. I hope that is OK. If not, let me know, I will change this part. [1] https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/zip/ZipFile/Zip64SizeTest.java#L121 - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v3]
> Can I please get a review and a sponsor for a fix for > https://bugs.openjdk.java.net/browse/JDK-8242882? > > As noted in that JBS issue, if the size of the Manifest entry in the jar > happens to be very large (such that it exceeds > the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can > lead to a `NegativeArraySizeException`. This > is due to the: if (len != -1 && len <= 65535) block which evaluates to > `true` when the size of the manifest entry is > larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the > code which can lead to the > `NegativeArraySizeException`. The commit in this PR fixes that issue by > changing those `if/else` blocks to prevent > this issue and instead use a code path that leads to the > `InputStream#readAllBytes()` which internally has the > necessary checks to throw the expected `OutOfMemoryError`. This commit also > includes a jtreg test case which > reproduces the issue and verifies the fix. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Second round of review comments addressed - Changes: - all: https://git.openjdk.java.net/jdk/pull/323/files - new: https://git.openjdk.java.net/jdk/pull/323/files/279c7c83..a011b0d6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=323=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=323=01-02 Stats: 34 lines in 2 files changed: 5 ins; 15 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/323.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/323/head:pull/323 PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v3]
On Wed, 30 Sep 2020 18:38:40 GMT, Lance Andersen wrote: >> I think it's fine either way. > > If you are going to validate the message, which I probably would not, it > would be important to make sure it document > if the message is changed in JarFile::getBytes, that the test needs updated. > Otherwise it will be easy to miss. Hello Lance, I decided to remove the assertion on the exception message. I have updated the PR accordingly. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Wed, 30 Sep 2020 17:26:18 GMT, Brent Christian wrote: >> test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 60: >> >>> 58: final OutOfMemoryError oome = >>> Assert.expectThrows(OutOfMemoryError.class, () -> jar.getManifest()); >>> 59: // additionally verify that the OOM was for the right/expected >>> reason >>> 60: if (!"Required array size too large".equals(oome.getMessage())) >>> { >> >> I wasn't too sure if I should add this additional check on the message of >> the `OutOfMemoryError`, but decided to do it >> anyway, given that from what I remember there were some discussions in the >> `core-libs-dev` list a while back on the >> exact messages that such OOMs should throw. So I guessed that it might be OK >> to rely on those messages in the tests >> within this project. However, I am open to removing specific check if it's >> considered unnecessary. > > I think it's fine either way. If you are going to validate the message, which I probably would not, it would be important to make sure it document if the message is changed in JarFile::getBytes, that the test needs updated. Otherwise it will be easy to miss. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Wed, 23 Sep 2020 15:09:44 GMT, Jaikiran Pai wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address the review comments and introduce an array size check in >> JarFile.getBytes() method itself > > test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 60: > >> 58: final OutOfMemoryError oome = >> Assert.expectThrows(OutOfMemoryError.class, () -> jar.getManifest()); >> 59: // additionally verify that the OOM was for the right/expected >> reason >> 60: if (!"Required array size too large".equals(oome.getMessage())) { > > I wasn't too sure if I should add this additional check on the message of the > `OutOfMemoryError`, but decided to do it > anyway, given that from what I remember there were some discussions in the > `core-libs-dev` list a while back on the > exact messages that such OOMs should throw. So I guessed that it might be OK > to rely on those messages in the tests > within this project. However, I am open to removing specific check if it's > considered unnecessary. I think it's fine either way. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Tue, 29 Sep 2020 11:39:20 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for >> https://bugs.openjdk.java.net/browse/JDK-8242882? >> >> As noted in that JBS issue, if the size of the Manifest entry in the jar >> happens to be very large (such that it exceeds >> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >> lead to a `NegativeArraySizeException`. This >> is due to the: if (len != -1 && len <= 65535) block which evaluates to >> `true` when the size of the manifest entry is >> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >> code which can lead to the >> `NegativeArraySizeException`. The commit in this PR fixes that issue by >> changing those `if/else` blocks to prevent >> this issue and instead use a code path that leads to the >> `InputStream#readAllBytes()` which internally has the >> necessary checks to throw the expected `OutOfMemoryError`. This commit also >> includes a jtreg test case which >> reproduces the issue and verifies the fix. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Address the review comments and introduce an array size check in > JarFile.getBytes() method itself If there is only modest improvement in test duration, we may want to add a jtreg timeout tag. Also, given the long duration but relative low priority of testing this, it perhaps should be moved out of Tier 1. I will look into those things after your next update. src/java.base/share/classes/java/util/jar/JarFile.java line 161: > 159: * OutOfMemoryError: Required array size too large > 160: */ > 161: private static final int MAX_BUFFER_SIZE = Integer.MAX_VALUE - 8; "/**" comments are generally only used for public documentation. For use here, probably a single line // comment would be sufficient to explain what this value is. This constant is also named, "MAX_ARRAY_SIZE" in various places, which seems more applicable to this case. src/java.base/share/classes/java/util/jar/JarFile.java line 801: > 799: if (len > MAX_BUFFER_SIZE) { > 800: throw new OutOfMemoryError("Required array size too > large"); > 801: } I would just add a new `long zeSize` to read and check `ze.getSize()`, and then (int) cast it into `len`, as before. Then I think no changes would be needed past L802, `int bytesRead;` test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 45: > 43: public class LargeManifestOOMTest { > 44: > 45: Unneeded blank lines test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 83: > 81: } > 82: } > 83: Extra line test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 78: > 76: bw.write("OOM-Test: "); > 77: for (long i = 0; i < 2147483648L; i++) { > 78: bw.write("a"); As you probably noticed, this test takes a little while to run. One way to speed it up a little would be to write more characters at a time. While we're at it, we may as well make the Manifest well-formed by breaking it into 72-byte lines. See "Line length" under: https://docs.oracle.com/en/java/javase/15/docs/specs/jar/jar.html#notes-on-manifest-and-signature-files Just write enough lines to exceed Integer.MAX_VALUE bytes. - Changes requested by bchristi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
On Thu, 24 Sep 2020 16:36:28 GMT, Brent Christian wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address the review comments and introduce an array size check in >> JarFile.getBytes() method itself > > src/java.base/share/classes/java/util/jar/JarFile.java line 791: > >> 789: private byte[] getBytes(ZipEntry ze) throws IOException { >> 790: try (InputStream is = super.getInputStream(ze)) { >> 791: int len = (int)ze.getSize(); > > I think the main issue is the casting of the 'long' value from > ZipEntry.getSize() into an 'int'. I think checking if > the size is > the maximum array size and throwing an OOME here might be a > sufficient fix on its own. Hello Brent, Thank you for the review and sorry about the delayed response - I was involved in a few other things so couldn't get to this sooner. I have taken your input and updated this patch to address this part. Specifically, I have introduced a new `MAX_BUFFER_SIZE` within the `JarFile`. This `MAX_BUFFER_SIZE` is an actual copy of the field (and value) of `java.io.InputStream#MAX_BUFFER_SIZE`. I have done a minor change to the javadoc of this field as compared to what is in the javadoc of its `InputStream` counterpart. I did this so that the OOM exception message being thrown matches the comment in this javadoc (the `InputStream` has a mismatch in its javadoc and the actual message that gets thrown). Additionally, in this patch, the `if (len != -1 ...)` has been changed to `if (len >= 0 ...)` to prevent the code from entering this block when `Zip64` format is involved where (from what I can gather) an uncompressed entry size value can have 2^64 (unsigned) bytes. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]
> Can I please get a review and a sponsor for a fix for > https://bugs.openjdk.java.net/browse/JDK-8242882? > > As noted in that JBS issue, if the size of the Manifest entry in the jar > happens to be very large (such that it exceeds > the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can > lead to a `NegativeArraySizeException`. This > is due to the: if (len != -1 && len <= 65535) block which evaluates to > `true` when the size of the manifest entry is > larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the > code which can lead to the > `NegativeArraySizeException`. The commit in this PR fixes that issue by > changing those `if/else` blocks to prevent > this issue and instead use a code path that leads to the > `InputStream#readAllBytes()` which internally has the > necessary checks to throw the expected `OutOfMemoryError`. This commit also > includes a jtreg test case which > reproduces the issue and verifies the fix. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Address the review comments and introduce an array size check in JarFile.getBytes() method itself - Changes: - all: https://git.openjdk.java.net/jdk/pull/323/files - new: https://git.openjdk.java.net/jdk/pull/323/files/76dcea76..279c7c83 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=323=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=323=00-01 Stats: 17 lines in 1 file changed: 10 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/323.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/323/head:pull/323 PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
On Wed, 23 Sep 2020 15:06:55 GMT, Jaikiran Pai wrote: > Can I please get a review and a sponsor for a fix for > https://bugs.openjdk.java.net/browse/JDK-8242882? > > As noted in that JBS issue, if the size of the Manifest entry in the jar > happens to be very large (such that it exceeds > the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can > lead to a `NegativeArraySizeException`. This > is due to the: if (len != -1 && len <= 65535) block which evaluates to > `true` when the size of the manifest entry is > larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the > code which can lead to the > `NegativeArraySizeException`. The commit in this PR fixes that issue by > changing those `if/else` blocks to prevent > this issue and instead use a code path that leads to the > `InputStream#readAllBytes()` which internally has the > necessary checks to throw the expected `OutOfMemoryError`. This commit also > includes a jtreg test case which > reproduces the issue and verifies the fix. Changes requested by bchristi (Reviewer). src/java.base/share/classes/java/util/jar/JarFile.java line 791: > 789: private byte[] getBytes(ZipEntry ze) throws IOException { > 790: try (InputStream is = super.getInputStream(ze)) { > 791: int len = (int)ze.getSize(); I think the main issue is the casting of the 'long' value from ZipEntry.getSize() into an 'int'. I think checking if the size is > the maximum array size and throwing an OOME here might be a sufficient fix on its own. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
Hello Brent, Thank you for sponsoring this change. In the meantime, I triggered the pre-submit GitHub action job to run the "tier1" tests for a duplicate branch of this PR. That completed successfully https://github.com/jaikiran/jdk/actions/runs/269960940. I'll wait for the reviews, before initiating any /integrate command. -Jaikiran On 23/09/20 10:21 pm, Brent Christian wrote: > On Wed, 23 Sep 2020 15:12:58 GMT, Jaikiran Pai wrote: > >>> Can I please get a review and a sponsor for a fix for >>> https://bugs.openjdk.java.net/browse/JDK-8242882? >>> >>> As noted in that JBS issue, if the size of the Manifest entry in the jar >>> happens to be very large (such that it exceeds >>> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >>> lead to a `NegativeArraySizeException`. This >>> is due to the: if (len != -1 && len <= 65535) block which evaluates to >>> `true` when the size of the manifest entry is >>> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >>> code which can lead to the >>> `NegativeArraySizeException`. The commit in this PR fixes that issue by >>> changing those `if/else` blocks to prevent >>> this issue and instead use a code path that leads to the >>> `InputStream#readAllBytes()` which internally has the >>> necessary checks to throw the expected `OutOfMemoryError`. This commit >>> also includes a jtreg test case which >>> reproduces the issue and verifies the fix. >> I had created a copy of this branch in my personal fork and included the >> `submit.yml` workflow as noted in this recent >> mail here[1] to try and run the `tier1` testing for this change. But it >> looks like that has failed for unrelated >> reasons[2]. So I'll go ahead and trigger the `tier1` tests locally instead. >> [1] >> https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004736.html >> [2] >> https://github.com/jaikiran/jdk/actions/runs/268948812 > Hi, Jaikiran. I can sponsor this change. > > - > > PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
On Wed, 23 Sep 2020 15:12:58 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for >> https://bugs.openjdk.java.net/browse/JDK-8242882? >> >> As noted in that JBS issue, if the size of the Manifest entry in the jar >> happens to be very large (such that it exceeds >> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >> lead to a `NegativeArraySizeException`. This >> is due to the: if (len != -1 && len <= 65535) block which evaluates to >> `true` when the size of the manifest entry is >> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >> code which can lead to the >> `NegativeArraySizeException`. The commit in this PR fixes that issue by >> changing those `if/else` blocks to prevent >> this issue and instead use a code path that leads to the >> `InputStream#readAllBytes()` which internally has the >> necessary checks to throw the expected `OutOfMemoryError`. This commit also >> includes a jtreg test case which >> reproduces the issue and verifies the fix. > > I had created a copy of this branch in my personal fork and included the > `submit.yml` workflow as noted in this recent > mail here[1] to try and run the `tier1` testing for this change. But it looks > like that has failed for unrelated > reasons[2]. So I'll go ahead and trigger the `tier1` tests locally instead. > [1] > https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004736.html [2] > https://github.com/jaikiran/jdk/actions/runs/268948812 Hi, Jaikiran. I can sponsor this change. - PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
On Wed, 23 Sep 2020 15:06:55 GMT, Jaikiran Pai wrote: > Can I please get a review and a sponsor for a fix for > https://bugs.openjdk.java.net/browse/JDK-8242882? > > As noted in that JBS issue, if the size of the Manifest entry in the jar > happens to be very large (such that it exceeds > the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can > lead to a `NegativeArraySizeException`. This > is due to the: if (len != -1 && len <= 65535) block which evaluates to > `true` when the size of the manifest entry is > larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the > code which can lead to the > `NegativeArraySizeException`. The commit in this PR fixes that issue by > changing those `if/else` blocks to prevent > this issue and instead use a code path that leads to the > `InputStream#readAllBytes()` which internally has the > necessary checks to throw the expected `OutOfMemoryError`. This commit also > includes a jtreg test case which > reproduces the issue and verifies the fix. I had created a copy of this branch in my personal fork and included the `submit.yml` workflow as noted in this recent mail here[1] to try and run the `tier1` testing for this change. But it looks like that has failed for unrelated reasons[2]. So I'll go ahead and trigger the `tier1` tests locally instead. [1] https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004736.html [2] https://github.com/jaikiran/jdk/actions/runs/268948812 test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 60: > 58: final OutOfMemoryError oome = > Assert.expectThrows(OutOfMemoryError.class, () -> jar.getManifest()); > 59: // additionally verify that the OOM was for the right/expected > reason > 60: if (!"Required array size too large".equals(oome.getMessage())) { I wasn't too sure if I should add this additional check on the message of the `OutOfMemoryError`, but decided to do it anyway, given that from what I remember there were some discussions in the `core-libs-dev` list a while back on the exact messages that such OOMs should throw. So I guessed that it might be OK to rely on those messages in the tests within this project. However, I am open to removing specific check if it's considered unnecessary. - PR: https://git.openjdk.java.net/jdk/pull/323