RFR: 8237962: give better error output for invalid OCSP response intervals in CertPathValidator checks

Sean Mullan sean.mullan at oracle.com
Tue Jan 28 15:45:59 UTC 2020


Hi Matthias,

On 1/28/20 8:18 AM, Baesken, Matthias wrote:
> Hello, please review this small change .
> 
> While working on
> https://bugs.openjdk.java.net/browse/JDK-8237869
> and
> https://bugs.openjdk.java.net/browse/JDK-8237888
> I noticed that the error output in case of invalid OCSP response  
> intervals  could be improved a bit.
> 
> Bug/webrev :
> > https://bugs.openjdk.java.net/browse/JDK-8237962

This should be an Enhancement, and not a Bug. You should also add an 
appropriate noreg label (maybe noreg-trivial).

> http://cr.openjdk.java.net/~mbaesken/webrevs/8237962.0/

Please try to keep the lines within 80 characters to be consistent with 
the rest of the file.

It seems you could combine lines 602 and 604. I would also tweak the 
wording a bit, ex:

debug.println("Checking validity of OCSP response on " +
               new Date(now) + " with allowed interval between " +
               nowMinusSkew + " and " + nowPlusSkew);

Lines 615 through 625 should only be executed if debug is enabled. They 
should be in an "if (debug != null)" block. Also, if we were going to 
add this, I would probably restructure this entire block of code so you 
avoid code duplication.

But, I actually don't think these extra debugging statements are 
necessary. There is enough information in lines 600-605 to know exactly 
why the validity check failed. My general opinion on debugging logging 
is to give enough information to help diagnose errors, but don't give 
too much extraneous information.

--Sean


> 
> Thanks, Matthias
> 


More information about the security-dev mailing list