RFR: 8303972: Make test/jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java independent of the zip command line [v5]

Lance Andersen lance.andersen at oracle.com
Fri Mar 17 19:59:57 UTC 2023



On Mar 17, 2023, at 3:32 PM, Eirik Bjorsnos <duke at openjdk.org<mailto:duke at openjdk.org>> wrote:

On Fri, 17 Mar 2023 18:36:02 GMT, Eirik Bjorsnos <duke at openjdk.org<mailto:duke at openjdk.org>> wrote:

Please review this PR which brings the currenly problem-listed test TestLocOffsetFromZip64EF back to life.

Instead of calling out to the `zip` command line, we produce a small-sized Zip64 file in the test itself. This file has the features  required to reproduce the ZipFileSystem issue, namely that the 'INFO-ZIP extended timestamp' field must come before  the 'Zip64 extended information'  field. (This would casue ZipFileSystem to read the LOC at position 0xFFFFFFFF).

This speed up the test (from 50s to 3s on my Macbook Pro), saves 4GB disk space during builds removes a dependency on the `zip` command in OS/distros.

Seee [JDK-8301183](https://bugs.openjdk.org/browse/JDK-8301183) for details on the problem-listing.

Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision:

 Add extra entry as a sanity check. Extract some constants for sizes and extract method makeOpaqueExtraField to allow some more in-depth commenting. Move extra field processing until after the LOC headers are written, such that this information only occurs in the CEN

Something worth mentioning:

This test  currently does not check that the last modified time stamp is actually the one read from the LOC and not from the CEN. And there is a discrepancey here, `ZipFileSystem` reads it from the LOC, while `ZipFile` reads it from the CEN.

We could test this by setting different last modified timestamps in the LOC and CEN and use assertEquals to verify that the expected time stamp is parsed.

@LanceAndersen Do you think this is worth pursuing in this PR? I have a sketch ready, but I fear will clutter the test with too many concerns. Perhaps better to test in a separate test which could be independent from Zip64 and local header offset concerns?

Hi Eirik,

Please treat the timestamp issue above separately so we are not morphing issues.


Best
Lance

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

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

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC at home]






Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com<mailto:Lance.Andersen at oracle.com>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/nio-dev/attachments/20230317/90f48398/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: oracle_sig_logo.gif
URL: <https://mail.openjdk.org/pipermail/nio-dev/attachments/20230317/90f48398/oracle_sig_logo.gif>


More information about the nio-dev mailing list