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

Reply via email to