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