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

Xuelei Fan xuelei.fan at oracle.com
Tue May 29 06:02:28 UTC 2012


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.



# 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. 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.

Thanks,
Xuelei



More information about the security-dev mailing list