According to \[[Ware 08|AA. Java References#Ware 08]\], methods should always return a value that allows the developer to know the current state of the object and/or the result of the operation. This is consistent with the advice in [jg:EXP02-J. Do not ignore values returned by methods ]. The returned value should be as representative of the last known state as possible and should be designed keeping in mind the perceptions and mental model of the Methods should be designed to return a value that allows the developer to learn about the current state of the object and/or the result of an operation. This advice is consistent with EXP00-J. Do not ignore values returned by methods. The returned value should be representative of the last known state and should be chosen keeping in mind the perceptions and mental model of the developer. Wiki Markup
Feedback can also be provided by throwing either standard or custom exception objects derived from the Exception
class. With this approach, the developer can still get precise information about the outcome of the method and proceed to take the necessary actions. To do so, the exception thrown should provide a detailed account of the abnormal condition at the appropriate abstraction level.
To gain better ability at telling apart correct from fallacious results and enforcing that the incorrect results be carefully handled, a combination of the aforementioned approaches is recommended. At the same time, there are cases when an error value should be returned instead of an exception and vice versa. For instance, if some method is capable of failing in a variety of different ways, it is better to return failure codes than endeavoring to throw scores of different exceptions. Not that no possible failure codes should be within the range of valid return values.
Wiki Markup |
---|
Sometimes a state testing method \[[Bloch 08|AA. Java References#Bloch 08]\] can be used to ensure that the object is in consistent state at all points in time. This approach is not useful in the absence of external synchronization. Also, there is a TOCTOU race condition between invocation of the object's state testing method and the call to a method that depends on its state. During this interval, the object's state could change surreptitiously. |
Noncompliant code example
APIs should use a combination of these approaches both to help clients distinguish correct results from incorrect ones and to encourage careful handling of any incorrect results. In cases where there is a commonly accepted error value that cannot be misinterpreted as a valid return value for the method, that error value should be returned; and in other cases, an exception should be thrown. A method must not return a value that can hold both valid return data and an error code; see ERR52-J. Avoid in-band error indicators for more details.
Alternatively, an object can provide a state-testing method [Bloch 2008] that checks whether the object is in a consistent state. This approach is useful only in cases where the object's state cannot be modified by external threads. This prevents a time-of-check, time-of-use (TOCTOU) race condition between invocation of the object's state-testing method and the call to a method that depends on the object's state. During this interval, the object's state could change unexpectedly or even maliciously.
Method return values and/or error codes must accurately specify the object's state at an appropriate level of abstraction. Clients must be able to rely on the value for performing critical decisions.
Noncompliant Code Example
The updateNode()
method in this noncompliant code example modifies a node if it can find it in a linked list and does nothing if the node is not found. As shown in this noncompliant example, methods that are susceptible to failure can silently corrupt the state of the object if they do not return a value that the developer can interpret.
Code Block | ||
---|---|---|
| ||
public void updateNode(int id, int newValue) { Node current = root; while (current != null) { if (current.getId() == id) { current.setValue(newValue); break; } current = current.next; } } |
Compliant solution
This method fails to indicate whether it modified any node. Consequently, a caller cannot determine that the method succeeded or failed silently.
Compliant Solution (Boolean)
This compliant solution returns A recommended solution is to return the result of the operation ; true
for success and false
for failureas true
if it modified a node and false
if it did not.
Code Block | ||
---|---|---|
| ||
public boolean updateNode(int id, int newValue) { Node current = root; while (current != null) { if (current.getId() == id) { current.setValue(newValue); return true; // Node successfully updated } current = current.next; } return false; } |
Compliant
...
Solution (Exception)
This compliant Another informative solution returns the updated Node
so that the developer can simply check for a null
value lest the operation fails. Appropriate return values for methods can vary depending on the control flow or the information that the developer finds more usefulmodified Node
when one is found and throws a NodeNotFoundException
when the node is not available in the list.
Code Block | ||
---|---|---|
| ||
public Node updateNode(int id, int newValue){ throws NodeNotFoundException { Node current = root; while (current != null) { if (current.getId() == id) { current.setValue(newValue); return current; } current = current.next; } return nullthrow new NodeNotFoundException(); } |
Compliant solution
Using exceptions to indicate failure can be a good design choice, but throwing exceptions is not always appropriate. In general, a method should throw an exception only when it is expected to succeed but an unrecoverable situation occurs or when it expects a method higher up in the call hierarchy to initiate recovery.
Compliant Solution (Null Return Value)
This compliant solution returns the updated Node
so that the developer can simply check for a null
value if the operation fails. This solution combines the best of both worlds - exceptions and status codes. In this case, an exception is thrown if the operation is not successful. This ensures that the client has to handle the event wherein the Node
is not found.
Code Block | ||
---|---|---|
| ||
public Node updateNode(int id, int newValue) throws IdNotFoundException { Node current = root; while (current != null) { if (current.getId() == id) { current.setValue(newValue); return current; } current = current.next; } throw new NodeNotFoundException()return null; } |
Risk assesment
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MET04- J | medium | probable | medium | P8 | L2 |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
|
A return value that might be null is an in-band error indicator, which is discussed more thoroughly in ERR52-J. Avoid in-band error indicators. This design is permitted but is considered inferior to other designs, such as those shown in the other compliant solutions in this guideline.
Applicability
Failure to provide appropriate feedback through a combination of return values, error codes, and exceptions can lead to inconsistent object state and unexpected program behavior.
Bibliography
[Bloch 2008] | Item 59, "Avoid unnecessary use of checked exceptions" |
[Ware 2008] | Writing Secure Java Code |
...