はじめに: バグ修正の際に意識するべきこと
バグ修正の際に意識するべきこととは、何でしょうか。
単に「そのバグが直る」ことだけではなく、今後は同じ間違いに陥らないようにすることも含まれるのではないでしょうか。それにより、プログラムの品質が高まり、アプリケーションの価値を高めることにつながります。
本記事では、そのためのメソッド修正の基本的な手順について説明します。
やりたいこと
バグ修正の際には、下記の3つの要件を達成したいです。
- 問題となっている箇所では、動作を変更し、バグを修正したい
- 問題となっていない箇所では、動作を変更せず、悪影響が無いようにしたい
- 今後追加する箇所では、同じ間違いに陥らないようにしたい
解決策
理想的には、バグの原因が含まれるメソッド自体を修正するべきです。
ただし、そのためにはすべての呼び出し元(利用箇所)をリストアップすることが必要となる場合があります。
メソッドがすでに広く使われてしまっている場合、この解決策は現実的ではありません。
すべての呼び出し元をリストアップしての調査が現実的ではない場合、下記の手順で対応するのがスマートです。
1. 問題を解決した新しいメソッドを作る
問題が発生しているメソッドとは別に、問題を解決した新しいメソッドを作成します。
このとき、メソッド名や引数の数、順番なども、使いやすいように変えてしまって構いません1。
ただし、同時にメソッドのドキュメント(API仕様書)やテストコードも揃えるようにしましょう。
それらを揃える過程では、メソッドの引数や返り値を定義することが自然と必要になります。
すると、当初は気が付かなかった思わぬ仕様の考慮漏れを発見できることがあります。
2. 問題となった利用箇所から、新しいメソッドを呼び出す
作成した新しいメソッドを、問題となった利用箇所から呼び出します。
この時点では、「問題があるかどうかがわからない」他の利用箇所の修正については、ひとまず後回しにします。
3. 既存のメソッドが非推奨であることを伝える
ドキュメンテーションコメントなどを用いて、既存のメソッドには問題が発生しているため、非推奨であるということを伝えます。
具体例
既存のメソッドと新しいメソッドについては、Javaのサンプルコードで説明します。
前提: 既存のメソッド(修正前)
public String greet(String userName) {
return "<p>こんにちは、" + userName + "さん！</p>";
}
既存のメソッドには、クロスサイト・スクリプティング(XSS)脆弱性が存在していたとします。
引数に渡された文字列に対してエスケープ処理がされていないため、Webページの出力内容にスクリプトを埋め込めてしまう可能性があります。
XSS脆弱性の修正方法の一つには、ウェブページに出力する全ての要素に対して、エスケープ処理を施すことが必要です。このため、今回はエスケープ処理を加えることで2バグ修正を試みます。
ただし、修正の過程で呼び出し元を参照してみると、引数に「エスケープ前の文字列を渡している箇所」と「エスケープ済の文字列を渡している箇所」が混在していることがわかりました。
既存のメソッド自体にエスケープ処理を加えた場合、引数にエスケープ済の文字列を渡している箇所では、二重にエスケープされてしまいます。このため、既存のメソッドを直接修正することは難しいです。
また、外からエスケープした文字列を渡すことで、今回問題となった箇所を修正すること自体はできます。しかし、その方法だと、今後追加する箇所で、同じ間違いに陥らないようにすることはできません。
問題を解決した新しいメソッド
public String getGreetingHTML(String userName) {
return "<p>こんにちは、" + StringEscapeUtils.escapeHtml4(userName) + "さん！</p>";
}
ひとまず、問題を解決した新しいメソッドを作成します。
今回は、Apache Commons TextのStringEscapeUtils.escapeHtml4(String input)を利用してエスケープ処理を加えました。
ただし、これだけではまだ不十分です。メソッドの仕様を明確にすることができていません。
たとえば、引数にその他の特殊な値を渡したときの仕様がわかりません。
nullや空文字を渡した場合は、何を返り値とすべきなのでしょうか。
このため、加えてドキュメンテーションコメント(Javadoc)を記載するようにしましょう。
これにより、引数や返り値の仕様について、自然と意識することができます。
/**
* ユーザーにあいさつするHTML文字列を返します
* @param name ユーザー名(事前にエスケープ処理をしてはいけません)
* @return あいさつするHTML
* @throws NullPointerException nameがnullの場合
*/
それに応じて、テストコードを追加したり、メソッドの実装を修正していきましょう。
既存のメソッド(修正後)
問題が含まれている既存のメソッドには、ドキュメンテーションコメントを追記し、非推奨とします。
/**
* ユーザーにあいさつするHTML文字列を返します
* @param name HTMLエスケープ済のユーザー名
* @return あいさつするHTML
* @deprecated HTMLエスケープ前の文字列を渡すとXSS脆弱性を発生させるため
* {@link #getGreetingHTML(String)}
*/
ドキュメンテーションコメントには、代替となるメソッドの案内も含めるべきです。
上記のサンプルでは{@link}タグを用いて、新しいメソッドへと誘導しています。
また、同時に @Deprecated アノテーションもメソッドに付与しておきましょう。
まとめ
このような手順を踏むことで、コードの品質(内部品質)を高めつつ、バグ修正をすることができます。
ぜひ、素敵なバグ修正ライフを！