On Thu, 8 May 2025 12:08:09 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which addresses the issue noted in > https://bugs.openjdk.org/browse/JDK-8354799? > > `java.util.zip.ZipInputStream` when dealing with a `STORED` entry of zero > size was missing a CRC check for that entry. The change in this PR addresses > that and introduces a new jtreg test which reproduces the issue and verifies > the fix. > > tier testing is currently in progress. Hi Jai, I think the changes overall look good. You could simplify the test by creating a zip then converting the bytes to a Hex string/byte array and store it in the test. I mention this as you are searching for a Data Descriptor signature that is optional in the test. If you capture the byte array/Hex String and then write it out as part of the start of the test. The hex offsets from ZipDetails will be accurate. Again, not a big deal either way, but wanted to mention it test/jdk/java/util/zip/ZipInputStream/ZipInputStreamCRCCheck.java line 99: > 97: final ZipEntry entry = zis.getNextEntry(); > 98: assertEquals(entryName, entry.getName(), "unexpected entry > name"); > 99: try { I would consider changing this to something such as we did in open/test/jdk/java/util/zip/ZipInputStream/Zip64DataDescriptor.java: ` ZipException ex = assertThrows(ZipException.class, () -> { readZipInputStream(invalidZip64); }); String msg = String.format("Expected exeption message to contain 'invalid entry size', was %s", ex.getMessage()); assertTrue(ex.getMessage().contains("invalid entry size"), msg); }` ------------- PR Review: https://git.openjdk.org/jdk/pull/25116#pullrequestreview-2825110440 PR Review Comment: https://git.openjdk.org/jdk/pull/25116#discussion_r2079701386
