Q&A
ご回答ありがとうございます。
私も最近はCakePHPを使うことが多いので、
フレームワークの機能をうまく使いながら、
適切にチェックしていくようにします。
この辺りは趣味プログラマーと
プロのプログラマーの差が出そうなところですね。
精進いたします。
PHPなどでWebアプリケーション開発をしていると、変数などの値をチェックする場面が数多く出てくるかと思います。
このチェックというのは、どのタイミングで行うのがいいのでしょうか。最初にユーザーからの入力値を受け取ったときや、データベースと連携した前後など何か決まりがあったりするのでしょうか。
以下のようなプログラムの場合。
php
1 2<?php 3 4$login_id = $_POST['id']; //ユーザーの入力値を受け取る 5 6・・・1 7 8$name = $this->find( $login_id ); //$login_idからDBからあらかじめ登録されている名前を取得 9 10・・・2 11 12$encodeName = $this->encode($name); //取得した名前を暗号化 13 14・・・3 15 16$this->db->save($encodeName); //暗号化した名前をデータベースへ格納 17 18・・・4 19 20?> 21
説明用に適当に書いていますので、間違っている部分があるかもしれませんので、その辺りはご容赦ください。
仮にこのようなプログラムを組んだ場合には、どこでチェックするのがいいのでしょうか?
どこでどのエラーが出る分からないと考えてしまうと、エラーチェックだらけになってしまうなと思いまして。ただ、あまり何度もエラーチェックするのは美しくないと思い、じゃあどのタイミングでチェックすれば良いのかと思い、質問しております。
人によりかなり差が出るような気がしますので、自分のやり方や考え方で問題ないです。
よろしくお願いいたします。
回答7件
0
「変数」という区切りだと、処理によって必要なチェックをその処理（クラスや関数）の責任範囲について実施するとしか言えません。
必要なチェックは必ずどこかで実施されなければいけませんが、責任範囲を正確に定義・設計することで同じ意図のチェックが複数の箇所で行われることは避けることが可能です。
＊ユーザからの入力値に限定した場合は
基本的に入力値のチェックは出来る限り早いタイミングで行い、
意図した入力値以外は入力された場合はエラーにして
不必要な処理が実行され無いようにするのが基本になります。
例の場合だと、以下の様なところは最低限必要でしょうか。
１のタイミングで入力値の想定する範囲、文字種、文字数についてチェックを行い、（入力値のバリデーション）IDとして想定される値以外の場合はエラーとする
＊ユーザからの入力値になるので最も気を付けてチェックしないといけない部分です。
２のタイミングでは、
find()の内部で$login_idが想定する形式であるかチェックを実施して、想定外であれば失敗を返却するか例外をthrowする
$nameの内容によって名前の取得の成否を確認してその後の処理を分岐（これもチェックといえばチェックです
3のタイミングでは
encode()の中で$nameが想定した形式であるかのチェック 想定していない形式であれば失敗を返却するか例外をthrowする
失敗が返却される仕様であれば、$encodeNameの中身をチェックして成否を確認
4のタイミングでは、db->sage()の中で$encodeNameが想定した形式か（以下略）
＊出来れば成否についても確認したい
という感じで、全ての箇所でのチェックが必要です。
どこでどのエラーが出る分からないと考えてしまうと、エラーチェックだらけになってしまうなと思いまして。ただ、あまり何度もエラーチェックするのは美しくないと思い、じゃあどのタイミングでチェックすれば良いのかと思い、質問しております。
どこでどのエラーが出るかわからないのは、各処理の責任範囲と役割が曖昧になってしまっているからだと思います。
値のチェックはどこかで必ずしないといけませんが、
例えば、
$this->db->save($encodeName);
の仕様が
「正常な値のみが渡されることを想定する（正常な値を用意する責任はメソッドを呼び出す側にあるとする）、どんな値が渡されてもとにかく保存を試みる。SQL的に失敗したらPDOの例外がthrowされる」
という仕様なのであれば、save()内で引数のチェックは必要なくなりますが、メソッドを呼び出す前に値の正当性をどこかでチェックする必要があります。
おそらく、save()は様々なところで呼び出されるため、同じようなチェックをいろんなところに書く羽目になります。
この場合はsave()に値のチェックの責任を持たせることで、save()の再利用性が高まり、アプリケーション全体で同じコードを書く必要が減ります。
＊SQLインジェクション対策などのエスケープは入力値のチェックとは別概念なのでチェックとは無関係に「その値の出力時に出力先に応じたエスケープ処理を行う」という定義になります。
投稿2015/11/02 17:20
総合スコア18713
0
入力値のチェックであれば、１の段階でチェックしますね。
各モジュールでチェックが必要になるので、めんどくさくなってフレームワークに手を出し始めたりします。
まずは、めんどくさいことを理解することが大事だったりするので、どんどんチェックしていっていいと思います。
投稿2015/11/02 16:34
総合スコア780
ごめんなさい。
入力値のチェックと勘違いし、１のチェックでOKと答えてしまいました。
変数チェックという意味では、下記にコメントされている方々同様
全ての変数をチェックしますね。
＞なるべく短いコードでと考えてきましたが、
＞それよりも安全な安定稼働のほうが大切ですよね。
ご自身の返信でもコメントされてましたが、上記のとおりサービス第一で良いと思います。
安定稼働 ＞ 短いコード（かつ読みやすいコード）
かなぁと思ってます。
ご返信ありがとうございます。
大変参考になります。
さらに理解を深めていきます。
ありがとうございます。
0
皆様、書かれているように、
基本は、早い段階でチェックしてしまうのが、吉かと。
ただ、
ただ、あまり何度もエラーチェックするのは美しくないと思い
とありますが、私はそんなことは無いと思っています。
というのは、
$login_id = $_POST['id']; //ユーザーの入力値を受け取る //チェック追加（ここはあくまでも$_POST['id']の正当性チェック) if($login_id == xxyyzz)エラー処理 //ここで何が起きるかはわからない・・・。 //冗長だが・・・ここでもチェックしてもいいかも(ここはfind関数を守るためのチェック)。 if($login_id == aabbcc)エラー処理 $name = $this->find( $login_id ); //$login_idからDBからあらかじめ登録されている名前を取得
仮に、上記のように$_POSTの後にチェックを追加してそのチェックの結果を信頼して、find関数が実行される仕組みですが、find関数が脆弱な状態も考えられますので、不正値でどのような挙動になるかもわからずという危険があります。
もしかして、途中で変数書き換えられているかもしれませんし。
なので、冗長になっても、さらにfindの前で再度チェックをしておくコードを入れておくというのも、考えて良いかと思います。（find関数を誰が作ったか？外部ライブラリ？自分で？など、いかに信頼できるかによりますね）
また、プログラムは、いづれは自分の手から離れていき、他の他人がメンテナンスする可能性があります。
なので、いつか$_POSTの後に入れたチェックのif文の内容を変更されてしまうかもしれません。このような短いコードでは有り得ませんが、$_POSTから、findまでの処理の間が長い場合は危険です。意図しないコードが追加されるかもしれません。
そのような意味で、作ったときだけでなく、今後のメンテナンスも含めて、適時、チェックを行うのがよいかと思います。
冗長を嫌い、美しいコードを追求することも大切ですが、
長い時間、安定して、そして頑丈で、そしてメンテナンスしやすいことが、プログラムは第一ですので、
多少の冗長化などは無視してもかまわないと思います。
そして最後に、
WEBでDBを操作するような場合には、うるさいくらいの引数のチェックをしておいてよいかと思います。
これでもか、というほどチェックをかけても、ＳＱＬインジェクションなどの危険性が残っている可能性がありますので。
投稿2015/11/02 18:49
総合スコア1283
ご回答ありがとうございます。
>>ただ、あまり何度もエラーチェックするのは美しくないと思い
>とありますが、私はそんなことは無いと思っています。
この考え方は非常に参考になります。
うるさいくらいにチェックをかけて、
値チェックするほうがよさそうですね。
なるべく短いコードでと考えてきましたが、
それよりも安全な安定稼働のほうが大切ですよね。
勉強になりました。
ありがとうございます。
0
基本的に、チェックロジックは、プロジェクトの性格が出る物です。
仕事で開発しているのであれば、世知辛いですが、行数＝お金となります。
※趣味や社内向けならべつですが・・・。仕事であるなら費用対効果を考えるべき。
基本的に、企業で開発している場合、その企業やプロジェクトで用意されている標準チェック関数をインクルードして使用するというのが、一般的かと思います。
※そもそも文字列チェックであれば、文字列の長さや、禁止文字など、チェックする項目はある程度決まっているので。
※この標準チェック関数こそ、その企業の宝です。
チェック処理やエラー処理は、その都度書く物ではなく、なるべく標準化して関数として纏めるべきものです。
工数をあまりかけたくない案件の場合は、登録手前でチェック処理を走らせますし、
逆にフォーム毎や、入力一文字毎チェックを走らせる場合もあります。
こればかりは、どちらが良いとか正しいという訳ではなく、そのプロジェクトの性格に寄ります。
ですので、どちらのチェック方法も覚えておくべきものです。
個人で開発される場合も、なるべくチェック処理を関数化して、呼び出すという意識で開発される事をお勧めします。
＞あまり何度もエラーチェックするのは美しくない
というのは、関数化出来ていないから美しくないのであって、エラーチェックは、イレギュラーケースも含め考えられるエラーを二重にも三重にもチェックするべきものです。
そして、関数化してあるチェック処理を呼び出すだけだからこそ、バグに強い強固なプログラムが出来ます。
エラーに限らず、チェック処理の基本は、関数化し、基本boolean型で戻り値を返す。
※もし戻り値に、幾つかの状態があるのであれば、数値型で意味付けをしてコメントで書いておく。
さらに、そこで加工した何かを返したければ、引数で親に返してあげる。
上記の関数を呼び出して使う。
このような流れとなります。
また、どこでこのチェック処理をやるべきなのか？を考えながら書いて見てください。
例えば、入力した直後の方が良いのか、それともDBの仕様として全体に対してチェックするべき物なのか？
どこでチェックするべきなのか？を考える事により、実は１度で良い処理を２度書いてしまったり、抜けもれが発生しにくくなります。
例えば、ユーザーが入力した値を、その後加工して、加工後の結果をDB登録するのであれば、DB登録前でしかチェックしようがないですよね。
この場合は、ユーザーが入力する時に想定されるエラーチェックを事前にしておき、さらにDB登録前に加工後のエラーチェックをするというロジックであれば、抜けもれを防げますよね。
ところで、質問者さんが記述されているロジックですが、このロジックそのものも、関数化できそうですね。
名前でなくても、そもそも暗号化してデータベースへ格納する行為は同じなので、
登録用関数に、それぞれのチェック関数を組み込んで、標準化出来ますよね。
記述されているロジックであれば、入力時の想定されるエラーチェック１、DBからあらかじめ登録されている名前を取得際の、DB接続エラーや、そもそもDBにあった場合のチェック処理2、取得した名前を暗号化する際に考えられるチェック処理3と細かくチェックしていきます。
そこで素通りしてしまう事の怖さを考えると、その都度チェックするが、一番ですね。
色々開発していかれると分かりますが、素通りしてしまうと、一体どこのエラーであるのか、切り分けが出来ず、バグがバグを呼ぶ原因となります。
自分を守るという意味においても、都度チェックを細かく細かくするというのを心がけるべきかと思いますね。
投稿2015/11/04 14:53
編集2015/11/04 15:28
退会済みユーザー
総合スコア0
0
理想で言えば、
①1パラメータごと入力チェック
→未入力チェック、数字チェック、桁数チェック など
②複合入力チェック
→複数日付の整合性 など
③DB整合性チェック
→DB存在チェック など
の順番で、
1.に①②
2.③
です。
その意味は、「決まり事」というより本来の処理を実装する箇所で、入力値によるケースバイケースを考えたくないからですかね。
実際は、時間がなくてJavaScriptでだけチェックしてサーバー側ではチェックしてないケースが多々有ります。
ＳＱＬインジェクション対策について
SQLインジェクションの対策は、入力チェック時ではなく、
$sql = " select * from table where key = ? ";
みたいな感じで実装して、
$sql = " select * from table where key = ".入力値." ";
のような書き方をしないようにします。
入力値が「'';delete from table;」だったら大変なことになりますから。
ただ、これも後者の実装をよく目にします。（蛇足）
チェックはかなり慎重に行っている
について誤解のないように再補足します。
チェックする内容は、セキュリティの目的ではなく、ユーザーへの思いやりです。
それは、入力チェックを入れたからセキュリティ対策が取れたとはいえないですし、
逆に、入力チェックしなかったからといってセキュリティ対策が取れていないとも言えないです。
そもそもセキュリティ対策はテストきちんとすることです。
例えば、ロジック内で使用しないパラメータがきたらエラーを返すなんていう処理はあまり見ないですが、
セキュリティ対策として、使用しないパラメータをわざと乗せてテストすることはあります。
さて、ユーザーへの思いやりとしての入力チェックですが、
ご質問の$login_idを例にします。
・例えば、8文字しか入らないとか英数字のみだったりしたら、入力チェックどうします？
思いやりの無い実装→どのみちSQLで取得できないだろうから、入力チェックしない
思いやりのある実装→桁数チェックや英数字チェックしてNGなら、「英数８文字でご入力ください」と出力
・例えば、未入力チェックをした場合、エラー文言はなんと出力します？
思いやりの無い実装→「未入力です」
思いやりのある実装→「正しいログインIDをご入力ください」
更に思いやりがあれば、$login_idをトリムした上でチェックすることも検討できるかと思います。
是非とも、ユーザーさんが路頭に迷わないような入力チェックを心がけてみてください。
投稿2015/11/02 23:30
編集2015/11/04 12:02
総合スコア1124
> 実際は、時間がなくてJavaScriptでだけチェックしてサーバー側ではチェックしてないケースが多々有ります。
それは絶対にやってはいけないケースです。後々SQLインジェクションなどのセキュリティホールに繋がる可能性があり、大変危険です。
参考文献: http://www.ipa.go.jp/security/awareness/vendor/programmingv2/contents/504.html
私はSQLインジェクション**だけ**を問題にして言っているのではありません。よくある例としてSQLインジェクションをあげているだけでSQLインジェクション**など**のセキュリティホールに繋がると言っているのです。
SQLインジェクション**だけ**を問題にしているのであれば、サーバ側での入力値チェックをしないことの危険性を理解しているとは思えません。一度参考文献のトップから隅から隅まで読んで理解することを推奨します。参考文献のトップは下記からアクセス可能です。
http://www.ipa.go.jp/security/awareness/vendor/programmingv2/web.html
teratailは初心者も見るサイトです。セキュリティ上問題があるコードについて、残念ながら多く存在してしまっているとしても、その危険性を注意喚起せずに書くべきではありません。危険なコードについて「多々有ります。」や「後者の実装をよく目にします。」で終わらせないでください。経験を積んでいる回答者として責任を持つのであれば、そのあとに「その危険性を理解せずに、絶対にそのように書いてはいけません。」と後に続くべきです。そう言った危険性について理解していないのであれば、口を噤むべきです。
ご回答また補足などもご丁寧にありがとうございます。
なるほどですね、手間がかかってもチェックはかなり慎重に
行っているようですね。
勉強になりました。
0
- はまあ、絶対にないのは説明不要ですよね。
仮に3で行った場合、どのみちNGになるのであれば、その前の処理は無駄な処理となって、わずかな差とはいえ、マシンのリソースを無駄に使用していることになります。
ですから、入力値のチェックは可能な限り初めの段階で行うのが適切と言えます。
投稿2015/11/02 16:48
退会済みユーザー
総合スコア0
あなたの回答
tips
プレビュー
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2015/11/04 10:34