修正コード
第1回目で診断したPHPコードについて、
では、
<?php
define('DB_DSN', 'pgsql:dbname=app');
define('DB_USER', 'vagrant');
define('DB_PASS', 'pass');
/**
* PDOインスタンス生成
*
* @param string $dsn
* @param string $user
* @param string $pass
* @return \PDO
*/
function createPdo($dsn = DB_DSN, $user = DB_USER, $pass = DB_PASS) {
static $pdo = null;
if (is_null($pdo)) {
$pdo = new PDO($dsn, $user, $pass);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
return $pdo;
}
/**
* メールマガジンリスト取得
*
* @return array
*/
function selectMailMagazines() {
$pdo = createPdo();
$statement = $pdo->prepare('SELECT * FROM magazines');
$statement->execute();
$ret = $statement->fetchAll();
if (empty($ret)) {
return null;
}
$magazines = [];
foreach ($ret as $v) {
$magazines[$v['id']] = $v;
}
return $magazines;
}
/**
* HTMLタグをエスケープ
*
* @param $value
* @return string
*/
function escapeHtml($value) {
return htmlspecialchars($value, ENT_QUOTES, 'EUC-JP');
}
/**
* POST時処理
*
* @param mixed $frmEmail
* @param mixed $frmMagazines
* @param array $magazines
* @return array $errorMessages
* @throws PDOException
*/
function doPost($frmEmail, $frmMagazines, array $magazines) {
$errorMessages = [];
// 購読登録時、入力チェック
if (empty($frmEmail) && strlen($frmEmail) == 0) {
$errorMessages[] = 'メールアドレスを入力してください';
} elseif (preg_match("/^[_\.\-]/", $frmEmail) || !preg_match("/(^[0-9A-Za-z_\.\-]+)@([^_\.\-])([0-9A-Za-z_\.\-]+)([^_\.\-])$/", $frmEmail)) {
$errorMessages[] = 'メールアドレスの記入に誤りがあります';
}
// メールアドレス存在チェック
$pdo = createPdo();
$statement = $pdo->prepare('SELECT COUNT(*) FROM members WHERE lower(trim(email))=:email');
$statement->execute([':email' => $frmEmail]);
if ($statement->fetchColumn() > 0) {
$errorMessages[] = 'すでにメールマガジン購読が完了しています';
return $errorMessages;
}
// 購読メールマガジン入力チェック
if (!is_array($frmMagazines)) {
$errorMessages[] = '購読するメールマガジンを選択してください';
} else {
foreach ($frmMagazines as $id) {
if (!isset($magazines[$id])) {
$errorMessages[] = '購読するメールマガジンを選択してください';
break;
}
}
}
if (!empty($errorMessages)) {
return $errorMessages;
}
// 登録処理
try {
$now = new DateTime();
$pdo->beginTransaction();
$statement = $pdo->prepare('INSERT INTO members(email, created) VALUES(?, ?)');
$statement->execute([$frmEmail, $now->format('Y-m-d H:i:s')]);
$memberId = $pdo->lastInsertId('members_id_seq');
$statement = $pdo->prepare('INSERT INTO member_magazines(member_id, magazine_id) VALUES(?, ?)');
foreach ($frmMagazines as $magazineId) {
$statement->execute([$memberId, $magazineId]);
}
$pdo->commit();
} catch (\PDOException $e) {
$pdo->rollback();
$errorMessages[] = 'system error';
}
return $errorMessages;
}
/**
* メイン処理
*/
$errorMessages = [];
$isComplete = false;
$frmEmail = null;
$frmMagazines = null;
$magazines = selectMailMagazines();
if (empty($magazines)) {
$errorMessages[] = '購読できるメールマガジンがありません';
}
if (!empty($_POST)) {
if (array_key_exists('frm_email', $_POST)) {
$frmEmail = $_POST['frm_email'];
}
if (array_key_exists('frm_magazines', $_POST)) {
$frmMagazines = $_POST['frm_magazines'];
}
$errorMessages = doPost($frmEmail, $frmMagazines, $magazines);
if (empty($errorMessages)) {
$isComplete = true;
}
}
header('Content-Type: text/html; charset=EUC-JP');
?>
<html>
<head>
<meta HTTP-EQUIV="CONTENT-TYPE" CONTENT="text/html;CHARSET=EUC-JP">
<title></title>
</head>
<body>
<div align="center">
<form method="POST" action="">
<?php if (!empty($errorMessages)): ?>
<br>
<div align="center">
<table border="0" cellpadding="5">
<tr>
<td>
<div style="color: #ff0000">
<?php foreach($errorMessages as $message): ?>
<?php echo escapeHtml($message) ?><br>
<?php endforeach; ?>
</div>
</td>
</tr>
<tr>
<td align="center">
<input type="button" value="戻る" onclick="history.back()">
</td>
</tr>
</table>
</div>
<?php else: ?>
<table border="1" cellspacing="2" cellpadding="5" width="700">
<tr>
<td>
<?php if ($isComplete): ?>
<div align="center">
<table border="0" cellspacing="2" cellpadding="5">
<tr>
<td align="center" colspan="2" nowrap>メールマガジン購読完了</td>
</tr>
<tr>
<td align="center" nowrap>メールマガジン購読が完了しました。</td>
</tr>
</table>
</div>
<?php else: ?>
<div align="center">
<table border="0" cellspacing="2" cellpadding="5">
<tr>
<td align="center" colspan="2" nowrap>メールマガジン購読</td>
</tr>
<tr>
<td align="right" nowrap>email:</td>
<td align="left" nowrap><input type="text" name="frm_email" size=30 maxlength=100 value="<?php echo escapeHtml($frmEmail) ?>"></td>
</tr>
</table>
</div>
<?php if (!empty($magazines)): ?>
<?php if (count($magazines) == 1): ?>
<input type="hidden" name="frm_magazines[]" value="<?php echo escapeHtml($magazines[0]['id']) ?>" />
<?php else: ?>
<div>
購読希望するマガジンを選択してください。
</div>
<table border="0" cellpadding="5">
<?php foreach ($magazines as $magazine): ?>
<?php $checked = isset($frmMagazines[$magazine['id']]) ? ' checked="checked"' : ''; ?>
<tr>
<td align="left">
<input id="magazine<?php echo escapeHtml($magazine['id']) ?>" type="checkbox" name="frm_magazines[]" value="<?php echo escapeHtml($magazine['id']) ?>"<?php echo $checked ?> />
<label for="magazine<?php echo escapeHtml($magazine['id']) ?>"><?php echo escapeHtml($magazine['name']) ?></label>
</td>
</tr>
<?php endforeach; ?>
</table>
<?php endif; ?>
<?php endif; ?>
<div align="center">
<input type="submit" value="購読">
</div>
<?php endif; ?>
</td>
</tr>
</table>
<?php endif; ?>
</form>
</div>
</body>
</html>治療ポイント1. コードの分離
まず、require_文で読み込むということもやりやすくなります。
それぞれの部分について解説すると、define文を使って、
ロジック部分では、doPost()関数を実行してます。元のコードでは、switch文にて条件分岐を行い、case文の中で実際の処理を記述していたのですが、
そして、print文をやめて、
治療ポイント2. コードの簡素化
主にロジック部分になりますが、
元のコードでは、$mode変数で実行する処理を分岐していたのですが、$modeには2つの状態しかなく、$_POSTに値がない場合は前者、$mode変数自体をなくしました。
ほかには、switch文とbreak文をうまくreturn文で実装しています。
治療ポイント3. 今のPHPの書き方へ
2013年の今、
まずはDB処理について。今ではPDOpg_*関数はPDOを使った書き方に変えています。SQL文を実行する際は、
ほかには、DateTimeクラスを、array()ではなく[]
治療ポイント4. Noticeへの対応
元のコードを実行すると、
修正コードでは、define文について定義し、array_関数にてキーの存在を確認してから参照するようにしています。
治療ポイント5. セキュリティへの配慮
SQLインジェクションやクロスサイトスクリプティング 第1回と第2回では、 このコードはある意味すごいです。関数にまとめたり、 修正コードはあくまで一例で、 初めての診断はいかがだったでしょうか。今後もこのような流れで、htmlspecialchars()関数によるエスケープを行っています。htmlspecialchars()関数の実行はビューの中では何度となく出てくるので、escapeHtml()関数にまとめてしまって、まとめ