On 07/02/2013 02:49 PM, Vincent Ryan wrote:

I see your point Sean. My motivation for wrapping the RuntimeException was to 
ensure
that the failover mechanism (from OCSP to CRLs) did not get interrupted. But 
this is a
case when the failover should be interrupted. I've filed a new bug to address 
the issue:

Bug: 8019627: RuntimeException gets obscured during OCSP cert revocation 
checking
Webrev: http://cr.openjdk.java.net/~vinnie/8019627/webrev.00/

It simply re-throws RuntimeExceptions.

I would prefer to just revert to the previous code (before 8019259). The only checked exception thrown in that block is an IOException, so the extra try/catch blocks for RuntimeException and Exception are unnecessary.

--Sean




On 2 Jul 2013, at 16:19, Sean Mullan wrote:

On 07/01/2013 07:02 PM, Xuelei Fan wrote:
On 7/1/2013 8:56 PM, Vincent Ryan wrote:
I think that wrapping a RuntimeException (in CPVE) is acceptable in this case
because the goal is to activate the failover mechanism from OCSP to CRL.

Do you want RuntimeException to be re-thrown?

No. It is acceptable to me to wrap the RuntimeException.  It is not a
real runtime exception, but a wrong message. I prefer to use CPVE.

Sorry I was on vacation yesterday and was unable to review this until now. I'm 
not sure I like the fix. Catching a runtime exception and rethrowing it as a 
checked exception is generally not a good idea. In this case you have a 
situation where someone has misconfigured the property to specify an OCSP 
responder. Hiding that exception and allowing the revocation check to fallback 
to CRLs is dubious, even if we still capture the exception in a log file. I 
think it should be thrown as is, so the user knows about the issue immediately 
and can fix the configuration. So, even though it is technically a change in 
behavior from JDK 7, I think the current behavior is more correct, and I doubt 
this would cause many compatibility issues since it is a configuration error 
that needs manual intervention to be properly fixed.

Thanks,
Sean


Xuelei

On 29 Jun 2013, at 01:53, Xuelei Fan wrote:

Looks fine to me.

Hmm, it is a case to learn that RuntimeException should be token care of
sometimes.

Thanks,
Xuelei

On 6/29/2013 2:41 AM, Vincent Ryan wrote:
Hello,

Please review the following JDK 8 fix:

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8019259
Webrev:  http://cr.openjdk.java.net/~vinnie/8019259/webrev.00/

It corrects a problem during X.509 certificate revocation checking where 
failover to using CRLs is not
performed in the case when a malformed URL has been supplied as the URL of the 
OCSP responder.
The fix ensures all exceptions during OCSP are caught and wrapped so that the 
failover mechanism
does not get skipped.

Thanks.







Reply via email to