On Tue, 5 Dec 2023 15:58:14 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
> 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, see #12959. 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` or
> `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.
Here's the relevant section from `APPNOTE.TXT`:
4.4.4 general purpose bit flag: (2 bytes)
[..]
Bit 3: If this bit is set, the fields crc-32, compressed
size and uncompressed size are set to zero in the
local header. The correct values are put in the
data descriptor immediately following the compressed
data.
One could argue we should validate and throw a `ZipException` for such entries,
but that is perhaps a question for another PR.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16975#issuecomment-1847156258
