Skip to end of metadata
Go to start of metadata

Methods must not throw RuntimeException, Exception, or Throwable. Handling these exceptions requires catching RuntimeException, which is disallowed by ERR08-J. Do not catch NullPointerException or any of its ancestors. Moreover, throwing a RuntimeException can lead to subtle errors; for example, a caller cannot examine the exception to determine why it was thrown and consequently cannot attempt recovery.

Methods can throw a specific exception subclassed from Exception or RuntimeException. Note that it is permissible to construct an exception class specifically for a single throw statement.

Noncompliant Code Example

The isCapitalized() method in this noncompliant code example accepts a string and returns true when the string consists of a capital letter followed by lowercase letters. The method also throws a RuntimeException when passed a null string argument.

boolean isCapitalized(String s) {
  if (s == null) {
    throw new RuntimeException("Null String");
  }
  if (s.equals("")) {
    return true;
  }
  String first = s.substring(0, 1);
  String rest = s.substring(1);
  return (first.equals(first.toUpperCase()) &&
          rest.equals(rest.toLowerCase()));
}

A calling method must also violate ERR08-J. Do not catch NullPointerException or any of its ancestors to determine whether the RuntimeException was thrown.

Compliant Solution

This compliant solution throws NullPointerException to denote the specific exceptional condition:

boolean isCapitalized(String s) {
  if (s == null) {
    throw new NullPointerException();
  }
  if (s.equals("")) {
    return true;
  }
  String first = s.substring(0, 1);
  String rest = s.substring(1);
  return (first.equals(first.toUpperCase()) &&
          rest.equals(rest.toLowerCase()));
}

Note that the null check is redundant; if it were removed, the subsequent call to s.equals("") would throw a NullPointerException when s is null. However, the null check explicitly indicates the programmer's intent. More complex code may require explicit testing of invariants and appropriate throw statements.

Noncompliant Code Example

This noncompliant code example specifies the Exception class in the throws clause of the method declaration for the doSomething() method:

private void doSomething() throws Exception {
  //...
}

Compliant Solution

This compliant solution declares a more specific exception class in the throws clause of the method declaration for the doSomething() method:

private void doSomething() throws IOException {
  //...
}

Exceptions

ERR07-J-EX0: Classes that sanitize exceptions to comply with a security policy are permitted to translate specific exceptions into more general exceptions. This translation could potentially result in throwing RuntimeException, Exception, or Throwable in some cases, depending on the requirements of the security policy.

Risk Assessment

Throwing RuntimeException, Exception, or Throwable prevents classes from catching the intended exceptions without catching other unintended exceptions as well.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

ERR07-J

Low

Likely

Medium

P6

L2

Automated Detection

ToolVersionCheckerDescription
Parasoft Jtest
10.3
CODSTD.BP.NTX, EXCEPT.NTERRImplemented
SonarQube
6.7
S112Generic exceptions should never be thrown

Related Guidelines

MITRE CWE

CWE-397, Declaration of Throws for Generic Exception

Bibliography



