Hi Sean, new webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8237962.1/ Best Regards, Matthias > > Message: 2 > Date: Tue, 28 Jan 2020 10:45:59 -0500 > From: Sean Mullan <[email protected]> > To: [email protected] > Subject: Re: RFR: 8237962: give better error output for invalid OCSP > response intervals in CertPathValidator checks > Message-ID: <[email protected]> > Content-Type: text/plain; charset=windows-1252; format=flowed > > 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 >
