Posted at

22章 See No Evil (臭い物に蓋)

More than 3 years have passed since last update.


目的: 簡潔なコードを書く

開発者は少ないコードで仕事をしたいと考えている。簡潔なコードには以下の利点があるため、ほとんど本能的に不要なコードを削除しようとする。


  • より短時間でコーディングを行える

  • テスト・ドキュメント化・ピアレビュー対象となるコードの量が減る

  • コードが少ないので、バグが混入する可能性も減る


アンチパターン: 肝心な部分を見逃す

開発者がこのアンチパターンに陥るときには2つのパターンがある。


  • データベースAPIの返り値を無視すること

  • アプリケーションコード内に点在する SQL しか読まないこと

この2つのケースで、簡単に入手できる情報を見逃してしまうことになる。


診断せずに判断する

    <?php

$pdo = new PDO("mysql:dbname=test;host=db.example.com",
"dbuser", "dbpassword");
$sql = "SELECT bug_id, summary, date_reported FROM Bugs
WHERE assigned_to = ? AND status = ?"
;
$stmt = $pdo->prepare($sql);
$stmt->execute(array(1, "OPEN"));
$bug = $stmt->fetch();

このコードは一見簡潔であるが、数々の戻り値を無視しており、問題に気づくことができない。


  • ① のようなデータベース接続は API がエラーを返すケースの主なものである


    • 原因: DB 名やホスト名の誤入力・ユーザ名やパスワードの誤り・ネットワーク障害など

    • PDO ではコネクションのインスタンス化に失敗すると例外が送出され、スクリプトの処理はそこで終了する



  • ② の prepare メソッドは、SQL に構文エラーがある場合に false を返す


    • その場合、③ の execute メソッドが false に対して呼ばれるので Fatal Error が発生する



  • ③ の execute メソッド呼び出しが失敗するパターンもあり得る


    • SQL ステートメントの制約違反や、アクセス権限違反など

    • その場合は execute メソッドが false を返す



  • ④ の fetch メソッドの呼び出しも、その時点で DB 接続失敗などのエラーが発生すると false を返す

デービスさんのような姿勢の開発者は珍しくない。


  • 戻り値が意図せず null や false になったり、呼び出したメソッドが例外を出すというようなことは起こるはずがなく、それらをチェックするために自らのコードに手を加える必要はない

  • 例外処理は似たようなコードの繰り返しが多いので、クールじゃない

と考える傾向にある。

しかしユーザが目にするのはコードではなく、アウトプットである。いくらコードが簡潔でも、ユーザに真っ白なページや意味不明な例外メッセージが表示されたらダメ!


見逃しがちなコード

このアンチパターンの原因となる悪習は、SQL 文字列を組み立てるアプリケーションコードの側からデバッグを開始することである。SQL 文字列はアプリケーションロジックや文字列連結、アプリケーション変数の中身なのどを用いて構築されるため、最終的に構築された SQL 文字列を思い浮かべるのは難しい。

例えば

<?php

$sql = "SELECT * FROM Bugs";
if ($bug_id) {
$sql .= "WHERE bug_id = " . intval($bug_id);
}
$stmt = $pdo->prepare($sql);

で構築されるクエリがエラーと成る原因は何か?

SELECT * FROM BugsWHERE bug_id = 1234

このように SQL そのものではなく SQL を構築するコードを調べることによって、デバッグ時に多くの時間とエネルギーを浪費してしまう。


アンチパターンの見つけ方

最新の IDE だとチェック例外の処理を怠っていると教えてくれる。 が Perl や Ruby でそういった機能を実現する環境はなさそう。

以下のような発言があればアンチパターンの兆候なので注意。


  • 「データベースにクエリを発行した後で、プログラムがクラッシュする」


    • クエリが失敗したにも関わらず、その結果を使用したため



  • 「SQL エラーの特定を手伝って欲しい。これがコードだ」


    • まずは実際に構築された SQL を調べることから



  • 「エラー処理でコードをゴチャゴチャさせたくないな」


    • 一説では、堅牢なアプリケーションのコードは最大50%がエラー処理のコードであるという




アンチパターンを用いても良い場合


  • エラーに対してまったく何もする必要がないときは、エラーチェックを省略できる


    • 例えば、アプリケーション終了時に DB コネクションを close する場合など



  • オブジェクト指向言語の例外のしくみでは、処理の責任の例外を通過させることができる


    • 例外処理の責任があるのは呼び出し側であると確信できる場合チャッチせず通過させる




解決策: エラーから優雅に回復する

ダンスの嗜みがある人は、ダンスにはミスステップが避けられないことを知っています。そのまま優雅に踊り続ける秘策は、いかにミスから回復するかです。ミスの原因に気づくためのチャンスを自分に与えるのです。そうすれば、素早くスムーズに動いて、他の誰かに気づかれる前にリズムを取り戻せます。

この章の言い回しがよくわからん。。。


リズムを維持する

データベース API 呼び出しの戻り値・例外のチェックはステップにミスがないことを保証するための最善策である。冒頭の PHP で PDO を使うコードだと次のようになる。

    <?php

try {
$pdo = new PDO("mysql:dbname=test;host=localhost",
"dbuser", "dbpassword");
} catch (PDOException $e) {
report_error($e->getMessage());
return;
}

$sql = "SELECT bug_id, summary, date_reported FROM Bugs
WHERE assigned_to = ? AND status = ?"
;

if (($stmt = $pdo->prepare($sql)) === false) {
$error = $pdo->errorInfo();
report_error($error[2]);
return;
}

if ($stmt->execute(array(1, "OPEN")) === false) {
$error = $stmt->errorInfo();
report_error($error[2]);
return;
}

if (($bug = $stmt->fetch()) === false) {
$error = $stmt->errorInfo();
report_error($error[2]);
return;
}

PDO ではコネクションのインスタンス化に失敗すると例外が発生するので、それを①でキャッチできるようにしている。それ以外では返り値が false になるので②③④でチェックしている。


ステップをたどり直す

デバッグには SQL クエリを構築するコードではなく、実際に構築された SQL クエリを使用することも重要である。


  • データベース API の引数に渡すその場で直接 SQL を組み立てるのではなく、まず一時変数を使って SQL クエリを構築してから API に渡すようにする


    • SQL の入った変数を使用前に調べられるようになる



  • アプリケーションとは別の出力先に SQL を出力するようにする

  • Web アプリ出力の HTML コメント内に SQL クエリを表示しないようにする


    • SQL クエリを読んだ攻撃者に、DB 構造について多くの情報を与えてしまう



ORM を使っていると、デバッグ作業が複雑になる場合がある。SQL クエリをアプリケーション側から隠蔽しているためである。

多くの ORM は生成した SQL をログに出力することで、この問題を解決している。

また、ほとんどの DB 製品はアプリ側のコードではなく DB サーバ自身のロギングメカニズムを提供しているので、アプリ側で SQL ログを取得できない場合はこちらを用いればよい。