12 Comments

  1. There is a debate in the community about checked vs unchecked exceptions. Some publications consider checked exceptions 'old school' and a few others leave the decision to the developer - if the client code can be expected to recover from the exception, make it a checked exception else an unchecked exception (by wrapping it in RuntimeException) and throwing it from the handler). [Doshi 03] is one example that takes such a middle ground. [Goetz 04b] discusses either side pretty well.

    Since this guideline is controversial, perhaps EXC-33J and EXC-32J should be better left as recommendations?

    1. Whether you go for checked or unchecked exceptions, this item is about throwing instances of these exact classes vs proper subclasses. Whichever way you go, it's a reasonable point (though I don't think personally it has substantive security ramifications).

      (The example code is incorrect for strings starting with a surrogate pair, btw.)

      1. Ah, I think we might have to adjust the name of the rule to reflect that. No doubt, this rule does not really advise against using unchecked exceptions though it does appear to do so from the name.
        EXC00-J. Do not suppress or ignore checked exceptions contains duplicate advice it seems, and ironically it is a recommendation. (edit: recommendation deprecated)

        All the same, following Sun's advice on preferring checked exceptions, I am adding a separate recommendation to avoid (or be wary of) undeclared checked exceptions. Sounds good?

  2. At least two tools (Fortify and Cigital, look under 'Java/Error') classify throwing/catching a NullPointerException as a bug. Perhaps we should address this in the first CS. AFAIT It should be permissible to use a custom class to signify programmer-initiated unchecked exceptions [Müller 02] "4.2 Usage of Unchecked Exceptions" or the IllegalArgumentException as stated in the Cigital link.

  3. This rule is called into question by Doshi [Doshi 03] who says:

    "3. Do not suppress or ignore exceptions

    "When a method from an API throws a checked exception, it is trying to tell you that you should take some counter action. If the checked exception does not make sense to you, do not hesitate to convert it into an unchecked exception and throw it again, but do not ignore it by catching it with {} and then continue as if nothing had happened."

    The examples there use throw new RuntimeException(" ... ");.

  4. If the intended behavior of isCapitalized() is that the caller is supposed to be informed of and recover from calling isCapitalized() with a null argument, the CE isCapitalized() should probably throw a checked exception rather than an unchecked exception. If the caller is not expected to recover from calling isCapitalized() with a null argument, the caller will not catch any exceptions when calling isCapitalized() and throwing RuntimeException will not be problematic (the RuntimeException, if it is ever caught, will be handled by a general unchecked exception handler at a high level in the application).

    This raises a general issue about the Java exception handling rules/recommendations: Do the rules/recs subscribe to the philosophy that unchecked exceptions should only be used to indicate unrecoverable errors and should rarely, if ever be caught, or should unchecked exceptions be used to indicate both recoverable and unrecoverable errors and hence should be caught as the normal part of an applications execution?

    1. ... subscribe to the philosophy that unchecked exceptions should only be used to indicate unrecoverable errors

      Yes.

      and should rarely, if ever be caught,

      No. This is a controversial topic with no widespread consensus. According to the scope (see Introduction section), this may not be fit to be included in the standard.

      should unchecked exceptions be used to indicate both recoverable and unrecoverable errors and hence should be caught

      You may catch Throwable and forward to a general exception handler so the answer is yes. It is also possible to wrap a checked exception into an unchecked exception when recovery is not expected, but IMO this should be documented so that the client knows.

  5. I'm not sure what EXC13-EX0 actually means. I think it needs a code example.

    • Methods can throw a specific exception subclassed from Exception.

    Except for RuntimeException. Perhaps we can change that to "custom exception classes that extend Exception".

    • Why only RuntimeExc in the title? What about throwing Exception? We still have that NCE/CS pair.
      • Except for RuntimeException. Perhaps we can change that to "custom exception classes that extend Exception".

      No good...that excludes built-in exceptions like IOException. I wordsmithed that sentence. Not perfect, but IMHO the best we can hope for.

      • Why only RuntimeExc in the title? What about throwing Exception? We still have that NCE/CS pair.

      I changed the title, and the title to ERR14-J as well.

  6. In the first compliant solution, should NullPointerException be in parentheses?

  7. Does this code violate this rule?

    public static void logAndRethrow( String message, Exception e) throws Exception {
      Log.error( message);
      throw e;
    }
    

    I will argue that rule EX0 of rule ERR08-J. Do not catch NullPointerException or any of its ancestors seems to consider this code valid.

    Clearly any method that calls this method must throw Exception, even if it throws only a specific exception. To comply with ERR07-J, the coders may not use this method, and must modify it in a major way (prob by making it generic, perhaps?) However, Turning every exception into a generic one just for logging is at least bad style. As an alternative, one can easily expand the call to log and rethrow into a call to log followed by throw to keep the exception type, e.g.,

    catch (IOException e) {
      log(e);
      throw e;
    }