PHP comment system

Question

Basics

I created a simple comment system. My goal was it to create a system that can easily be used on everyone's server without having to install a load of programs. I also tried to create it as privacy-friendly as possible (no email-address, no cookies). I also need to solve this problem without databases.

Functionality

• Basic form to submit new comments
• Flag-functionality (with simple email send to the owner of the website)
• Answer functionality with indented answers

Code

simpleComments.php

This script provides the main functionality: Spam-protection (with suggestions from here and here), sending, answering and flagging comments. I think that especially the function save() looks is a rather hacky solution. If you know a better alternative (without databases), I would be happy to hear it.

//The password for the AES-Encryption (has to be length=16) $encryptionPassword = "****************"; //============================================================================================ //============================================================================================ // == // FROM HERE ON NO ADJUSTMENT NECESSARY == // == //============================================================================================ //============================================================================================ /** * Creates image * * This function creates a black image with the random exercise created by randText() on it. * Additionally the function adds some random lines to make it more difficult for bots to read * the text via OCR. The result (for example) looks like this: https://imgur.com/a/6imIE73 * * @author Philipp Wilhelm * * @since 1.0 * * @param string $rand Random exercise created by randText() * @param int $width Width of the image (default = 200) * @param int $height Height of the image (default = 50) * @param int $textColorRed R-RGB value for the textcolor (0-255) (default = 255) * @param int $textColorGreen G-RGB value for the textcolor (0-255) (default = 255) * @param int $textColorBlue B-RGB value for the textcolor (0-255) (default = 255) * @param int $linesColorRed R-RGB value for the random lines (0-255) (default = 192) * @param int $linesColorGreen G-RGB value for the random lines (0-255) (default = 192) * @param int $linesColorBlue B-RGB value for the random lines (0-255) (default = 192) * @param int $fontSize font size of the text on the image (1-5) (default = 5) * @param int $upperLeftCornerX x-coordinate of upper-left corner of the first char (default = 18) * @param int $upperLeftCornerY y-coordinate of the upper-left corner of the first char (default = 18) * @param int $angle angle the text will be rotated by (default = 10) * * @return string created image surrounded by <img> */ function randExer($rand, $width = 200, $height = 50, $textColorRed = 255, $textColorGreen = 255, $textColorBlue = 255, $linesColorRed = 192, $linesColorGreen = 192, $linesColorBlue = 192, $fontSize = 5, $upperLeftCornerX = 18, $upperLeftCornerY = 18, $angle = 10) { global $encryptionPassword; $random = openssl_decrypt($rand,"AES-128-ECB", $encryptionPassword); $random = substr($random, 0, -40); //Creates a black picture $img = imagecreatetruecolor($width, $height); //uses RGB-values to create a useable color $textColor = imagecolorallocate($img, $textColorRed, $textColorGreen, $textColorBlue); $linesColor = imagecolorallocate($img, $linesColorRed, $linesColorGreen, $linesColorBlue); //Adds text imagestring($img, $fontSize, $upperLeftCornerX, $upperLeftCornerY, $random . " = ?", $textColor); //Adds random lines to the images for($i = 0; $i < 5; $i++) { imagesetthickness($img, rand(1, 3)); $x1 = rand(0, $width / 2); $y1 = rand(0, $height / 2); $x2 = $x1 + rand(0, $width / 2); $y2 = $y1 + rand(0, $height / 2); imageline($img, $x1, $x2, $x2, $y2, $linesColor); } $rotate = imagerotate($img, $angle, 0); //Attribution: https://stackoverflow.com/a/22266437/13634030 ob_start(); imagejpeg($rotate); $contents = ob_get_contents(); ob_end_clean(); $imageData = base64_encode($contents); $src = "data:" . mime_content_type($contents) . ";base64," . $imageData; return "<img alt='' src='" . $src . "'/>"; }; /** * Returns time stamp * * This function returns the current time stamp, encrypted with AES, by using the standard function time(). * * @author Philipp Wilhelm * * @since 1.0 * * @return int time stamp */ function getTime() { global $encryptionPassword; return openssl_encrypt(time() . bin2hex(random_bytes(20)),"AES-128-ECB", $encryptionPassword); } /** * Creates random exercise * * This function creates a random simple math-problem, by choosing two random numbers between "zero" and "ten". * The result looks like this: "three + seven" * * @author Philipp Wilhelm * * @since 1.0 * * @return string random exercise */ function randText() { global $encryptionPassword; //Creating random (simple) math problem $arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"); $item1 = $arr[array_rand($arr)]; $item2 = $arr[array_rand($arr)]; $random = $item1 . " + " . $item2; $encrypted = openssl_encrypt($random . bin2hex(random_bytes(20)),"AES-128-ECB", $encryptionPassword); return $encrypted; } /** * flags comment * * This function sends an email to the specified adress containing the id of the flagged comment * * @author Philipp Wilhelm * * @since 1.0 * * @param string $to Email-adress the mail will be send to * @param string $url URL of the site the comment was flagged on * */ function flag($to, $url) { //Which comment was flagged? $id = $_POST["comment"]; //At what side was the comment flagged? $referer = $_SERVER["HTTP_REFERER"]; $subject = "FLAG"; $body = $id . " was flagged at " . $referer . "."; //Send the mail mail($to, $subject, $body); //Redirect to what page after flag? //(In this case to the same page) header("Location:" . $url); exit(); } /** * redirects to the same page, but with the added parameter to specify to which * comment will be answered and jumps right to the comment-form * * * @author Philipp Wilhelm * * @since 1.0 * * @param string $url the url of the current page * @param string $buttonName URL of the site the comment was flagged on * @param string $urlName the "id-name" * */ function answer($url, $buttonName, $urlName) { header("Location:" . $url . "?" . $urlName . "=" . $_POST["comment"] . "#" . $buttonName); exit(); } /** * error message * * Redirects to the specified url to tell the user that something went wrong * e.g. entered wrong solution to math-exercise * * @author Philipp Wilhelm * * @since 1.0 * * @param string $urlError The specified url * */ function error($urlError) { header("Location:" . $urlError); die(); } /** * Redirects to specified url when user enters words that are on the "blacklist" * * @author Philipp Wilhelm * * @since 1.0 * * @param string $urlBadWords The specified url to which will be redirected * */ function badWords($urlBadWords) { header("Location:" . $urlBadWords); die(); } /** * Redirects to same url after comment is successfully submitted - comment will be visible * immediately * * @author Philipp Wilhelm * * @since 1.0 * * @param string $url URL of the site * */ function success($url) { header("Location:" . $url); die(); } /** * checks if user enters any words that are on the "blacklist" * * @author Philipp Wilhelm * * @since 1.0 * * @param string $text The user-entered text * @param string $blackList filename of the "blacklist" * * @return boolean true if user entered a word that is on the "blacklist" * */ function isForbidden($text, $blackList) { //gets content of the blacklist-file $content = file_get_contents($blackList); $text = strtolower($text); //Creates an array with all the words from the blacklist $explode = explode(",", $content); foreach($explode as &$value) { //Pattern checks for whole words only ('hell' in 'hello' will not count) $pattern = sprintf("/\b(%s)\b/",$value); if(preg_match($pattern, $text) == 1) { return true; } } return false; } /** * saves a new comment or an answer to a comment * * @author Philipp Wilhelm * * @since 1.0 * * @param string $url Email-adress the mail will be send to * @param string $urlError URL to the "error"-page * @param string $urlBadWords URL to redirect to , when user uses words on the "blacklist" * @param string $blacklist filename of the blacklist * @param string $fileName filename of the file the comments are stored in * @param string $nameInputTagName name of the input-field for the "name" * @param string $messageInputTagName name of the input-field for the "message" * @param string exerciseInputTagName name of the input-field the math-problem is stored in * @param string solutionInputTagName name of the input-field the user enters the solution in * @param string $answerInputTagName in this field the id of the comment the user answers to is saved * (if answering to a question) * @param string $timeInputTagName name of the input-field the timestamp is stored in * */ function save($url, $urlError, $urlBadWords, $blacklist, $fileName, $nameInputTagName, $messageInputTagName, $exerciseInputTagName, $solutionInputTagName, $answerInputTagName, $timeInputTagName) { global $encryptionPassword; $solution = filter_input(INPUT_POST, $solutionInputTagName, FILTER_VALIDATE_INT); $exerciseText = filter_input(INPUT_POST, $exerciseInputTagName); if ($solution === false || $exerciseText === false) { error($urlError); } $time = openssl_decrypt($_POST[$timeInputTagName], "AES-128-ECB", $encryptionPassword); if(!$time) { error($urlError); } $time = substr($time, 0, -40); $t = intval($time); if(time() - $t > 300) { error($urlError); } //Get simple math-problem (e.g. four + six) $str = openssl_decrypt($_POST[$exerciseInputTagName], "AES-128-ECB", $encryptionPassword); $str = substr($str, 0, -40); if (!$str) { error($urlError); } $arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"); //gets array with written numbers $words = array_map("trim", explode("+", $str)); //gets the numbers as ints $numbers = array_intersect($arr, $words); if (count($numbers) != 2) { error($urlError); } $sum = array_sum(array_keys($numbers)); $urlPicture = "identicon.php/?size=24&hash=" . md5($_POST[$nameInputTagName]); //Did user enter right solution? if ($solution == $sum) { $name = $_POST[$nameInputTagName]; $comment = htmlspecialchars($_POST[$messageInputTagName]); $content = file_get_contents($fileName); if(strcmp($content, "<p>No comments yet!</p>") == 0 || strcmp($content, "<p>No comments yet!</p>\n") == 0) { $content = "<p>Identicons created with <a href='https://github.com/timovn/identicon'>identicon.php</a> (licensed under <a href='http://www.gnu.org/licenses/gpl-3.0.en.html'>GPL-3.0</a>).</p>"; } $id = bin2hex(random_bytes(20)); $answerID = $_POST[$answerInputTagName]; //Checks if user used any words from the blacklist if(isForbidden($comment, $blacklist)) { badWords($urlBadWords); } //Case the user writes a new comment (not an answer) if(strlen($answerID) < 40) { file_put_contents($fileName, //Needed styles "<style>" . ".commentBox {" . "display: block;" . "background: LightGray;" . "width: 90%;" . "border-radius: 10px;" . "padding: 10px;" . "margin-bottom: 5px;" . "} " . "input[name='flag'], input[name='answer'] {" . "border: none;" . "padding: 0;" . "margin: 0;" . "margin-top: 5px;" . "padding: 2px;" . "background: transparent;" . "}" . "</style>" . //get random avatar "<img class='icon' style='vertical-align:middle;' src='" . $urlPicture . "'/>" . //Displaying user name "<span><b> " . $name . "</b></span> says:<br>" . //Current UTC-time and -date "<span style='font-size: small'>" . gmdate("d-m-Y H:i") . " UTC</span><br>" . //The main comment "<div class='commentBox'>" . $comment . "<br>" . "</div>". "<div style='width: 90%; font-size: small; float: left'>" . //Flag-button "<form style='margin: 0; padding: 0; float: left;' method='POST' action='simpleComments.php'>" . "<input style='display: none;' name='comment' type='text' value='" . $id . "'/>" . "<input style='color: red;' type='submit' name='flag' value='Flag'/>" . "</form>" . //Answer-button "<form id='answer' style='margin-left: 0; padding: 0; float: left;' method='POST' action='simpleComments.php'>" . "<input style='display: none;' name='comment' type='text' value='" . $id . "'/>" . "<input style='color: green;' type='submit' name='answer' value='Answer'/>" . "</form>" . "<!-- " . $id . " -->" . "</div>" . "<br><br>" . $content); success($url); } //Case that user writes an answer else { if(strpos($content, $answerID) !== false) { $explode = explode("<!-- " . $answerID . " -->", $content); file_put_contents($fileName, $explode[0] . "</div>" . "<br><br>" . //Needed styles "<style>" . ".answerBox {" . "display: block;" . "background: LightGray;" . "width: 90%;" . "border-radius: 10px;" . "padding: 10px;" . "margin-bottom: 5px;" . "} " . "input[name='flag'] {" . "border: none;" . "padding: 0;" . "margin: 0;" . "margin-top: 5px;" . "padding: 2px;" . "background: transparent;" . "}" . "</style>" . "<div style='margin-left: 50px'>" . //get random avatar "<img class='icon' style='vertical-align:middle;' src='" . $urlPicture . "'/>" . //Displaying user name "<span><b> " . $name . "</b></span> says:<br>" . //Current UTC-time and -date "<span style='font-size: small'>" . gmdate("d-m-Y H:i") . " UTC</span><br>" . //The main comment "<div class='answerBox'>" . $comment . "<br>" . "</div>". //Flag-button "<div style='width: 90%; font-size: small; float: left'>" . "<form style='margin: 0; padding: 0; float: left;' method='POST' action='simpleComments.php'>" . "<input style='display: none;' name='comment' type='text' value='" . $id . "'/>" . "<input style='color: red;' type='submit' name='flag' value='Flag'/>" . "</form><br><br>" . "</div>" . "<!-- " . $answerID . " -->" . $explode[1]); success($url); } } } error($urlError); } //============================================================================================ //============================================================================================ // == // FROM HERE ON ADJUSTMENT ARE NECESSARY == // == //============================================================================================ //============================================================================================ /** * start point of the script * * @author Philipp Wilhelm * * @since 1.0 * * */ function start() { //To what email-adress should the flag-notification be send? $to = "[email protected]"; //What's the url you are using this system for? (exact link to e.g. the blog-post) $url = "https://example.com/post001.html"; //Which page should be loaded when something goes wrong? $urlError = "https://example.com/messageError.html"; //What page should be loaded when user submits words from your "blacklist"? $urlBadWords = "https://example.com/badWords.html"; //In which file are the comments saved? $fileName = "testComments.php"; //What's the filename of your "blacklist"? $blackList = "blacklist.txt"; //Replace with the name-attribute of the respective input-field //No action needed here, if you didn't update form.php $nameInputTagName = "myName"; $messageInputTagName = "myMessage"; $exerciseInputTagName = "exerciseText"; $solutionInputTagName = "solution"; $answerInputTagName = "answerID"; $timeInputTagName = "time"; $buttonName = "postComment"; $urlName = "id"; if (isset($_POST["flag"])) { flag($to, $url); } if (isset($_POST["answer"])) { answer($url, $buttonName, $urlName); } if (isset($_POST[$buttonName])) { save($url, $urlError, $urlBadWords, $blackList, $fileName, $nameInputTagName, $messageInputTagName, $exerciseInputTagName, $solutionInputTagName, $answerInputTagName, $timeInputTagName); } } start(); ?> 

