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"

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

Commit messages:
 - Run CenSizeMaximum with /othervm -Xmx2500M
 - Inline the only use of the `limit` local variable
 - Clean up iteration of CEN headers in ZipFile.Source.initCEN

Changes: https://git.openjdk.org/jdk/pull/21378/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21378&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8341595
  Stats: 264 lines in 2 files changed: 251 ins; 4 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/21378.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/21378/head:pull/21378

PR: https://git.openjdk.org/jdk/pull/21378

Reply via email to