First your code looks rather clean, and easy to read: that's already an important point.
Then an possible improvement I immediately notice is that, because processing of the query parameters is quite similar for all of them, you can dramatically reduce your code, both in control part and in HTML part.
To do so, first simply register your query parameters:
$params = [
'first-name' => NULL,
'last-name' => NULL,
'email' => NULL,
'message' => NULL,
];
Then the four successive occurrences of primary control part becomes:
foreach ($params as $key => $value) {
if (!empty(trim($_POST[$key]))) {
$params[$key] = htmlspecialchars(stripslashes(trim($_POST[$key])));
}
}
Above you can see that:
- here we don't any more generate an error message: seems not useful to do it now, will be done in HTML part so we don't use supplemental variables for this.
- since this is now a function it can include what you're previously doing in
test_input()
.
- but we added the
trim()
early to the !empty($_POST[$key])
, because if the initial content was one or more space(s) it wouldn't be seen as empty!
Once each parameter is controlled using either way above, now we can also reduce the way we accept or not to use the whole input, like this:
if (array_filter($params) == $params) {
foreach ($params as $key => $value) {
$_SESSION[$key] = $value;
}
header("Location: contact9SessionsCRSF2.php");
exit;
}
Here we take advantage of how array_filter()
works when no callback function is provided, simply dropping "not-true" items.
Last point regarding this control part, I don't understand why you generate token
and token-time
twice: once (always) just before controlling, and another time if control failed.
So I'm giving it up in this complete version of the control part (you can re-establish it if there is a good reason):
session_start(); //allows use of session variables
if ($_SERVER["REQUEST_METHOD"] == "POST"
&& !empty($_SESSION['token'])
&& $_SESSION['token'] == $_POST['token']) {
$token = md5(uniqid(mt_rand(), true));
$_SESSION['token'] = $token;
$_SESSION['token_time'] = time();
$params = [
'first-name' => NULL,
'last-name' => NULL,
'email' => NULL,
'message' => NULL,
];
foreach ($params as $key => $value) {
if (!empty(trim($_POST[$key]))) {
$params[$key] = htmlspecialchars(stripslashes($_POST[$key]));
}
}
if (array_filter($params) == $params) {
foreach ($params as $key => $value) {
$_SESSION[$key] = $value;
}
header("Location: contact9SessionsCRSF2.php");
exit;
}
}
Then we may do something pretty equivalent for the HTML part (only the main-container
content changes):
<div id="main-container" class="col-xs-12">
<form class="form-horizontal" method="post" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]);?>">
<input type="hidden" name="token" value="<?php echo $token; ?>" />
<?php
$param_names = [
'first-name' => 'First Name',
'last-name' => 'Last name',
'email' => 'email',
'message' => 'message',
];
foreach ($param_names as $key => $value) {
?>
<div class="form-group">
<label class="col-sm-2 control-label"><?php echo $value; ?></label>
<div class="col-sm-10">
<input class="form-control" name="<?php echo $key; ?>" id="<?php echo $key; ?>" placeholder="<?php echo $value; ?>" type="text" value="<?php echo $params[$key]; ?>">
<?php
if(!$params[$key])
?>
<span class="error">* <?php echo $value; ?> is required</span>
</div>
</div>
<div class="form-group">
<div class="col-sm-offset-2 col-sm-10">
<button type="submit" class="btn btn-default">Submit</button>
</div>
</div>
</form>
</div>