The conditional AND and OR operators (&&
and ||
respectively) exhibit short-circuit behavior. That is, the second operand is evaluated only when the result of the conditional operator cannot be deduced solely by evaluating the first operand. 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.
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) { // Use array } else { // Handle error }
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:
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.
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) { // Use array } else { // Handle error } } else { // Handle 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 (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.
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:
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 &&" |
8 Comments
David Svoboda
The NCCE/CS is subject to a TOCTOU race condition where an attacker can manipulate the filesystem such that the check that the original file exists succeeds, but then the attacker moves the original file so the rename operation fails.
Also, the NCCE is 'fixed' by relying on the short-circuit behavior. We need a NCCE/CS where the NCCE behaves badly b/c of short-circuiting, and the CS fixes things by not using logical operators (say by unrolling the if clause into statements). Preferrably the code samples should not rely on files b/c of the above vul. (But if necessary we could stipulate that the files are in a secure directory so TOCTOU is not possible.)
Robert Seacord
There was a phrase "Like most other Java operators, they evaluate both operands: first the left operand and then the right. "
It seems unlikely to me that Java provides any such guarantee. Rather than look it up, I'm removing the comment about ordering as I don't think it is important here anyway.
Robert Seacord
Phrases like "Nevertheless, this solution is preferable if the error-handling routines for each potential condition failure were different." makes me feel like this guideline was inadequately changed from the corresponding C rule. Doesn't Java handle stuff like this with exceptions?
Fred Long
Yes, but this refers to the error-handling code in the example. I've made a slight change to the wording to, hopefully, make this clearer.
James Ahlborn
Although i have some disagreements with EXP51-JG (assignment in conditional expression), if you are going to keep that rule, then you should probably fix the second CS which does 2 assignments in the conditional expression.
David Svoboda
Addressed in my most recent comment to EXP51-JG. The CS does not in fact violate EXP51-JG.
Dhruv Mohindra
&&
)Dean Sutherland
Dhruv's code changes appear to be fine.