+        Path javaHomePath = Paths.get(javaHomeProperty, "conf", "security",
+                "policy").normalize();

This is not javaHomePath, but policyPath.

You added "cryptoPolicyProperty" to some exceptions, but the exception titles are "Unexpected jurisdiction policy filename found: " and "Couldn't parse jurisdiction policy files: ". cryptoPolicyProperty is not file(s). Maybe "... file(s) in " + cryptoPolicyProperty?

No other comment.

Thanks
Max

On 8/25/2016 8:21, Bradford Wetmore wrote:
Max/SeanC/SeanM,

The latest update:

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

On 8/17/2016 9:26 PM, Wang Weijun wrote:
Before this change, you require default policy in neither export nor
import to be empty but do not care about the getMinimum() result. In
this change, you make sure the final result is not empty. I assume
this is a fix?

I made the change to allow for our traditional (default) export/import
mechanism, but other additional styles could be added/used.  Since we no
longer sign, distros are free to edit, add and/or remove files.  But
before doing any JCE operation, the environment needs to grant
something, otherwise there are no perms and no JCE available.

283                     // Did we find a default perms?

What does this line mean?

I've moved to the right position in the file.  I meant did we find a
default perms file, vs an exempt.

296                         // This should never happen

But you can still print out the file name.

I'm concerned that the exception might print out the entire path instead
of just the filename, which would include java.home and probably should
not be made available.

Can you rename policydir-tbd to something else? I am afraid it will be
confused with policy.url.1 etc.

Changed to:  crypto.policydir-tbd?

The original README.TXT in unlimited says "are exportable from the
United States." and you have "is exportable." now. Is this intended?
(IANAL)

Changed.

TestUnlimited.java:
45 "// Use the AES are the test Cipher", you mean "Use AES as the test
Cipher"?
51 "throw new Exception ("Unlimited policy is NOT active");". No space
before "(".

Fixed.

Sean Mullan wrote:

 > What about setting the default value to "limited"? And then this
 > would only be changed to "unlimited" if the build --enable-unlimited-
 > crypto option is specified?

I could, but I'm concerned that a build with --enabled-unlimited-crypto
would expect that the compiled-in version default would also be
unlimited and would be surprised with limited.

Upon Max's suggestion above, I've changed the name of the marker to
"crypto.policy=crypto.policydir-tbd."  Does that work for you?

 > Instead of throwing an exception here, I wonder if it would make more
 > sense to assume a default value of "limited" if the property is not
 > set or is empty.

We could, but see above.

Sean Coffey wrote:

Please include the exception 'e' in your last exception here.

Again, I'm concerned about outputting java.home, so I'm just going to
output the final directory name.

3. Test case.

The TestUnlimited.java testcase seems to be lacking. Do you want to
test other values for crypto.policy ? 'limited' would be one.
Throwing in some dummy value would also be good so that the exception
handling code gets exercised.

Done.

 * @run main/othervm TestUnlimited limited fail
 * @run main/othervm TestUnlimited unlimited pass
 * @run main/othervm TestUnlimited NosuchDir exception
 * @run main/othervm TestUnlimited . exception
 * @run main/othervm TestUnlimited /tmp/unlimited exception
 * @run main/othervm TestUnlimited ../policy/unlimited exception
 * @run main/othervm TestUnlimited ./unlimited exception

It needs to be run in ovm mode since you're setting a Security
property.

Yes, good catch.

Brad

Reply via email to