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

現在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'];」
になります。

kei344

2016/09/12 07:09

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

kotan0

2016/09/12 08:48

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

行動規範の内容に同意します

回答2件

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

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

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

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

PHP
1$staff_name=filter_input(INPUT_POST,'name');
2

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

投稿2016/09/12 06:33

yambejp

総合スコア114784

kotan0

2016/09/12 09: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>’’’

行動規範の内容に同意します

ベストアンサー

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

php
1<!DOCTYPE html>
2<html>
3	<head>
4		<meta charset="UTF-8">
5		<title>タイトル</title>
6	</head>
7	<body>
8
9		<?php
10		try {
11
12			/*
13			 * グローバル変数に直接アクセスしない
14			 * 
15			  $staff_code = $_POST['code'];
16			  $staff_name = $_POST['name'];
17			  $staff_pass = $_POST['pass'];
18			 */
19
20			$staff_code = filter_input(INPUT_POST, 'code');
21			$staff_name = filter_input(INPUT_POST, 'name');
22			$staff_pass = filter_input(INPUT_POST, 'pass');
23
24			/*
25			 *  SQLに渡す変数を htmlspecialchars 使うのは論外
26			 *
27			  $staff_name = htmlspecialchars($staff_name);
28			  $staff_pass = htmlspecialchars($staff_pass);
29			 */
30
31
32			/**
33			 * try 〜 catch でエラーを細くするためには、以下の設定が必要
34			 */
35			$options = array(
36				PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
37			);
38
39			$dsn = 'mysql:dbname=shop;host=localhost;charset=utf8;';
40			$user = 'root';
41			$password = '';
42			$dbh = new PDO($dsn, $user, $password, $options);
43
44			/*
45			 * 文字コードは $dsn で
46			  $dbh->query('SET NAMES utf8');
47			 */
48
49			$sql = 'UPDATE mst_staff SET name=?,password=? WHERE code=?';
50			$stmt = $dbh->prepare($sql);
51			$data[] = $staff_name;
52			$data[] = $staff_pass;
53			$data[] = $staff_code;
54			$stmt->execute($data);
55
56			/**
57			 * いらない
58			  $dbh = null;
59			 */
60		} catch (Exception $e) {
61			print 'ただいま障害により大変ご迷惑をおかけしております。';
62
63			/**
64			 * ここで exit すると、htmlが崩れる
65			 */
66			// exit();
67		}
68		?>
69
70		修正しました。<br/>
71		<br/>
72		<a href="staff_list.php">戻る</a>
73
74	</body>
75</html>

欲を言えばこうしたい

php
1<?php
2try {
3	if (null != filter_input_array(INPUT_POST)) {
4		$staff_code = filter_input(INPUT_POST, 'code');
5		$staff_name = filter_input(INPUT_POST, 'name');
6		$staff_pass = filter_input(INPUT_POST, 'pass');
7
8		$options = array(
9			PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
10		);
11
12		$dsn = 'mysql:dbname=shop;host=localhost;charset=utf8;';
13		$user = 'root';
14		$password = '';
15		$dbh = new PDO($dsn, $user, $password, $options);
16
17		$sql = 'UPDATE mst_staff SET name=?,password=? WHERE code=?';
18		$stmt = $dbh->prepare($sql);
19		$data[] = $staff_name;
20		$data[] = $staff_pass;
21		$data[] = $staff_code;
22		$stmt->execute($data);
23	}
24} catch (Exception $e) {
25	$err = 'ただいま障害により大変ご迷惑をおかけしております。';
26}
27?>
28<!DOCTYPE html>
29<html>
30	<head>
31		<meta charset = "UTF-8">
32		<title>タイトル</title>
33	</head>
34	<body>
35		<?php if (isset($err)) : ?>
36			<p><?= $err; ?></p>
37		<?php elseif (null != filter_input_array(INPUT_POST)) : ?>
38			<p>更新しました。</p>
39		<?php endif; ?>
40		<a href = "staff_list.php">戻る</a>
41	</body>
42</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
1<html>
2<body>
3...
4

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

欲を言えばこうしたい

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

php
1if (null != filter_input_array(INPUT_POST)) {
2...
3}

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

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

投稿2016/09/12 06:23

編集2016/09/12 10:26

退会済みユーザー

総合スコア0

kotan0

2016/09/12 09:32 編集

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 10:50

回答に追記しました。

kotan0

2016/09/12 13:28

Kousuke_Shibuya様貴重なご指摘並びに目からウロコの数々のご指南、誠にありがとうございます。詳細なご回答により、ご指摘頂けた部分を理解することが出来ました。特に、SQLインジェクションの最も確実で根本的な対策は、プリペアードステートメントを利用することという点には自分なりにWEBを調べてみても納得いたしました。これからPHPをはじめとしたプログラミングを勉強したいと考えている私にとって分かりやすい書籍が頼りでしたが、それを鵜呑みにしていることの危険さを改めて感じました。今後は他の書籍や、皆様の様な諸先輩の方の意見も参考にさせて頂き、自分のスキルを上げていきたく思います。この度は、誠にありがとうございました。

行動規範の内容に同意します

あなたの回答