On Sun, 6 Oct 2024 16:42:15 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this PR which suggests we clean up iteration and validation > logic in `ZipFile.Source::initCEN` and some related methods to use a simpler > and more consistent style: > > * The main loop in `ZipFile.Source::initCEN` currently updates two iteration > variables (`pos` and `entryPos`), where `entryPos` is always just `pos + > CENHDR`. One variable should be sufficient. `entryPos` can be moved to an > effectively final local variable scoped inside the loop. > * The local variable `int limit = cen.length` no longer carries its weight > and is inlined into its only use site > * Since `entryPos` is no longer in scope for the loop predicate, this is > updated to `pos <= cen.length - CENHDR`, instead of the current `entryPos <= > limit` > * The byte array passed to `countCENHeaders` contains only CEN data now, so > the `size` parameter can be removed. > * `countCENHeaders` is updated to loop on the same predicate as `initCEN`. > Additionally, this method is updated to throw early if a CEN entry exceeds > the CEN size (for consistency with similar logic in `checkAndAddEntry`) > * The `headerSize` validation in `checkAndAddEntry` is updated to avoid > widening conversion to `long` of a variable which can provably never exceed > `Integer.MAX_VALUE` and to be consistent with `countCENHeaders`. > > Testing: > > A new test `CenSizeMaximum` is added: > * This produces a ZIP file with a CEN size of exactly `MAX_CEN_SIZE` and > verifies that the iteration logic handles ZIP files where the CEN size is on > or near the limit. > * This test also manipulates the END headers 'total number of entries' > field, in order to exercise `countCENHeader`. > * The test is a bit of a resource hog: It produces a ~1.5GB ZIP file on > disk, requires > 2GB heap and takes > 15 seconds to run on my laptop. Let me > know if this should be made a manual test. > > GHA tests are currently pending. I have run the following tests locally: > > > % make test TEST="test/jdk/java/util/zip" > % make test TEST="test/jdk/java/util/jar" This pull request has now been integrated. Changeset: f7bb647d Author: Eirik Bjørsnøs <eir...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/f7bb647dc88f835fe819e7ab0434c931f243304a Stats: 265 lines in 2 files changed: 252 ins; 4 del; 9 mod 8341595: Clean up iteration of CEN headers in ZipFile.Source.initCEN Reviewed-by: lancea, redestad ------------- PR: https://git.openjdk.org/jdk/pull/21378
