JPCERT コーディネーションセンター

MET06-J. clone() からオーバーライド可能なメソッドを呼び出さない

clone()メソッドからオーバーライド可能なメソッドを呼び出すのは危険である。第1に、悪意あるサブクラスがメソッドをオーバーライドし、clone()メソッドの動作を操作する危険がある。第2に、サブクラスは信頼できるものだったとしても、構築完了前の、初期化途中のクローンオブジェクトにアクセスしてしまう(そして変更を加えてしまう)危険がある。いずれの場合も、サブクラスは、クローンして作られたオブジェクトとクローンされる側のオブジェクトのいずれかまたは両方を、矛盾した状態にする可能性がある。それゆえ、cloneメソッドはfinalあるいはprivate宣言されたメソッドのみを呼び出すべきである。

このルールは「MET05-J. コンストラクタにおいてオーバーライド可能なメソッドを呼び出さない」と密接に関連している。

違反コード

次の違反コード例は、CloneExampleSubの2つのクラスを示している。クラスCloneExampleはオーバーライド可能なdoSomething()メソッドを呼び出す。オーバーライドされる側のメソッドではクッキーの値が設定され、オーバーライドする側のメソッドではドメイン名が設定される。サブクラスSubdoSomething()メソッドは、ポリモーフィズムのため実行時に2度、誤って呼び出される。最初の呼出しはCloneExample.clone()によって、2度目の呼出しはSub.clone()によって行われる。そのため、クッキーの値は初期化されず、ドメイン名だけが2度初期化される。

さらに、サブクラスは矛盾した状態にあるクローンを見るだけでなく、矛盾した状態のコピーが作成されるようにクローンに変更を加えている。このような状況が発生する理由は、deepCopy()メソッドがdoSometing()メソッドの後で呼び出され、オーバーライドするdoSomething()がオブジェクトに誤って変更を加えるからである。

class CloneExample implements Cloneable {
  HttpCookie[] cookies;

  CloneExample(HttpCookie[] c) {
    cookies = c;
  }

  public Object clone() throws CloneNotSupportedException {
    final CloneExample clone = (CloneExample) super.clone();
    clone.doSomething(); // オーバーライド可能なメソッドを呼びだす
    clone.cookies = clone.deepCopy();
    return clone;
  }

  void doSomething() { // オーバーライド可能
    for (int i = 0; i < cookies.length; i++) {
      cookies[i].setValue("" + i);
    }
  }

  HttpCookie[] deepCopy() {
    if (cookies == null) {
      throw new NullPointerException();
    }

    // ディープコピー
    HttpCookie[] cookiesCopy = new HttpCookie[cookies.length];

    for (int i = 0; i < cookies.length; i++) {
      // 配列の各要素を手作業でコピーする
      cookiesCopy[i] = (HttpCookie) cookies[i].clone();
    }
    return cookiesCopy;
  }
}

class Sub extends CloneExample {
  Sub(HttpCookie[] c) {
    super(c);
  }

  public Object clone() throws CloneNotSupportedException {
    final Sub clone = (Sub) super.clone();
    clone.doSomething();
    return clone;
  }

  void doSomething() { // 誤って実行される
    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);
    }
    CloneExample bc = new Sub(hc);
    bc.clone();
  }
}

オブジェクトの浅いコピーに対してオーバーライド可能なメソッドが実行されると、コピー元のオブジェクトも変更されてしまう。

適合コード

以下の適合コードでは、doSomething()メソッドとdeepCopy()メソッドの両方をfinal宣言し、オーバーライドされるのを防止している。

class CloneExample implements Cloneable {
  final void doSomething() {
    // ...
  }
  final HttpCookie[] deepCopy() {
    // ...
  }

  // ...
}

オーバーライドされたメソッドの呼出しを防止する別の方法としては、これらのメソッドを private 宣言する、あるいはこれらのメソッドを含むクラスを final 宣言する、という方法も考えられる。

リスク評価

構築途中のクローンのオーバーライド可能なメソッドの呼出しは、クラスの内部状態を悪意あるコードに晒す恐れがある。また、初期化途中のクローンを信頼できるコードに公開することで、クローンオブジェクトやクローンされるオブジェクト、あるいはその両方の状態が破壊される可能性が生まれ、クラスの不変条件が侵害される恐れもある。

ルール 深刻度 可能性 修正コスト 優先度 レベル
MET06-J P12 L1
自動検出

自動検出は容易である。

参考文献
[Bloch 2008] Item 11. Override clone judiciously
[Gong 2003]  
翻訳元

これは以下のページを翻訳したものです。

MET06-J. Do not invoke overridable methods in clone() (revision 58)

Top へ

Topへ
最新情報(RSSメーリングリストTwitter