The code was checked with phpcodechecker.com and it didn't find any problems.

The other files are not really worth reviewing, so I will leave it here.

Links

For those who are nevertheless interested in the other files and a how-to, please see the repository for this project.

There also is a live-demo for those of you who want to test it.

Question

Every suggestions are welcome. As mentioned before, I would be especially interested in a more elegant solution for the save()-function.

Answer 1

Initial Feedback

I like the usage of docblocks above the functions. The save() function makes good use of returning early to limit indentation levels, except for the last check - when $solution does not match $sum then it can call error() right away. Overall that function is quite lengthy - it violates the Single Responsibility Principle. The functionality to write to the file could be moved to separate functions for each case (comment vs answer). The stylesheets can be moved out to a CSS file(s).

Like I mentioned in this answer CSRF tokens could replace the need for the image creation, encoding and decoding.

Suggestions

Global variables

As others have suggested, global variables have more negative aspects than positives. You could pass the encryption password to each function that needs it, but that would require updating the signature of each function that needs it. Another option is to create a named constant using define().

define('ENCRYPTION_PASSWORD', 'xyz'); 

This could be done in a separate file that is included via include() (or include_once()) or require() (or require_once()), which could be separate from version control (e.g. a .env file)

Constants can also be created using the const keyword - outside of a class as of PHP 5.3.0¹.

