Looks good to me. I didn't know you could pass a plain file 'name' to java.security.properties. The docs indicate that a URL is required but the jdk code suggests your approach will work.

The /policy=<file> jtreg tag was also another possible solution.

regards,
Sean.


On 17/11/2016 01:33, Bradford Wetmore wrote:


On 11/16/2016 4:14 PM, Wang Weijun wrote:

On Nov 17, 2016, at 6:10 AM, Bradford Wetmore <bradford.wetm...@oracle.com> wrote:


Current iteration:

   http://cr.openjdk.java.net/~wetmore/8169335/webrev.01

Changes:

1.  Using Debug "jca" instead of "policy"

2. Using debug.println (System.err), as the other jca calls are also using it.

Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca") always be true?

Ok, I think I've finally figured it out. For namespaces like JSSE and x509, there is a "main" namespace like "ssl" or "x509", and then subnames such as "plaintext" and "ava" which allow for more specific output selection criteria (e.g. ssl/trustmanager). For this case, I don't need to get more specific, so I can skip the isOn.

(My first thought was that maybe older versions of the JDK would allow for dynamic changes of the variable, but never saw that kind of code anywhere.)


3. Added regression test. Strips out any crypto.policy entry to create a new file, then uses it.

Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
in.filter(x -> !x.contains("crypto.policy")).forEach(out::println);
   }

Or:

        try (PrintWriter out = new PrintWriter(FILENAME)) {
            Files.lines(path)
                    .filter(x -> !x.trim().startsWith("crypto.policy"))
                    .forEach(out::println);
        }


Latest at:

http://cr.openjdk.java.net/~wetmore/8169335/webrev.02

Brad



--Max



4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:
In the recent jdk8u-dev edits of this file for 8157561, we introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:
Never noticed that before!  We have NOT been consistent in whether we
use:

   System.out.println()
or
   debug.println()

I knew SeanC wants to rework the JCA/JCE/Security debugging output in
another project, so I will remove the prefix for now. Thanks for
catching it.

I will also add a simple regression Test before I push. In hindsight,
it's not as trivial a change as I initially thought.  If you want to
review it, I can wait until you are back tomorrow.

Brad


On 11/15/2016 4:12 PM, Wang Weijun wrote:
You create a debug field with a prefix string and then check both
debug != null and Debug.isOn("policy") and then use
System.out.println to print the message. Something must be useless.

--Max

On Nov 16, 2016, at 3:31 AM, Bradford Wetmore
<bradford.wetm...@oracle.com> wrote:

Simple codereview:

http://cr.openjdk.java.net/~wetmore/8169335/webrev.00

The "crypto.policy" Security property is normally defined/configured
in the java.security file at build time.  (e.g. "limited" or
"unlimited") Rather than currently failing catastrophically if this
value doesn't exist, there should be a sensible default if it is
undeclared for whatever reason.  We will use a sane fallback value
of "limited".

If the distribution has also removed the "limited" policy directory
then the VM will still fail to initialize, but we have at least made
an effort to recover.

Thanks,

Brad





Reply via email to