tags:

views:

56

answers:

3

I've occasionally written Java code in the following manner:

public static TrustManager[] getTrustManagers(TruststoreParams tsParams)
throws IOException, GeneralSecurityException {
    TrustManagerFactory tmf = null;
    FileInputStream fis = null;
    try {
        ...
        return tmf.getTrustManagers();
    }
    catch (NoSuchAlgorithmException nsae) {
        throw new RuntimeException(nsae);
    }
    finally {
        ...
    }

where I am treating the NoSuchAlgorithmException a bit differently than other security-related exceptions. I am wondering if there is anything fundamentally wrong with the exception handling approach above where I have the GeneralSecurityException super-type in the throws list, but I am specifically catching and handling one of its sub-types and wrapping it as a RuntimeException.

How would you rewrite the above method skeleton, if differently than how I have it setup above, and why?

+3  A: 

It looks reasonable to me. By throwing a RuntimeException, you're essentially treating that case as a fatal error, which would make sense if your application requires you to use a particular algorithm.

Justin K
I agree with this - although you could throw OTHER security exceptions (or IOExceptions, or whatever), you have decided that this exception requires special handling. I might java doc it to make sure any callers are aware of this, and for documentation purposes.
aperkins
I'll admit that I've taken this approach from a couple of Java security related books that I've read, and quite a few blog/web postings. Just wanted to know if there was any exception-related subtlety that I was glossing over.
ssahmed555
+2  A: 

Catching a more-specific exception than you throw can make sense, in that it's always good to respond to specific cases IF YOU HAVE A SPECIFIC RESPONSE. In this case, though, why wrap it in a RuntimeException? Is it a worse case than GeneralSecurityException, i.e. it's so bad you want to bypass the usual handling by the caller? A different GeneralSecurityException wouldn't be as bad?

My approach has changed over the years. Lately I declare a lot fewer exceptions, typically just ones that the caller CAN programmatically deal with, mostly business-related (e.g. NoSuchUser). "System" or infrastructure issues get put into a RuntimeException of some variety, since the caller really can't do anything but bail out on.

In your case, is there anything the caller could do with the IOException or the GeneralSecurityException? If not, I wouldn't put them in the signature at all (it only confuses things for the caller) and just wrap any exceptions that occur in a RuntimeException.

Rodney Gitzel
+1 : bail out... or deal with them as close to the end user as possible. If the intermediate layers cannot do anything about it, then it also serves no purpose to fill them with piles of catch and throw clauses (making the log files unreadable in the process).
Peter Tillemans
The reason for adding IOException and GeneralSecurityException to the throws list is to allow the 'upper' layers to respond to those types of errors in a meaningful manner (ie display the appropriate error/log msg and exit accordingly). In retrospect, however, I agree with you that NSAE probably doesn't deserve to be treated much differently from other types of security exceptions. It's just that since I use the standard algorithms readily available in the default Sun JCE I don't expect the NSAE to ever be thrown - if it does get thrown it indicates badly setup/configured JRE/JDK security.
ssahmed555
A: 

I think that if one of the methods above is interested in NoSuchAlgorithmException then it should deal with it without further assistance.

I would do something like

 try {
     ...
     trustManager.getTrustmanagers(...);

 } catch (GeneralSecurityException e) {
     if (e instanceof NoSuchAlgorithmException) {
        // do something special 
     } else {
        // do regular error handling
     }
 }

In this case no superfluous structures need to be created which trigger actions over a distance, which I find always confusing when I stumble over them.

I must admit that I prefer RuntimeExceptions over checked exception genrally, but not in this case, since it just adds special cases on top of special cases.

Peter Tillemans