質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

87.78%

パスワード修正のコードエラーについて

解決済

回答 2

投稿 編集

  • 評価
  • クリップ 0
  • VIEW 1,582

score 10

現在PHPの入門書にて、WEBサイトからDBへスタッフの名前とパスワードを追加した後、そのパスワードを修正するというプログラムを練習していましたところ、追加まではうまくいくのですが、修正を実行した際にエラーメッセージが出てしまいます。

該当箇所を見たのですが、どうしても原因が特定できず、お力をお借りできればと思い、投稿させて頂きました。

何卒よろしくお願いいたします。

以下、実行しているコードになります。
================================

<!DOCTYPE html>
<html>
 <head>
  <meta charset="UTF-8">
  <title>タイトル</title>
 </head>
 <body>

  <?php

  try
  {

  $staff_code=$_POST['code'];
  $staff_name=$_POST['name'];
  $staff_pass=$_POST['pass'];

  $staff_name=htmlspecialchars($staff_name);
  $staff_pass=htmlspecialchars($staff_pass);

  $dsn='mysql:dbname=shop;host=localhost';
  $user='root';
  $password='';
  $dbh=new PDO($dsn,$user,$password);
  $dbh->query('SET NAMES utf8');

  $sql='UPDATE mst_staff SET name=?,password=? WHERE code=?';
  $stmt=$dbh->prepare($sql);
  $data[]=$staff_name;
  $data[]=$staff_pass;
  $data[]=$staff_code;
  $stmt->execute($data);

  $dbh=null;

  }
  catch(Exception $e)
  {
      print 'ただいま障害により大変ご迷惑をおかけしております。';
      exit();
  }

  ?>

  修正しました。<br/>
  <br/>
  <a href="staff_list.php">戻る</a>

 </body>
</html>

================================

ちなみに15行目は
「  $staff_name=$_POST['name'];」
になります。

