This is a sample, bare-bones form where people will go to leave a comment. It’s quite unadorned at the moment. Just wondering if it will be secure.
And, I don’t know if the regex is correct for what I want: “Comments can only contain letters, numbers, commas, periods, and white spaces.”
<?php
// if(isset($_POST['submit'])){
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
$to = "my@email.com";
$subject= "Feedback";
$body = trim(htmlspecialchars($_POST['body']));
$body = validate($_POST['body']);
if (!preg_match('/^.*,*.\\.$\W\s', $body)) {
echo "Comments can only contain letters, numbers, commas, periods, and white spaces.";
}
$body = "Message: " . $body;
if(mail($to, $subject, $body)){
echo "Thank you - Your feedback was sent to me. I can't wait to read it!";
}else{
echo "Sorry, something went wrong with sending your comments.";
}
}
?>
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Feedback</title>
</head>
<body>
<div id="wrapper">
<form class="form" method="POST" enctype="text/plain">
<h1>Feedback Form</h1>
<p>For your security, please do not include your email or other private info. If you want to email us, please use the email on the website.</p>
<p><em>Comments can only contain letters, numbers, commas, periods, and white spaces.</em></p>
<textarea name="body"></textarea>
<button type="submit" name="submit">Submit</button>
</form>
</div>
<!--
https://blog.sqreen.com/top-10-security-best-practices-for-php/
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Cheatsheet
-->
</body>
</html>
I don’t think you need to be running your body through htmlspecialchars before you validate it. Remember that htmlspecialchars is primarily to convert special characters to HTML entities so that they can then be displayed safely to the user in an HTML document. You can accept what the user inputs as is, validate that and only when you go to print it out to the user then run it through that function. Remember sanitize input and escape output. htmlspecialchars and its related htmlentities are for escaping output. TIP: Make sure to always specify an encoding with those functions.
If you take out that usage of htmlspecialchars, your validate function will probably be a lot easier to implement and your preg_match can be a lot simpler to implement as well.
I feel that because you are encoding early, it is going to make your validation and preg_matching much more error prone than they need to be. Maybe consider some of this and see if it makes things a bit more straight forward.
w3schools advocates using htmlspecialchars (here).
I have switched over to using FILTER_SANITIZE_STRING (reference) which completely strips out any HTML tags. I feel this is preferable, especially where data from a form is being forwarded to the email address of a client.
Either way, I question whether a preg_match is necessary.
Hey John… it really depends on the type of email you are sending. As you probably know, you can send HTML email or plain text email. If you are sending HTML format, you can certainly use the nl2br method of converting them to <br/> tags and be fine. If you are using plain text, you obviously don’t want to be sending any HTML tags. Plain text is actually pretty safe for the most part as nothing is really executed on the client side. The only trick is your formatting can be less than savory unless you can make sure linefeeds are in the right places.
Think of it like sending a typical string variable. You can print a string as plain text without issue. Things like < and > will show up just as those characters, not rendered as HTML tags. So I don’t think you need to do anything special with the text other than making sure your linefeeds are where you want them.
HTML email of course has a little more involvement with security and which is why in those cases you should be using htmlspecialchars etc. But as I was saying to the original poster, you don’t need to use that function until you actually go to print the HTML (or in this case, send the HTML email content).
NO, NO, NO, Absolutely not! Throw that function in the trash. It is an old leftover relic from the 90’s and is always misused on top of being incorrectly written.
Don’t assume that just because it’s posted somewhere, it’s necessarily the best way. I post on stackoverflow, for example, doesn’t mean I know what I’m talking about.
I used this function for outputing data for years didnt have a problem.
but when inserting data need to use filters or entities etc…
htmlspecialchars — Convert special characters to HTML entities.
stripslashes – Un-quotes a quoted string.
trim — Strip whitespace (or other characters) from the beginning and end of a string.
I think if anyone pass them, then you have nothing to do
Technically no, you can pretty much name a function anything you want but would you really want to name something “db_connection” that just validates an email? So yes, it does matter.
In this case it is outputting, but “everywhere” you see this function being used is on input to a database. Here are stackoverflow examples of exactly this kind of misuse.
Think of how often you write code and how often you read code. In general, you read way more than write. Thus, if you use a little bit of time to think of proper names while writing the code, you can save lots of time when reading the code and don’t have to stop all the time and go “huh?”.
Write your code as if the developer that will replace you is a madman with a chainsaw that knows where you live.
enctype has to do with how the values of the form will be sent and has nothing to do with how the form is presented to the browser by the user seeing the form. You use htmlspecialchars when it comes to the form HTML being rendered to the user, not how the user sends form data. Two different things here.
