RFR: 8303920: Avoid calling out to python in DataDescriptorSignatureMissing test [v8]

Lance Andersen lancea at openjdk.org
Mon Oct 30 15:05:47 UTC 2023


On Sat, 28 Oct 2023 20:01:56 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> Please review this PR which brings  the DataDescriptorSignatureMissing test back to life.
>> 
>> This test currently calls out to Python to create a test vector ZIP with a Data Descriptor without the recommended but optional signature. The Python dependency has turned out to be very brittle, so the test is currently marked with `@ignore` 
>> 
>> The PR replaces Python callouts with directly creating the test vector ZIP in the test itself. We can then remove the `@ignore`tag and run this useful test automatically.
>
> Eirik Bjorsnos has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge branch 'master' into signature-less-data-descriptor
>    
>    # Conflicts:
>    #	test/jdk/java/util/zip/DataDescriptorSignatureMissing.java
>  - The END header LOC offset field and the second entry's CEN LOC offset fields need to be update to account for the four missing signature bytes.
>  - Merge branch 'master' into signature-less-data-descriptor
>  - Add assertNotNulls to catch unexpectedly missing entries
>  - Revert change to Google copyright line, add an Oracle copyright line instead.
>  - Use "Signatureless" consistently
>  - Remove reference to python in the @summary of DataDescriptorSignatureMissing
>  - Update copyright years
>  - Add method comments
>  - Instead of calling out to python, create a ZIP file and remove the data descriptor signature.

Thanks for trying to move this forward.  Please see my comments below.

Another question, is the zip that is generated by this test readable by other zip tools such as info-zip, Apache Common-compress, winzip?

test/jdk/java/util/zip/DataDescriptorSignatureMissing.java line 3:

> 1: /*
> 2:  * Copyright 2012 Google, Inc.  All Rights Reserved.
> 3:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

I am not sure the copyright can be updated this way @irisclark, could you provide guidance

test/jdk/java/util/zip/DataDescriptorSignatureMissing.java line 34:

> 32:  * without data descriptors was found.
> 33:  * @run testng DataDescriptorSignatureMissing
> 34:  */

Can we convert this please to use junit

test/jdk/java/util/zip/DataDescriptorSignatureMissing.java line 76:

> 74:     /**
> 75:      * Produce a ZIP file where the first entry has a signature-less data descriptor
> 76:      */

I think it would be useful to show the  what the internal zip representation of the LOC and CEN looks like to make it clear what a signature-less data descriptor is meant to be for future maintainers

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

PR Review: https://git.openjdk.org/jdk/pull/12959#pullrequestreview-1704351462
PR Review Comment: https://git.openjdk.org/jdk/pull/12959#discussion_r1376354980
PR Review Comment: https://git.openjdk.org/jdk/pull/12959#discussion_r1376355486
PR Review Comment: https://git.openjdk.org/jdk/pull/12959#discussion_r1376360218


More information about the core-libs-dev mailing list