- ベストアンサー
プリぺアードステートメントのエスケープ処理について
- プリぺアードステートメントを使用して、usersテーブルのデータを変更する方法について質問しています。
- 質問の中で示されたコードは一応動作するものの、適切なエスケープ処理が行われているかどうかについて疑問を抱いています。
- お答えいただければ幸いです。
- infinity38
- お礼率74% (86/116)
- PHP
- 回答数3
- ありがとう数1
- みんなの回答 （3）
- 専門家の回答
質問者が選んだベストアンサー
- ベストアンサー
求めている仕様が完全にこの通りであれば特に問題ありません。 以下、自分なりのこだわりをつけて続けますｗ --------------------------------------------------- connectDb() 関数は（推測ですが）以下のような実装になっていると思われます。 try { return new PDO('mysql:dbname=test;host=localhost;charset=utf8', 'user', 'pass'); } catch (PDOException $e) { die($e->getMessage()); } これはオブジェクト指向の利点を台無しにする誤った書き方です。 以下のまとめで例外処理に関しての記述をご覧ください。 Qiita - PHPオブジェクト指向入門 http://qiita.com/mpyw/items/41230bec5c02142ae691 これを踏まえた上で…正しい実装はこうです。 return new PDO( 'mysql:dbname=test;host=localhost;charset=utf8', 'user', 'pass', array(ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION) ); try ～ catch は関数を "使う側" で用意すべきであり、 更にその中で die() は使わずに適切に「エラーメッセージ」として HTMLの中に埋め込んで表示させるのが正しい使い方です。 （※見られて困る場合は「データベースエラーが発生しました」 など代わりのメッセージを使用します） 加えて、 PDO::ERRMODE_EXCEPTION を設定してコンストラクタ 以外でも操作に失敗したときに例外をスローするようにしています。 使用する側は catch(Exception $e) にして幅広く例外を全部捕まえる ようにしましょう。 更に発展させるのであれば、PDOクラスをプロパティに入れて保持した 「データベースクラス」を作るべきです。コンストラクタだけ関数で独立して 定義するのではなく、データベースに関連する操作を全てメソッドにして しまいましょう。 class DB { private $pdo; public function __construct() { $this->pdo = new PDO( 'mysql:dbname=test;host=localhost;charset=utf8', 'user', 'pass', array( ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, ATTR_EMULATE_PREPARES => false, ) ); } public function updateUser($id, $new_data) { $sql = "UPDATE users SET news_cnt = news_cnt + 1"; $sql .=" news = CONCAT(news, :newData) WHERE id = :id"; $stmt = $this->pdo->prepare($sql); $stmt->bindValue(':new_data', $new_data); $stmt->bindValue(':id', $id, PDO::PARAM_INT); $stmt->execute(); if (!$stmt->rowCount()) { throw new RuntimeException('更新対象が見つかりませんでした'); } } } 「ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC」はデフォルト フェッチモードを連想配列形式だけに限定するオプションです。今回は SELECTしてないので関係ないですが、メソッドを追加していったときに このオプションは設定しておいた方が便利だと思います。 executeでまとめて配列で渡して実行してもいいのですが、 このときはidも文字列としてバインドし、本来あるべき整数型への キャストをデータベース側に任せていることになります。 そうではなく、PHP側で整数としてバインドしたい場合は PDO::PARAM_INT を明示しなければなりません。 そしてこれをうまく働かせるためには PDO::ATTR_EMULATE_PREPARES を無効にする必要があります… UPDATEやINSERTが成功したかどうかは$stmt->rowCount()が1以上に なっているかどうかを見れば分かります。ここでは、更新対象が見つからなかった ときは自分でRuntimeExceptionをスローすることにしてみます。 あんまり長々書くとアレなのであとは記事の説明に投げたいと思いますｗ Qiita - PHPでデータベースに接続するときのまとめ http://qiita.com/mpyw/items/b00b72c5c95aac573b71 --------------------------------------------------- 上記で定義したクラスを使用する側はこんな感じの実装になると思います。 <?php function h($str) { return htmlspecialchars($str, ENT_QUOTES, 'UTF-8'); } $id = $_POST['id']; $new_data = $_POST['new_data']; try { $db = new DB; $db->updateUser($id, $new_data); $msg = '更新しました'; } catch (Exception $e) { $msg = $e->getMessage(); } header('Content-Type: text/html; charset=utf-8'); ?> <!DOCTYPE html> <html> <body> <p><?=h($msg)?></p> </body> </html> さて、 $id = $_POST['id']; $new_data = $_POST['new_data']; この部分ですが、ここは以下のように書きましょう。 $id = filter_input(INPUT_POST, 'id'); $new_data = filter_input(INPUT_POST, 'new_data'); 理由は以下で解説しています。 Qiita - $_GET, $_POSTなどを受け取る際の処理 http://qiita.com/mpyw/items/2f9955db1c02eeef43ea --------------------------------------------------- 以上、口うるさい回答でしたｗ
その他の回答 (2)
再三訂正失礼します。定数の頭にいくつか「PDO::」が抜けておりました。
訂正： 「使用する側は catch(Exception $e) にして幅広く例外を全部捕まえる ようにしましょう。」 を記述する場所を誤りました。RuntimeExceptionについて言及している文の次です。
お礼
ご回答いただきありがとうございます。 素晴らしいの一言です！ ただ、私の知識レベルでは理解できない部分があるため汗)、 じっくり見させていただき、勉強したいと思います。 この度は本当にありがとうございました。