You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 5 Next »

Calling overridable methods from the clone() method is dangerous 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 the object in an uninitialized state because its construction may not have concluded.

Noncompliant Code Example

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 gets executed erroneously. Not only does the subclass see the clone in an inconsistent state, it's 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.

class BadClone implements Cloneable {
  HttpCookie[] cookies;
  
  BadClone(HttpCookie[] c) {
    cookies = c;
  }
 
  public Object clone() throws CloneNotSupportedException {		
    final BadClone clone = (BadClone) super.clone();
    clone.doSomething();
    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 gets 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();
  }
}

Compliant Solution


  • No labels