const ENCRYPTION_PASSWORD = 'xyz'; 

As was already suggested, using a class with a namespace is a great idea. A class would allow the use of a class constant that would be namespaced to the class and have a specific visibility as of PHP 7.1².

Hopefully your code is running on PHP 7.2 or later, since those versions are officially supported³.

Iteration by reference

The function isForbidden iterates over the contents of the file pointed to in $blacklist assigning the value by reference:

 foreach($explode as &$value) { 

This seems unnecessary because $value is not modified within the loop. It might be best to avoid such a practice unless you are certain the array elements need to be modified.

Strict equality

You may have heard this already: it is a good habit to use strict comparison operators - i.e. === and !== when possible - e.g. for this comparison within save():

if (count($numbers) != 2) { 

count() returns an int and 2 is an int so !== can be used as there is no need for type conversion.

Hidden inputs

The HTML generated for the forms contains:

<input style='display: none;' 

This could be simplified slightly using the hidden input type:

<input type="hidden" 

While any input could be displayed by the user by modifying the page via browser console or other means, the hidden input was created for the purpose of hiding form values.

Answer 2

This is just a single suggestion, and not a complete review of your code.

Most comment systems are used for commenting on something that is not itself a comment. Like in your demo page. This means that your code will be included in someone else's page. This might be a very complex page. In other words, your code will have to live alongside, a possibly endless variety of, other code. What if that code includes functions like getTime(), error() or save()? Then your code will break that page.

This is why, code we want to share with other developers, is almost always written in an object oriented programming (OOP) style. Objects, and name spaces, are used isolate your code from the code of those who use your code.

Some links:

https://phpenthusiast.com/object-oriented-php-tutorials

https://www.thoughtfulcode.com/a-complete-guide-to-php-namespaces

https://phptherightway.com

Even if you leave your code, as it is now, I would advise to be more creative with the names you choose. For instance, the function name randExer() means nothing to me. A better name would be something like getCaptchaImageHtml(). This name actually describes what the function does and what it returns. The same applies to the other functions. It is my opinion that uncommon abbreviation should be avoided in function names.

Answer 3

Avoid using global variables.

The variable $encryptionPassword should be passed as an argument to all functions that need it (randExer, getTime, randText and save).

There are several reasons for doing so:

• You avoid conflicts with other global variables (not that there should be any)
• You avoid accidental access to that variable by those who should not access it.
• Signatures of the functions hide the dependency on an encryption password, which makes it harder to understand the code by readers, because they have to read the function body to understand that there is such a dependency. Reading just the function's signature should be enough.
• The functions are easier to test
• and probably more...

A function that uses a global variable cannot be pure by definition. Pure functions are generally prefered for the reasons mentioned above.

EDIT: A possible way to solve the global variable problem is to promote the functions to class methods and pass the encryption password to its constructor:

class ASuitableClassName { private string $encryptionPassword; public function __construct(string $encryptionPassword) { $this->encryptionPassword = $encryptionPassword; } public function getTime() { return openssl_encrypt(time() . bin2hex(random_bytes(20)),"AES-128-ECB", $this->encryptionPassword); } // .... } $obj = new ASuitableClassName("****************"); $obj->getTime();