イメージ説明

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • kei344

    2016/09/12 16:09

    コードはコードブロックで囲んでいただけませんか? ```(バッククオート3つ)で囲み、前後に改行をいれるか、コードを選択して「<code>」ボタンを押すとコードブロックになります。

    キャンセル

  • kotan0

    2016/09/12 17:48

    kei344様
    ご指摘いただきありがとうございます。
    ただいまコード部分を修正いたしました。
    お手数をおかけして申し訳ございませんでした。

    キャンセル

回答 2

checkベストアンサー

+1

場当たりな対処を施したコード

<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <title>タイトル</title>
    </head>
    <body>

        <?php
        try {

            /*
             * グローバル変数に直接アクセスしない
             * 
              $staff_code = $_POST['code'];
              $staff_name = $_POST['name'];
              $staff_pass = $_POST['pass'];
             */

            $staff_code = filter_input(INPUT_POST, 'code');
            $staff_name = filter_input(INPUT_POST, 'name');
            $staff_pass = filter_input(INPUT_POST, 'pass');

            /*
             *  SQLに渡す変数を htmlspecialchars 使うのは論外
             *
              $staff_name = htmlspecialchars($staff_name);
              $staff_pass = htmlspecialchars($staff_pass);
             */


            /**
             * try 〜 catch でエラーを細くするためには、以下の設定が必要
             */
            $options = array(
                PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
            );

            $dsn = 'mysql:dbname=shop;host=localhost;charset=utf8;';
            $user = 'root';
            $password = '';
            $dbh = new PDO($dsn, $user, $password, $options);

            /*
             * 文字コードは $dsn で
              $dbh->query('SET NAMES utf8');
             */

            $sql = 'UPDATE mst_staff SET name=?,password=? WHERE code=?';
            $stmt = $dbh->prepare($sql);
            $data[] = $staff_name;
            $data[] = $staff_pass;
            $data[] = $staff_code;
            $stmt->execute($data);

            /**
             * いらない
              $dbh = null;
             */
        } catch (Exception $e) {
            print 'ただいま障害により大変ご迷惑をおかけしております。';

            /**
             * ここで exit すると、htmlが崩れる
             */
            // exit();
        }
        ?>

        修正しました。<br/>
        <br/>
        <a href="staff_list.php">戻る</a>

    </body>
</html>

欲を言えばこうしたい

<?php
try {
    if (null != filter_input_array(INPUT_POST)) {
        $staff_code = filter_input(INPUT_POST, 'code');
        $staff_name = filter_input(INPUT_POST, 'name');
        $staff_pass = filter_input(INPUT_POST, 'pass');

        $options = array(
            PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
        );

        $dsn = 'mysql:dbname=shop;host=localhost;charset=utf8;';
        $user = 'root';
        $password = '';
        $dbh = new PDO($dsn, $user, $password, $options);

        $sql = 'UPDATE mst_staff SET name=?,password=? WHERE code=?';
        $stmt = $dbh->prepare($sql);
        $data[] = $staff_name;
        $data[] = $staff_pass;
        $data[] = $staff_code;
        $stmt->execute($data);
    }
} catch (Exception $e) {
    $err = 'ただいま障害により大変ご迷惑をおかけしております。';
}
?>
<!DOCTYPE html>
<html>
    <head>
        <meta charset = "UTF-8">
        <title>タイトル</title>
    </head>
    <body>
        <?php if (isset($err)) : ?>
            <p><?= $err; ?></p>
        <?php elseif (null != filter_input_array(INPUT_POST)) : ?>
            <p>更新しました。</p>
        <?php endif; ?>
        <a href = "staff_list.php">戻る</a>
    </body>
</html>

> グローバル変数に直接アクセスしない

ご指摘ありがとございます。
このページでは{}内なのでローカル変数かと思っておりましたが、
確かに上位ページではグローバルの領域にこの変数を定義しておりま
すので、それでグローバル変数に該当するという事なのですね。

ちなみにグローバル変数に直接アクセスしないのは、プログラミング
においては当たり前のルールなのでしょうか。
始めたばかりの初心者で申し訳ございません。

ここでグローバル変数と言っているのは、$_POST['xxxx']の事です。
$_POST, だけではなく、$_SERVER や $_GET も然り。マニュアルでは「スーパーグローバル」と表現されています。
$GLOBALS もそうですが、この変数は「どこからでもアクセスできる」と同時に「どこからでも書き換えできる」性質があります。「どこからでもアクセスできる」ことはそれほど問題ないのですが、「どこからでも書き換えできる」ということが、意図せず発生した場合、バグの特定が困難になり、コードの保守性・安全性を損なう結果になってしまうので、出来る限り、ソースコードに $_POST や $_GET が登場しないように書きましょうといういわば、作法です。

また、filter_input()
関数を使って間接的に取得することで、グローバル変数に影響を与えなく出来るという事なのですね。

そうですね。同時に、

$val = (isset($_POST['key'])) ? $_POST['key'] : null;

という冗長な書き方をせずに済みます。

> SQLに渡す変数を htmlspecialchars 使うのは論外
こちらは脆弱性対策のためのサニタイジングとして入れていたのですが、
SQLに渡す変数では使ってはいけないのですね。
理由としては、変数に何らかの影響が出てしまうという事なのでしょうか。
例えば以下のようなことなのでしょうか。
http://php-archive.net/php/htmlspecialchars/

いいえ。これは全く無関係です。
単純に、SQLインジェクション対策のための関数ではないので、ここで使ってはいけないということです。
SQLインジェクション対策のために必要なことは、PDO においては、プリペアドステートメントを利用します。

とある、PHPの初心者向けの書籍のせいで、誤用が広まっている…

そもそも、これがSQLインジェクションに有用な関数なのであれば、PHPの開発チームは htmlspecialchars って関数名はつけないでしょう。おかしいと思いません?(→ 某書籍の作者さん、どうですか?)

> 文字コードは「$dsn」で「$dbh->query('SET NAMES utf8');」
DBを指定するときに文字コードを指定すべきなのですね。

今のバージョンでは、そうすべきです。古いバージョンのPHPでは、SET NAMES utf8 を使うことは許容されます。(それ以外に方法がないから)

> いらない「$dbh=null;」
こちらはSQLへの接続を閉じるために必要かと思っておりましたが不要なのでしょうか。

こちらは、接続が切れたら勝手に NULL になります。あえて書く必要はありません。

> ここで exit すると、htmlが崩れる
これはexitの後が表示されなくなってしまうという事なのでしょうか。

exit; で処理が止まるので、<html> 以下は表示されるが、</body></html> の部分は出力されなくなりますね。

<html>
<body>
...

となって、</body> や </html> がないって、気持ち悪いですよね…
html 的には、</body> や </html>は省略可能ですが…

> 欲を言えばこうしたい
こちらのソースでは、PHP処理をHTML内より抜き取り、HTMLには別のコードを追加して
おりますが、そこではエラーの場合に(
INPUT_POST)という場所から変数を持ってきて更新するという処理が書かれているのでしょうか。

if (null != filter_input_array(INPUT_POST)) {
...
}

の部分ですね。
この if 文がないと、$sql = 'UPDATE mst_staff SET name=?,password=? WHERE code=?';

以下の部分も実行され、UPDATE mst_staff SET name='',password='' WHERE code='' というSQLが実行されてしまいます。
POSTされたデータが存在しないなら、あえてSQLを実行する意味はありません。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/09/12 18:31 編集

    Kosuke_Shibuya様

    大変詳細なご回答誠にありがとうございます。
    また初心者のため、ご指摘の点を正確に理解することが出来ておらず、
    以下不明点について、記載をいたしました。

    お手数をおかけいして誠に申し訳ございませんが、
    ご確認何卒よろしくお願いいたします。

    ========================

    >グローバル変数に直接アクセスしない
    ご指摘ありがとございます。
    このページでは{}内なのでローカル変数かと思っておりましたが、確かに上位ページではグローバルの領域にこの変数を定義しておりますので、それでグローバル変数に該当するという事なのですね。

    ちなみにグローバル変数に直接アクセスしないのは、プログラミングにおいては当たり前のルールなのでしょうか。
    始めたばかりの初心者で申し訳ございません。

    また、filter_input()関数を使って間接的に取得することで、グローバル変数に影響を与えなく出来るという事なのですね。


    >SQLに渡す変数を htmlspecialchars 使うのは論外
    こちらは脆弱性対策のためのサニタイジングとして入れていたのですが、SQLに渡す変数では使ってはいけないのですね。理由としては、変数に何らかの影響が出てしまうという事なのでしょうか。例えば以下のようなことなのでしょうか。
    http://php-archive.net/php/htmlspecialchars/

    >文字コードは「$dsn」で「$dbh->query('SET NAMES utf8');」
    DBを指定するときに文字コードを指定すべきなのですね。

    >いらない「$dbh=null;」
    こちらはSQLへの接続を閉じるために必要かと思っておりましたが不要なのでしょうか。

    >ここで exit すると、htmlが崩れる
    これはexitの後が表示されなくなってしまうという事なのでしょうか。

    >欲を言えばこうしたい
    こちらのソースでは、PHP処理をHTML内より抜き取り、HTMLには別のコードを追加しておりますが、そこではエラーの場合に(INPUT_POST)という場所から変数を持ってきて更新するという処理が書かれているのでしょうか。
    ========================

    キャンセル

  • 2016/09/12 19:50

    回答に追記しました。

    キャンセル

  • 2016/09/12 22:28

    Kousuke_Shibuya様
    貴重なご指摘並びに目からウロコの数々のご指南、誠にありがとうございます。
    詳細なご回答により、ご指摘頂けた部分を理解することが出来ました。
    特に、SQLインジェクションの最も確実で根本的な対策は、プリペアードステートメントを利用することという点には自分なりにWEBを調べてみても納得いたしました。

    これからPHPをはじめとしたプログラミングを勉強したいと考えている私にとって分かりやすい書籍が頼りでしたが、それを鵜呑みにしていることの危険さを改めて感じました。
    今後は他の書籍や、皆様の様な諸先輩の方の意見も参考にさせて頂き、自分のスキルを上げていきたく思います。

    この度は、誠にありがとうございました。

    キャンセル

+1

Noticeは致命的なエラーではありませんが、不適切な表現ということです
error_reportingにてE_NOTICE
端的にいえば

$staff_name=$_POST['name'];
と記載する場合nameというパラメータを必ずpostしなくてはなりません。
もし値がない場合が想定されるのであれば

$staff_name=isset($_POST['name'])?$_POST['name']:"";


のような処理をいれてください
場合によってはfilter_inputで処理してもよいかと思います

$staff_name=filter_input(INPUT_POST,'name');


filter_inputすればデフォルト値の設定や想定値との整合性のバリデートなど可能です。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/09/12 18:49 編集

    yambejp様

    $staff_name=isset($_POST['name'])?$_POST['name']:"";
    こちらに変更することで、エラー表示自体は表示されなくなりました。filter_inputはかなり便利なコードなのですね。
    非常に勉強になりました。ありがとうございます。

    ただ、このままの処理では「name」自体が空で、DBにはパスワードしか反映されていないようでした。
    (パスワードも暗号化?されている為、変更されているかは確認できませんでしたが。)

    前のページ(以下のソース)を確認してみましたところ、formタグでは、「pass」「code」しかhiddenで表示されておりませんでしたので、新たに
    print '<input type="hidden" name="name" value="'.$staff_name.'">';
    の一文を追加したところDBへの反映も正常に完了することが出来ました。本当にありがとうございます。

    ’’’<!DOCTYPE html>
    <html>
    <head>
    <meta charset="UTF-8">
    <title>ろくまる農園</title>
    </head>
    <body>

    <?php

    $staff_code=$_POST['code'];
    $staff_name=$_POST['name'];
    $staff_pass=$_POST['pass'];
    $staff_pass2=$_POST['pass2'];

    $staff_name= htmlspecialchars($staff_name);
    $staff_pass= htmlspecialchars($staff_pass);
    $staff_pass2= htmlspecialchars($staff_pass2);

    if($staff_name=='')
    {
    print 'スタッフ名が入力されていません。<br/>';
    }
    else
    {
    print 'スタッフ名:';
    print $staff_name;
    print '<br/>';
    }

    if($staff_pass=='')
    {
    print 'パスワードが入力されていません。 <br/>';
    }

    if($staff_pass!=$staff_pass2)
    {
    print 'パスワードが一致しません。<br/>';
    }

    if($staff_name==''||$staff_pass=''||$staff_pass!=$staff_pass2)
    {
    print '<form>';
    print '<input type="button" onclick="history.back()" value="戻る">';
    print '<form/>';
    }
    else
    {
    $staff_pass=md5($staff_pass);
    print '<form method="post" action="staff_edit_done.php">';
    print '<input type="hidden" name="code" value="'.$staff_code.'">';
    print '<input type="hidden" name="pass" value="'.$staff_pass.'">';
    print '<br/>';
    print '<input type="button" onclick="history.back()" value="戻る">';
    print '<input type="submit" value="OK">';
    print '</form>';
    }

    ?>

    </body>
    </html>’’’

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 87.78%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る