安全かつ前向きなバグ修正の手順～新しいメソッドを作り、既存メソッドを非推奨にする～

# はじめに: バグ修正の際に意識するべきこと

バグ修正の際に意識するべきこととは、何でしょうか。
単に「そのバグが直る」ことだけではなく、今後は同じ間違いに陥らないようにすることも含まれるのではないでしょうか。それにより、プログラムの品質が高まり、アプリケーションの価値を高めることにつながります。

本記事では、そのためのメソッド修正の基本的な手順について説明します。

# やりたいこと

バグ修正の際には、下記の3つの要件を達成したいです。

- 問題となっている箇所では、動作を変更し、バグを修正したい
- 問題となっていない箇所では、動作を変更せず、悪影響が無いようにしたい
- 今後追加する箇所では、同じ間違いに陥らないようにしたい

# 解決策

理想的には、バグの原因が含まれるメソッド自体を修正するべきです。
ただし、そのためにはすべての呼び出し元(利用箇所)をリストアップすることが必要となる場合があります。
メソッドがすでに広く使われてしまっている場合、この解決策は現実的ではありません。

すべての呼び出し元をリストアップしての調査が現実的ではない場合、下記の手順で対応するのがスマートです。

## 1. 問題を解決した新しいメソッドを作る

問題が発生しているメソッドとは別に、問題を解決した新しいメソッドを作成します。
このとき、メソッド名や引数の数、順番なども、使いやすいように変えてしまって構いません[^1]。

ただし、同時にメソッドのドキュメント(API仕様書)やテストコードも揃えるようにしましょう。
それらを揃える過程では、メソッドの引数や返り値を定義することが自然と必要になります。

すると、当初は気が付かなかった思わぬ仕様の考慮漏れを発見できることがあります。

## 2. 問題となった利用箇所から、新しいメソッドを呼び出す

作成した新しいメソッドを、問題となった利用箇所から呼び出します。

この時点では、「問題があるかどうかがわからない」他の利用箇所の修正については、ひとまず後回しにします。

## 3. 既存のメソッドが非推奨であることを伝える

ドキュメンテーションコメントなどを用いて、既存のメソッドには問題が発生しているため、非推奨であるということを伝えます。

# 具体例

既存のメソッドと新しいメソッドについては、Javaのサンプルコードで説明します。

## 前提: 既存のメソッド(修正前)

```java
public String greet(String userName) {
  return "<p>こんにちは、" + userName + "さん！</p>";
}
```

既存のメソッドには、クロスサイト・スクリプティング(XSS)脆弱性が存在していたとします。
引数に渡された文字列に対してエスケープ処理がされていないため、Webページの出力内容にスクリプトを埋め込めてしまう可能性があります。

XSS脆弱性の修正方法の一つには、[ウェブページに出力する全ての要素に対して、エスケープ処理を施す](https://www.ipa.go.jp/security/vuln/websecurity-HTML-1_5.html)ことが必要です。このため、今回はエスケープ処理を加えることで[^2]バグ修正を試みます。

ただし、修正の過程で呼び出し元を参照してみると、引数に「エスケープ前の文字列を渡している箇所」と「エスケープ済の文字列を渡している箇所」が混在していることがわかりました。

既存のメソッド自体にエスケープ処理を加えた場合、引数にエスケープ済の文字列を渡している箇所では、二重にエスケープされてしまいます。このため、既存のメソッドを直接修正することは難しいです。

また、外からエスケープした文字列を渡すことで、今回問題となった箇所を修正すること自体はできます。しかし、その方法だと、**今後追加する箇所で、同じ間違いに陥らないようにすること**はできません。

## 問題を解決した新しいメソッド

```java
public String getGreetingHTML(String userName) {
    return "<p>こんにちは、" + StringEscapeUtils.escapeHtml4(userName) + "さん！</p>";
}
```

ひとまず、問題を解決した新しいメソッドを作成します。
今回は、[Apache Commons Text](https://commons.apache.org/proper/commons-text/)の[StringEscapeUtils.escapeHtml4(String input)](https://commons.apache.org/proper/commons-text/javadocs/api-release/org/apache/commons/text/StringEscapeUtils.html)を利用してエスケープ処理を加えました。

ただし、これだけではまだ不十分です。メソッドの仕様を明確にすることができていません。
たとえば、引数にその他の特殊な値を渡したときの仕様がわかりません。
nullや空文字を渡した場合は、何を返り値とすべきなのでしょうか。

このため、加えてドキュメンテーションコメント(Javadoc)を記載するようにしましょう。
これにより、引数や返り値の仕様について、自然と意識することができます。

```java:(ちょっと過剰ですが)ドキュメンテーションコメントのサンプル
/**
 * ユーザーにあいさつするHTML文字列を返します
 * @param name ユーザー名(事前にエスケープ処理をしてはいけません)
 * @return あいさつするHTML
 * @throws NullPointerException nameがnullの場合
 */
```

それに応じて、テストコードを追加したり、メソッドの実装を修正していきましょう。

## 既存のメソッド(修正後)

問題が含まれている既存のメソッドには、ドキュメンテーションコメントを追記し、非推奨とします。

```java:ドキュメンテーションコメントのサンプル
/**
 * ユーザーにあいさつするHTML文字列を返します
 * @param name HTMLエスケープ済のユーザー名
 * @return あいさつするHTML
 * @deprecated HTMLエスケープ前の文字列を渡すとXSS脆弱性を発生させるため
 * {@link #getGreetingHTML(String)}
 */
```

ドキュメンテーションコメントには、代替となるメソッドの案内も含めるべきです。
上記のサンプルでは[{@link}タグ](https://qiita.com/maku77/items/6410c67ce95e08d8d1bd#%E3%83%89%E3%82%AD%E3%83%A5%E3%83%A1%E3%83%B3%E3%83%88%E9%96%93%E3%81%AE%E3%83%AA%E3%83%B3%E3%82%AF)を用いて、新しいメソッドへと誘導しています。

また、同時に `@Deprecated` アノテーションもメソッドに付与しておきましょう。

# まとめ

このような手順を踏むことで、コードの品質(内部品質)を高めつつ、バグ修正をすることができます。
ぜひ、素敵なバグ修正ライフを！

# 参考URL

- [いつ、どのように API を非推奨とするか](https://docs.oracle.com/javase/jp/1.5.0/guide/javadoc/deprecation/deprecation.html)
- [Javadoc ドキュメンテーションコメントの書き方](https://qiita.com/maku77/items/6410c67ce95e08d8d1bd)

[^1]: テスト駆動開発をすると、自然と使いやすいAPIを意識できるのでおすすめです
[^2]: 理想的には[Thymeleaf](https://www.thymeleaf.org/doc/tutorials/3.0/usingthymeleaf_ja.html)のようなテンプレートエンジンを利用して対応する方が好ましいです