Unless otherwise specified below, it was accepted..
http://cr.openjdk.java.net/~ascarpino/8154015/webrev.02/
Tony
On 06/30/2016 12:00 PM, Sean Mullan wrote:
Just a few comments -
AlgorithmChecker.java:
79 private Date pkixdate = null;
81 private Timestamp jarTimestamp = null;
These can be marked final I think.
Bcause the below NPE issue, it make sense to make them final. I can't
nest the constructors like I wanted to.
150 * {@code AlgorithmConstraints}.
s/AlgorithmConstraints/Timestamp/
153 * path for JAR files from deploy.
Avoid using "from deploy".
s/JAR files from deploy/signed JAR files that are timestamped./
159 this(certPathDefaultConstraints, jarTimestamp.getTimestamp());
This will throw NPE if jarTimestamp is null.
177 public AlgorithmChecker(AlgorithmConstraints constraints, Date
pkixdate) {
I think this should be private. You are only calling it within the
class. Also, the javadoc ctor description needs to be updated a little.
SunJSSE doesn't call this ctor AFAICT.
Actually the whole constructor isn't needed because of the previous NPE
possibility
PKIX.java:
107 this.params = ((PKIXTimestampParameters)
params).getPKIXBuilderParameters();
Shouldn't this be:
this.params = (PKIXBuilderParameters) params;
The passed in params doesn't itself contain the original
PKIXBuilderParamters data. It's the wrapper with an internal
PKIXBuilderParameter object it holds the data.
getPKIXBuilderParameters() passes the orignal PKIXBuilderParameters object.
Without it being setup this way I don't see how I can get and set the
timestamp.
201 Timestamp getTimestamp() {
Can you rename this to timestamp() to be consistent with rest of classes
methods that return params.
CertConstraintParameters.java
44 // Timestamp of the JAR file from deploy
s/JAR file from deploy/signed JAR file/
PKIXValidator.java
Since Timestamp is a new supported parameter, can you update the javadoc
of Validator.validate() to describe it?
Yes it should. I didn't notice that
PKIXTimestampParameters.java
- missing copyright and class description
30 public PKIXBuilderParameters getPKIXBuilderParameters() {
Do you need this method? See above comment on line 107 of PKIX.java
See response in PKIX..
44 public void setTimestampTrustAnchors(Set<TrustAnchor> t)
Does anyone call this method? Can it be removed?
yes it can...
--Sean
On 06/28/2016 05:17 PM, Anthony Scarpino wrote:
Hi,
I need a review of the below code. It's a continuation of the previous
certpath related changes. Additional constraints checking on
timestamped jars being checked by the deploy code
http://cr.openjdk.java.net/~ascarpino/8154015/webrev.01/
thanks
Tony