Please review this PR which suggests we retire the ZIP test 
`NoExtensionSignature` along with its `test.jar` test vector. 

The concern of a missing data descriptor signature is covered by the recently 
updated  `DataDescriptorSignatureMissing` test. That test is more complete, 
includes more documentation and uses a programmatically generated test vector.

One might argue that 'more tests are always better', but in this case I think 
the 21 year old `NoExtensionSignature` test with its binary test vector is 
nebulous and requires extensive analysis to understand, more so to update. I 
think it does not carry its weight and should be retired.

Careful analysis of the deleted `test.jar` test vector revealed that it 
contains a local header with non-zero `compressed size` and `uncompressed size` 
fields for a streaming-mode entry. `APPNOTE.TXT` mandates that when bit 3 of 
the general purpose bit flag is set, then these fields and the `crc` field 
should all be set to zero. 

By injecting assertions into `ZipInputStream.readLOC` I was able to determine 
that `NoExtensionSignature` is the only test currently parsing a ZIP file with 
such non-zero fields in streaming mode. 

Because of this, and out of caution, this PR introduces a new test 
`DataDescriptorIgnoreCrcAndSizeFields` which  explicitly verifies that 
`ZipInputStream` does not read any non-zero `crc`, `compressed size` and 
`uncompressed size` field values from a local header when in streaming mode. 
`ZipInputStream` should ignore these values and it would be good to have a test 
which asserts that this invariant holds even when the fields are non-zero.

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

Commit messages:
 - Rename 'nameAndContent' parameter to 'expected'
 - Retire the test NoExtensionSignature in favor of 
DataDescriptorSignatureMissing. Introduce the new test 
DataDescriptorIgnoreCrcAndSizeFields covering unrelated aspects implicitly 
tested by NoExtensionSignature.

Changes: https://git.openjdk.org/jdk/pull/16975/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16975&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8321396
  Stats: 222 lines in 3 files changed: 180 ins; 42 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/16975.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16975/head:pull/16975

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

Reply via email to