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

Compare with Current View Page History

« Previous Version 4 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

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 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