Calling overridable methods from the clone() method is insecure for two reasons. First, a malicious subclass may override the method and affect the behavior of the clone() method. Second, a trusted subclass may observe can modify the object in an uninitialized state before its construction has concluded. Consequently, it is possible to leave the clone as well as the object being cloned, in an inconsistent state.
...
This noncompliant code example shows two classes, BadClone and Sub. BadClone calls an overridable method doSomething(). The overridden method sets the value of the cookies whereas the overriding method sets the values of the domain names. At runtime, because of polymorphism, the doSomething() method of the subclass Sub is erroneously executed. Not only does the subclass see the clone in an inconsistent state, its doSomething() method modifies it in a way that creates inconsistent copies. This is because the deepCopy() method occurs after the call to the doSomething() method and the overriding doSomething() implementation erroneously modifies the object undesirably .
| Code Block | ||
|---|---|---|
| ||
class BadClone implements Cloneable {
HttpCookie[] cookies;
BadClone(HttpCookie[] c) {
cookies = c;
}
public Object clone() throws CloneNotSupportedException {
final BadClone clone = (BadClone) super.clone();
clone.doSomething(); // Invokes overridable method
clone.cookies = clone.deepCopy();
return clone;
}
void doSomething() { // Overridable
for (int i = 0; i < cookies.length; i++) {
cookies[i].setValue("" + i);
}
}
HttpCookie[] deepCopy() {
if (cookies == null) {
throw new NullPointerException();
}
// deep copy
HttpCookie[] cookiesCopy = new HttpCookie[cookies.length];
for (int i = 0; i < cookies.length; i++) {
// Manually create a copy of each element in array
cookiesCopy[i] = (HttpCookie) cookies[i].clone();
}
return cookiesCopy;
}
}
class Sub extends BadClone {
Sub(HttpCookie[] c) {
super(c);
}
public Object clone() throws CloneNotSupportedException {
final Sub clone = (Sub) super.clone();
clone.doSomething();
return clone;
}
void doSomething() { // Erroneously executed
for (int i = 0;i < cookies.length; i++) {
cookies[i].setDomain(i + ".foo.com");
}
}
public static void main(String[] args) throws CloneNotSupportedException {
HttpCookie[] hc = new HttpCookie[20];
for (int i = 0 ; i < hc.length; i++){
hc[i] = new HttpCookie("cookie" + i,"" + i);
}
BadClone bc = new Sub(hc);
bc.clone();
}
}
|
...
This compliant solution declares the doSomething() and deepCopy() methods final, preventing these methods from being overridden.
| Code Block | ||
|---|---|---|
| ||
final void doSomething() {
// ...
}
final HttpCookie[] deepCopy() {
// ...
}
|
...