Hi Tony,
Here are the comments for the remaining file.
<ProviderList.java>
- line 621, is there a particular reason to return immediately when
s.ids is null? If there is, it should be commented here. The
get(ServiceList s) method will fall back to s.type and s.algorithm when
s.ids is null. Would be nice to state the reason for the difference in
implementation.
- line 698, the method name "compare" seems a little misleading, it only
compares type and algorithm and it returns boolean instead of integer as
the compare in other classes. Maybe names like "lookup" or "match" would
be better...
- line 705, there is no check on "t" being non-null, why not use
type.compareToIgnoreCase(t) instead?
- line 710, exact match on algorithm? I thought u will support partial
match, e.g. AES entry matching AES/CBC/PKCS5Padding request. Why don't u
use "toString()" in the debug output as it provides more info.
- line 722, why is "type" not included in the toString()?
Thanks,
Valerie
On 10/15/2015 11:42 AM, Valerie Peng wrote:
<MakeJavaSecurity.java>
- line 58-59, the "[openjdk target cpu architecture]" one should be
moved up. The optional restricted packages file names are at the end.
<General>
- for the javadoc changes, the approved CCC has @implNote instead of
@implSpec.
Instead of just {@code getProviders}, it seems {@code
Security.getProviders} is clearer.
<XMLSignatureFactory.java>
- line 262 - 267, given that there is an argument specifying provider
name, I don't think your changes applies to this method. If correct,
the javadoc change should be removed.
<java.security>
- looks fine.
I will continue to look at ProviderList.java and send u comments in a
separate email.
Thanks,
Valerie
On 10/9/2015 10:06 AM, Anthony Scarpino wrote:
Ping for a security review..
Tony
On 10/02/2015 10:08 AM, Anthony Scarpino wrote:
Hi all,
I'm need a review of the last developement piece to JEP 246, the
configuration changes.
I've copied the build-dev in case there were any comments on the minor
changes in the make directory related to the java.security file.
http://cr.openjdk.java.net/~ascarpino/8133151/webrev/
thanks
Tony
