Re: RFR 8075618: Create tests to check jarsigner work with multi-version jar

[Wang Weijun]

Hi Amanda

Everything is fine. I have no other comment.

Thanks
Max

> On Dec 19, 2016, at 2:43 PM, Amanda Jiang  wrote:
> 
> 
> 
> On 12/18/16 10:23 PM, Wang Weijun wrote:
>>> Please see updated webrev : 
>>> http://cr.openjdk.java.net/~amjiang/8075618/webrev.04/ (also attached)
>>> 
>>> I moved the checkPermission() calls into v9/bersion/Version.java, but I did 
>>> not check if the output contains "I'm running on version 9", I think it 
>>> does not make sense to make test fail with later or earlier version of Java 
>>> (10 or 8).
>> This test will run in JDK 10 and your checkPermission(perm,bool) method will 
>> not get called. Suppose a regression is introduced one day that breaks the 
>> access control checking, this test will not be able to catch it.
>> 
>> If you check for the "version 9" string, the test will fail which reminds 
>> you to move the checkPermission call to Version.java of JDK 10 and it will 
>> catch that regression.
>> 
>> Thanks
>> Max
> Version check added to the test 
> :http://cr.openjdk.java.net/~amjiang/8075618/webrev.05
> Thanks,
> Amanda
> 

Re: RFR 8075618: Create tests to check jarsigner work with multi-version jar

[Wang Weijun]

> On Dec 19, 2016, at 5:38 AM, Amanda Jiang  wrote:
> 
> Hi Paul, Artem and Max,
> 
> Thanks for your comments.  Please check the new webrev at: 
> http://cr.openjdk.java.net/~amjiang/8075618/webrev.03/  (It looks like 
> "cr.openjdk.java.net" is down, I also attach the webrev with this email).
> 
> In the new webrev, I fixed some syntax issue mentioned by Paul and also 
> simplied the test codes with Artem and Max's suggestions.  Unfortunately I 
> cannot fully apply some functions from 
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/d4d7f1f0d688/test/lib/security/SecurityTools.java
>  . Those functions returns deprecated "OutputAnalyzer " class in 
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/ab164f8b8569/test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
>  ,  which conflicts with the "OubputAnalyzer" class I imported for my test ( 
> http://hg.openjdk.java.net/jdk9/dev/file/5e79c9bac1b5/test/lib/jdk/test/lib/process/OutputAnalyzer.java)

This is a pity. We should move it to the root repo before more people start 
using it.

> 
> Max,
> 
> I also add one test case for permission granted to signed multi-release jar 
> files,

Maybe it's better to move the checkPermission() calls into 
v9/version/Version.java? This would demonstrate that the versioned class itself 
is granted permissions.

In order to make sure it's not another version of Version.java that is running, 
I think you can call assertContains("I am running on version 10").

And if you don't intend to reuse Main.java, I think it's not worth passing into 
arguments.

Thanks
Max

> other functional tests are covered by 
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/ab164f8b8569/test/java/util/jar/JarFile/mrjar/MultiReleaseJarSecurity.java
> 
> Thanks
> Amanda
> On 12/12/16 6:31 PM, Wang Weijun wrote:
>> Hi Amanda
>> 
>> Can you also test the new JarSigner API?
>> 
>>http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ce33c780cfbd
>> 
>> line 2.72 has an example on it.
>> 
>>> On Dec 13, 2016, at 9:22 AM, Artem Smotrakov  
>>> wrote:
>>> 
>>> You can use 
>>> http://hg.openjdk.java.net/jdk9/dev/jdk/file/d4d7f1f0d688/test/lib/security/SecurityTools.java
>>>  which would simplify the code. This lib was added to be used in such tests.
>> Correct. I also wonder if there are existing methods on javac compilation.
>> 
>> Do we have existing tests on checking if a signed multi-version jar works as 
>> expected? Say, permission granted, getCertificates() returning non-null, etc?
>> 
>> Thanks
>> Max
>> 
> 
>