The conditional AND and OR operators (&&
and ||
, respectively) exhibit " short-circuit " operationbehavior. That is, the second operand is not evaluated if only when the result can of the conditional operator cannot be deduced solely by evaluating the first operand.
One should 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.
Consequently, when the result of the conditional operator can be deduced solely from the result of the first operand, the second operand will remain unevaluated; its side effects, if any, will never occur.
The bitwise AND and OR operators (&
and |
) lack short-circuit behavior. Similar to most Java operators, they evaluate both operands. They return the same Boolean result as &&
and ||
respectively but can have different overall effects depending on the presence or absence of side effects in the second operand.
Consequently, either the &
or the &&
operator can be used when performing Boolean logic. However, there are times when the short-circuiting behavior is preferred and other times when the short-circuiting behavior causes subtle bugs.
Noncompliant Code Example (Improper &
)
This noncompliant code example, derived from Flanagan [Flanagan 2005], has two variables with unknown variables. The code must validate its data and then check whether array[i]
is a valid index.
Code Block | ||
---|---|---|
| ||
int array[]; // May be null
int i; // May be an invalid index for array
if (array != null & | ||
Code Block | ||
int i = /* initialize to user supplied value */ int max = /* initialize to maximum value */ if ( (i >= 0) & i < array.length & ( (i++) <= max) )array[i] >= 0) { // Use array } else { /*/ codeHandle */error } |
Although the behavior is well defined, it is not immediately obvious whether i
gets incremented or not.
Noncompliant Code Example
This noncompliant examples is designed to rename a given file if it is present, perform operations on it and then delete it. It can be deceptive that the renameTo()
method will execute when the exists()
method returns true
, and an unsuspecting developer would incorrectly delete the new non-existent 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.
This code can fail as a result of the same errors it is trying to prevent. When array
is NULL
or i
is not a valid index, the reference to array
and array[i]
will cause either a NullPointerException
or an ArrayIndexOutOfBoundsException
to be thrown. The exception occurs because the &
operator fails to prevent evaluation of its right operand even when evaluation of its left operand proves that the right operand is inconsequential.
Compliant Solution (Use &&
)
This compliant solution mitigates the problem by using &&
, which causes the evaluation of the conditional expression to terminate immediately if any of the conditions fail, thereby preventing a runtime exception:
Code Block | ||
---|---|---|
| ||
int array[]; // May be null
int i; // May be an invalid index for array
if (array != null && i >= 0 &&
i < array.length && array[i] >= 0) {
// Handle array
} else {
// Handle error
}
|
Compliant Solution (Nested if
Statements)
This compliant solution uses multiple if
statements to achieve the proper effect.
Code Block | ||
---|---|---|
| ||
int array[]; // May be null
int i; // May be a valid index for array
if (array != null) {
if (i >= 0 && i < array.length) {
if (array[i] >= 0) | ||
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)) { // handleUse errorarray } else { // Handle error do something with fNew} if(!fNew.delete())} else { // handleHandle error } } else { } // Handle }error } |
Although correct, this solution is more verbose and could be more difficult to maintain. Nevertheless, this solution is preferable when the error-handling code for each potential failure condition is different.
Noncompliant Code Example
...
In this noncompliant example, 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 exists depending on which the security check will be performed. This is a classic case of trying to club 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 would violate the invariants of d since 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 carried out as desired.
(Improper &&
)
This noncompliant code example demonstrates code that compares two arrays for ranges of members that match. Here i1
and i2
are valid array indices in array1
and array2
respectively. Variables end1
and end2
are the indices of the ends of the matching ranges in the two arrays.
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 would not be carried out 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 would sequentially execute the subexpressions and avoid an array indexing exception due to the checks that execute prior to the last (offending) 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]\] 15.23 "Conditional-And Operator &&" and 15.24 "Conditional-Or Operator ||"
\[[Flanagan 05|AA. Java References#Flanagan 05]\] 2.5.6. Boolean Operators |
if (end1 >= 0 & i2 >= 0) {
int begin1 = i1;
int begin2 = i2;
while (++i1 < array1.length &&
++i2 < array2.length &&
array1[i1] == array2[i2]) {
// Arrays match so far
}
int end1 = i1;
int end2 = i2;
assert end1 - begin1 == end2 - begin2;
}
|
The problem with this code is that when the first condition in the while
loop fails, the second condition does not execute. That is, once i1
has reached array1.length
, the loop terminates after i1
is incremented. Consequently, the apparent range over array1
is larger than the apparent range over array2
, causing the final assertion to fail.
Compliant Solution (Use &
)
This compliant solution mitigates the problem by judiciously using &
, which guarantees that both i1
and i2
are incremented regardless of the outcome of the first condition:
Code Block | ||
---|---|---|
| ||
public void exampleFunction() {
while (++i1 < array1.length & // Not &&
++i2 < array2.length &&
array1[i1] == array2[i2]){
//doSomething
}
} |
Applicability
Failure to understand the behavior of the bitwise and conditional operators can cause unintended program behavior.
Bibliography
§2.5.6., "Boolean Operators" | |
[JLS 2013] | §15.23, "Conditional-And Operator &&" |
...
EXP05-J. Be careful about the wrapper class and autoboxing 02. Expressions (EXP) 02. Expressions (EXP)