JDK 8 Review Request for 6854712 (JEP 124), 6637288 and 7126011

Sean Mullan sean.mullan at oracle.com
Tue May 29 17:25:47 UTC 2012


On 5/29/12 2:02 AM, Xuelei Fan wrote:
> On 5/26/2012 1:11 AM, Sean Mullan wrote:
>>> That's my comment on specification.  I may look into the implementation
>>> update next Monday.
> 
> # KeyChecker.java, ConstraintsChecker.java
> # PolicyChecker.java, ConstraintsChecker.java, minor comment:
> 
> public void check(Certificate cert, Collection<String> unresCritExts)
> - if (unresCritExts != null && !unresCritExts.isEmpty()) {
> + if (!unresCritExts.isEmpty()) {
> 
> This change may throw NPE when the unresCritExts argument is null. We
> used to allow null unresCritExts. This change may not cause
> compatibility issue because it is an internal class. But as might be
> confusing for the caller because it is not follow the spec strictly, it
> may be regard as null-argument-safe before reading into the
> implementation.  Just my very personal opinion.

You're right. I have restored the prior code.

> # SunCertPathBuilder.java
> 
> public CertPathBuilderResult engineBuild(CertPathParameters params)
> private PKIXCertPathBuilderResult build()
> -    result = buildCertPath(buildForward, true, adjList);
> +    result = buildCertPath(true, adjList);
> 
> This update disables reverse building.  The reverse building can only be
> set by SunCertPathBuilderParameters. 

I'm not sure I understand this comment. Reverse building could only be set by
SunCertPathBuilderParameters before my changes, so I haven't changed anything.
See lines 140-144 of the previous version of SunCertPathBuilder.

> It seems that this class is never
> used except the testing cases.  It's OK to disable it. I was just
> wondering we may be also want to delete the
> SunCertPathBuilderParameters.java file, related test cases, and update
> the comment of SunCertPathBuilder.engineBuild().  In the comment, it is
> talked that SunCertPathBuilderParameters can be used for reverse building.
> 
> 
> Please also refer to my previous comments. Otherwise, looks fine to me
> so far.

Great, I'll plan on posting a 2nd webrev later today. Since most of the comments
were minor, I'll plan on pushing my changes in the next day or so unless I get
more comments.

Thanks,
Sean



More information about the security-dev mailing list