Poor code quality leads to unpredictable behavior. From a user's perspective that often manifests itself as poor usability. For an attacker it provides an opportunity to stress the system in unexpected ways.
notify()
is called.notify()
.notifyJob()
calls notify()
.
public synchronized notifyJob() {
flag = true;
notify();
}
...
public synchronized waitForSomething() {
while(!flag) {
try {
wait();
}
catch (InterruptedException e)
{
...
}
}
...
}
wait()
, but it is possible that notify()
will notify a different thread than the intended one.run()
method instead of calling start()
.Thread
object's run()
method is a bug. The programmer intended to begin a new thread of control, but accidentally called run()
instead of start()
, so the run()
method will execute in the caller's thread of control.run()
instead of start()
.
Thread thr = new Thread() {
public void run() {
...
}
};
thr.run();
stop()
method, potentially leaking resources.Thread
object's stop()
method is a bug. The programmer intended to stop a thread from running, but was unaware that this is not a suitable way to stop a thread. The stop()
function within Thread
causes a ThreadDeath
exception anywhere within the Thread
object, likely leaving objects in an inconsistent state and potentially leaking resources. Due to this API being inherently unsafe, its use was deprecated long ago.Thread.stop()
.
...
public static void main(String[] args){
...
Thread thr = new Thread() {
public void run() {
...
}
};
...
thr.start();
...
thr.stop();
...
}
clone()
method but does not implement the Cloneable
interface.Cloneable
interface because it implements a method named clone()
. However, the class does not implement the Cloneable
interface and the clone()
method will not behave correctly.clone()
for this class will result in a CloneNotSupportedException.
public class Kibitzer {
public Object clone() throws CloneNotSupportedException {
...
}
}
ICloneable
interface specifies a weak contract for its Clone
method and should be avoided.ICloneable
interface does not guarantee deep cloning, classes that implement it may not behave as expected when they are cloned. Classes that implement ICloneable
and perform only shallow-cloning (copies only the object, which includes existing references to other objects) may result in unexpected behavior. Because deep-cloning (copies the object and all referenced objects) is typically the assumed behavior of a clone method, the use of the ICloneable
interface is error prone and should be avoided.clone()
method within the class calls a function that can be overridden.clone()
function calls an overridable function, it may cause the clone to be left in a partially initialized state, or become corrupted.clone()
function calls a method that can be overridden.
...
class User implements Cloneable {
private String username;
private boolean valid;
public Object clone() throws CloneNotSupportedException {
final User clone = (User) super.clone();
clone.doSomething();
return clone;
}
public void doSomething(){
...
}
}
doSomething()
and its enclosing class are not final
, it means that the function can be overridden, which may leave the cloned object clone
in a partially initialized state, which may lead to errors, if not working around logic in an unexpected way.equals()
method can result in unexpected behavior.equals()
method should be called instead of the operators ==
and !=
. The Java Specification states about boxing conversions:Boolean
or Byte
), only a range of values will be cached, or memoized. For a subset of values, using ==
or !=
will return the correct value, for all other values outside of this subset, this will return the result of comparing the object addresses.
...
Integer mask0 = 100;
Integer mask1 = 100;
...
if (file0.readWriteAllPerms){
mask0 = 777;
}
if (file1.readWriteAllPerms){
mask1 = 777;
}
...
if (mask0 == mask1){
//assume file0 and file1 have same permissions
...
}
...
Example 1
uses Integer
boxed primitives to try to compare two int
values. If mask0
and mask1
are both equal to 100
, then mask0 == mask1
will return true
. However, when mask0
and mask1
are both equal to 777
, now mask0 == maske1
will return false
as these values are not within the range of cached values for these boxed primitives.