I am leaving for vacation tonight and probably can't review the updates for Mandy's comments. Existing changes are good enough for me, and what Mandy suggested sounds fine with me.
As long as the tests run as expected, I have no further comments.
Thanks,
Valerie

On 12/9/2015 10:25 AM, Mandy Chung wrote:
Some high-level comment:
1. I suggest to rename JigsawSecurityUtils.java to SecurityUtils.java.  The 
Jigsaw prefix is not needed.
2. SecurityProviderModularTest does the setup and launch the test with a set of 
different settings. I suggest to convert it to TestNG and you can reference the 
existing test/jdk/jigsaw tests which are a driver test that does the 
compilation and setup with @BeforeTest and @Test for each run test together 
with @DataProvider.  [1] is a simple test for you to get started.

Some comments on JigsawSecurityUtils.java:
71         Arrays.asList(options).stream().forEach(o ->  command.append(SPACE + 
o));

this can be replaced with
     Arrays.stream(options).collect(Collectors.joining(SPACE));

111         System.out.println(String.format(

You can do:  System.out.format(….) instead;

Mandy
[1] 
http://hg.openjdk.java.net/jigsaw/jake/jdk/file/c1d583efa466/test/jdk/jigsaw/reflect/Proxy/ProxyTest.java

On Dec 9, 2015, at 10:02 AM, Sibabrata Sahoo<sibabrata.sa...@oracle.com>  wrote:

Hi Valerie,

Here is the updated webrev: http://cr.openjdk.java.net/~ntv/siba/webrev.00/

Changes applied as per previous comments.
- File names are modified and do not include the term JCE.
- Source folder structure is flat now and all belongs to 
"java/security/Provider" folder.
- Sign jar functionality removed.

Thanks,
Siba

-----Original Message-----
From: Valerie Peng
Sent: Wednesday, December 09, 2015 5:37 AM
To: Sibabrata Sahoo
Cc: jigsaw-...@openjdk.java.net; security-dev@openjdk.java.net; Alan Bateman
Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security providers 
if they are in signed/unsigned modular JARs

Hi Siba,

Here are some nits:

I think it's somewhat misleading to use the term JCE here as what you are 
testing here is just security provider loading. JCE is more about security 
providers supporting export-controlled services/algorithms.
Since your provider is just an empty one, I don't think u need to sign it 
(again, it's only for JCE providers).
You can remove a lot of code as a result.
Also, your directory seems a bit deep, e.g.
modules/src/jceprovidermodule/provider/TestJCEProvider.java vs 
modules/src/jceclientmodule/provider/TestJCEProvider.java. Can all of these 
files be under the same directory instead of spreading over several level of 
subdirectories? The file names are different enough to tell which is which. 
Other regression tests for provider loading functionality are under 
test/java/security/Provider. Easier to find this way.

Can you please make the appropriate changes, e.g. don't use the term JCE, get 
rid of the signing, and simplify the directory structure if possible?

Thanks,
Valerie

On 12/8/2015 2:04 PM, Valerie Peng wrote:
Right, that'd be my expectation as well. Sounds like everything works.
I will change to look at your latest webrev.
Valerie

On 12/8/2015 6:09 AM, Sibabrata Sahoo wrote:
Hi Valerie,

Here is the updated webrev:
http://cr.openjdk.java.net/~ralexander/8130360/webrev.00/

Now the modular behavior for the test works as per expectation
through JAKE build with the following condition.
If the provider jar is available under ModulePath then the
"java.security" file should have the provider configuration entry as
ProviderName while in case of ClassPath the entry should hold the
full class name.

Thanks,
Siba

-----Original Message-----
From: Valerie Peng
Sent: Tuesday, December 08, 2015 4:46 AM
To: Sibabrata Sahoo
Cc: Alan Bateman; security-dev@openjdk.java.net;
jigsaw-...@openjdk.java.net
Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security
providers if they are in signed/unsigned modular JARs

Siba,

I have just started to review this webrev and not done yet.
As for your question, the java.security file in OpenJDK9 still uses
the provider class names instead of provider names. Are you talking
about the java.security file in Jigsaw? Which build (OpenJDK or
Jigsaw) have you tested against. I am pretty sure that I tested the
3rd party provider on classpath setting when I worked on the 7191662
changes.

Supposedly, if the jar files are available on class path, then you
should specify its full *class name* in the java.security file for it
to be instantiated. Otherwise, how would it find the class? Only the
classes discovered by ServiceLoader can be identified using the
provider name (which is different from the class name referred
above). So, in your scenario, that would be
"provider.TestJCEProvider" instead of "TEST".

If you still run into problems, try enable the java security debug
flag and u should get a good  idea why it isn't loaded. Let me know
if you still have questions.

Thanks,
Valerie

On 11/30/2015 6:47 AM, Sibabrata Sahoo wrote:
I would like to know more about this. As far, I can see the
"java.security" provider configuration available with JDK9, it holds
provider names instead of provider class names. In that case how it
resolve the fall back?

Thanks,
Siba

-----Original Message-----
From: Alan Bateman
Sent: Monday, November 30, 2015 4:54 PM
To: Sibabrata Sahoo; security-dev@openjdk.java.net;
jigsaw-...@openjdk.java.net
Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security
providers if they are in signed/unsigned modular JARs

On 30/11/2015 11:13, Sibabrata Sahoo wrote:
Here is the updated webrev:
http://cr.openjdk.java.net/~asmotrak/siba/8130360/webrev.02/

I have one question:
What should be the behavior when the older version of 3rd party JCE
provider jar file(without service descriptor
"META-INF/services/*"&    working with<= JDK8) configured by
"java.security" file, will be place in CLASS_PATH, running through
JDK9 and the client is using Security.getProvider() to look for the
provider?

Currently the scenario fails to find the JCE provider. Is this
right behavior? If it is, then jdk9 is not backward compatible to
find the security provider provided through older jar files from
CLASS_PATH.

The JCE work in JDK 9 (via JDK-7191662) was meant to address this
point by falling back and attempting to load the class name
specified via the security.provider.<N>    properties in the
java.security file. I'm sure Valerie can say more about this.

-Alan

Reply via email to