The conditional AND and OR operators (&&
and ||
, respectively) exhibit short-circuit behavior. That is, the second operand is not evaluated if the result can be deduced solely by evaluating the first operand.
Exercise caution if the operands following the first operand contain side effects. In the following code, the value of i
is incremented only when i >= 0
.
Code Block |
---|
int i = /* initialize to user supplied value */ int max = /* initialize to maximum value */ if ( (i >= 0) && ( (i++) <= max) ) { /* code */ } |
Although the behavior is well defined, it is not immediately obvious whether i
gets incremented or not. Even in the absence of side effects, it is possible to write confusing expressions that do not execute as desired. This guideline exemplifies both these conditions.
Noncompliant Code Example
This noncompliant example is designed to rename a given file if it is present, perform operations on it, and then delete it. However, the renameTo()
method does not execute when the exists()
method returns true
, and an unsuspecting developer would incorrectly attempt to delete the fNew
file instead of the original one. This problem is exacerbated by the fact that File.delete()
does not throw an exception but returns an error code on failure, which is often silently ignored or perceived as unnecessary. (See EXP02-J. Do not ignore values returned by methods)
Code Block | ||
---|---|---|
| ||
class BadRenameFile { public static void main(String[] args) { File fOriginal = new File("original.txt"); File fNew = new File("new.txt"); if(fOriginal.exists() || fOriginal.renameTo(fNew)) { // do something with fNew fNew.delete(); // fNew does not exist as renameTo() was not executed } } } |
Compliant Solution
Knowledge of the short-circuit behavior can be used to enforce the desired specification. This program traps an error if the file does not exist or when it cannot be renamed to the new file name. Operations on the new file follow.
Code Block | ||
---|---|---|
| ||
class RenameFile { public static void main(String[] args) { File fOriginal = new File("original.txt"); File fNew = new File("new.txt"); if(!fOriginal.exists() || !fOriginal.renameTo(fNew)) { // handle error } // do something with fNew if(!fNew.delete()) { // handle error } } } |
Noncompliant Code Example
This noncompliant example differs from the previous one in that, there are no side effects in the right hand side operand. Nevertheless, an unaware programmer can get caught in the short-circuit behavior of the conditional AND and OR operators. The programmer has combined two expressions in the if
statement. The first checks whether the d
object is null and the second checks if the default security manager has been installed (by comparinf sm
with null
) depending on which the security check will be performed. This is a case of trying to combine together two null
checks into one statement. A conditional &&
is used as using a conditional ||
would mean that whenever d
is null
, the complete expression can still succeed depending on the value of sm
(see the next noncompliant example). This violates the invariants of d
as it is desired that operations on it be prohibited if it is null
.
Unfortunately, when d
is equal to null
as shown, the current if
expression evaluates to false
and the security check is not executed.
Code Block | ||
---|---|---|
| ||
// d = null SecurityManager sm = System.getSecurityManager(); if(d != null && sm != null) { FilePermission perm = new FilePermission("file.dat", "read"); sm.checkPermission(perm); /* do something with d */ } /* read the file (skips security check) */ |
Noncompliant Code Example
In this example, the programmer switches the ordering of the two subexpressions and uses a ||
operator to ensure the security check is carried out but does not realize that the second check is evaluated if the first one succeeds. The result is the inadvertent modification of d
.
Code Block | ||
---|---|---|
| ||
SecurityManager sm = System.getSecurityManager(); if(sm != null || d != null) { FilePermission perm = new FilePermission("file.dat", "read"); sm.checkPermission(perm); // do something with d } // read the file |
Compliant Solution
Decouple distinct operations that use the conditional AND and OR operators from expressions constituting decision statements. When inevitable, be aware of the short-circuit behavior and code accordingly.
Code Block | ||
---|---|---|
| ||
if(d != null) { /* do something with d */ } if(sm != null) { FilePermission perm = new FilePermission("file.dat", "read"); sm.checkPermission(perm); } /* read the file (default security check is carried out) */ |
Exceptions
Wiki Markup |
---|
*EXP06-J-EX1:* Sometimes programmers who are aware of the short-circuit behavior use it to their advantage, as Flanagan \[[Flanagan 05|AA. Java References#Flanagan 05]\] exemplifies: |
Code Block |
---|
if (data != null && i < data.length && data[i] != -1) ... |
This snippet sequentially executes the subexpressions while avoiding an array indexing exception resulting from the checks that execute prior to the last subexpression.
Risk Assessment
Failing to understand the short-circuit behavior of the logical AND and OR operators may cause unintended program behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
EXP06- J | low | unlikely | medium | P2 | L3 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[JLS 05|AA. Java References#JLS 05]\] Sections [15.23|http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#15.23] "Conditional-And Operator &&" and [15.24|http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#15.24] "Conditional-Or Operator ||" \[[Flanagan 05|AA. Java References#Flanagan 05]\] 2.5.6. Boolean Operators |
EXP05-J. Be careful about the wrapper class and autoboxing 03. Expressions (EXP) 03. Expressions (EXP)