RFR: 8061842: Package jurisdiction policy files as something other than JAR

[Weijun Wang]

+        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
>