[9] request for review: 8049171: Additional tests for jarsigner's warnings

Artem Smotrakov artem.smotrakov at oracle.com
Thu Jan 22 11:40:19 UTC 2015


Thanks, Max.

Please see inline.

On 01/21/2015 11:29 AM, Wang Weijun wrote:
> Thanks for adding so many tests. Some suggestions:
>
> - JarUtils.java
>
> You can use the new InputStream.transferTo() method.
Sure, I didn't know about that, thanks.
>
> I am not sure if I understand updateJar correctly. It looks like srcJarFile is opened multiple times so its entries are duplicated a lot in the destination. Or is there a secret break?
There is no any secret, just a bug. It is not necessary to open 
srcJarFile multiple times.

I have updated the webrev, updateJar() method does the following:
- creates a new jar file (destJarFilename)
- puts files which are specified in files parameter to destJarFilename
- copies files from srcJarFilename to destJarFilename if they are no 
files with the same names in destJarFilename
>
> - Utils.java
>
> The mixed using of File and Files is strange, but you are free to keep it.
Agree. I removed this method since tests don't need it anymore after I 
removed cleanup() methods (please see below).
>
> - TimestampCheck.java
>
> You can make Handler Autocloseable to use try-with-resources in tests.
Good idea.
>
> - Various tests
>
> I am not a fan of calling Utils.cleanup() in final block. Unless you have created huge garbages, those files are precious when the test fails (given you provide -retain in jtreg, which I always do).
Agree, those files may be helpful, they are not huge. I have removed a 
cleanup() methods.

Here is an updated webrev with your suggested changes and a couple of 
others:
- added @ignore tag for BadKeyUsageTest since it fails due to a bug in JDK
- updated MultipleWarningsTest test to check ExtendedKeyUsage case 
instead of KeyUsage

http://cr.openjdk.java.net/~asmotrak/8049171/webrev.01/

Artem

> Thanks
> Max
>
>> On Jan 21, 2015, at 14:35, Artem Smotrakov <artem.smotrakov at oracle.com> wrote:
>>
>> Hello,
>>
>> Please review a couple of new tests for jarsigner's warnings. Basically tests run jarsigner and check warning/error messages and exit codes according to [1].
>>
>> https://bugs.openjdk.java.net/browse/JDK-8049171
>> http://cr.openjdk.java.net/~asmotrak/8049171/webrev.00
>>
>> [1] http://docs.oracle.com/javase/7/docs/technotes/tools/windows/jarsigner.html
>>
>> Artem
>>
>>



More information about the security-dev mailing list