On Wed, 25 Sep 2024 16:22:55 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:

>> Please review this cleanup PR which makes `ZipFile.Source.initCEN` not 
>> include the 22-byte trailing`END` header when reading the `CEN` section of 
>> the ZIP file.
>> 
>> The reading of the END header was probably brought over from native code 
>> with the transition to Java in JDK 9.
>> 
>> In the current JDK, the END header is unused. This needlessly complicates 
>> multiple code paths accessing the array since they must account for the 
>> trailing END record when calculating the end of CEN position.
>> 
>> Additionally, the enforcement of the maximum CEN size limit is currently off 
>> by one. It allows the construction of a byte array of size 
>> `Integer.MAX_VALUE - 1`, but this size is not supported by OpenJDK. Instead, 
>> the maximum CEN limit should be such that is does not exceed  
>> `Integer.MAX_VALUE - 2`.
>> 
>> Testing:
>> 
>> The `EndOfCenValidation` test is updated to test the rejection of a CEN of 
>> size `Integer.MAX_VALUE - 1` as the new minumum rejected CEN size.
>> 
>> The `ZipFileOpen` benchmark seems neutral to this change.
>
> Eirik Bjørsnøs has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into zipfile-endhdr
>  - Merge branch 'master' into zipfile-endhdr
>  - Do not include the END header when reading the CEN section

Overall, I think the changes are OK.   Please give it until Monday before 
integrating

-------------

Marked as reviewed by lancea (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20905#pullrequestreview-2334154820

Reply via email to