1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

Flat File Comments Script

Discussion in 'PHP' started by medialab, Feb 2, 2016.

  1. #1
    Hey Everyone,
    SEMrush
    I have written a little flat file commenting system and everything works great! It get's the post ID from the page, and any comments tagged with that ID are displayed. The form works perfectly as well.

    I was wondering how safe it was though and if there is a way to make it so that people can't use it to hack into my website. I know there's no real issue when adding a comment to the text file, but I am worried about displaying the comment back on the page. There's no approval process so once a comment is made it shows up.

    I used PHP strip_tags to remove code from the 3 fields but I am not sure if that is enough. Any hep would be much appreciated!

        <!-- ADD COMMENT -->
        <?php
       
        if (isset($_POST['submit'])) {
       
            // CHECK FOR EMPTY FIELDS
            $required = array('id', 'date', 'name', 'email', 'comment');
            $error = false;
           
            foreach($required as $field) {
                if (empty($_POST[$field])) {
                $error = true;
                }
            }
           
            if ($error) {
                    $name = $_POST['name'];
                    $email = $_POST['email'];
                    $comment = $_POST['comment'];
                    echo 'All Fields Are Required';
                } else {
                    $id = $postid;
                    $date = $_POST['date'];
                    $name = strip_tags($_POST['name']);
                    $email = strip_tags($_POST['email']);
                    $comment = strip_tags($_POST['comment']);
                    $fp = fopen("comments.txt","a");
       
                    // WRITE COMMENT TO FILE
                    if (@fwrite($fp, $id.'|'.$date.'|'.$name.'|'.$email.'|'.$comment."\n")) {
                        echo 'Your Comment Was Added';
                        $name = "";
                        $email = "";
                        $comment = "";
                        fclose($fp);
                    } else {
                        echo 'Your Comment Was Not Added';
                        fclose($fp);
                }
            }
        }
       
        ?>
       
        <div class="clear"></div>
    
        <!-- COMMENT FORM -->
        <form name="input" method="post" action="<?php echo $_SERVER['SCRIPT_NAME']?>">
        <input type="hidden" name="id" id="id" value="<?php echo $postid ?>">
        <input type="hidden" name="date" id="date" value="<?php echo date('F j, Y'); ?>">
        <input type="text" class="comment-name" name="name" id="name" placeholder="Your Name" value="<?php echo $name ?>">
        <input type="text" class="comment-email" name="email" id="email" placeholder="Email Address" value="<?php echo $email ?>">
        <textarea name="comment" id="comment" placeholder="What's Your Question or Comment?"><?php echo $comment ?></textarea>
        <input class="button" type="submit" name="submit" value="Ask A Question &middot; Comment">
        </form>
       
        <div class="clear"></div>
       
        <!-- DISPLAY COMMENTS -->
        <?php
       
        $file = fopen("comments.txt", "r");
       
        // WRITE EACH COMMENT
        while (!feof($file) ) {
        $line_of_text = fgets($file);
        $parts = explode('|', $line_of_text);
            if ($parts[0] == $postid) {
            print "<div class='name-comment'>" . $parts[2] . " Said:</div><div class='text-comment'>" . $parts[4] . "</div><div class='date-comment'>" . $parts[1] . "</div>";
            }
        }
        fclose($file);
    
        ?>
    PHP:
     
    medialab, Feb 2, 2016 IP
    SEMrush
  2. ThePHPMaster

    ThePHPMaster Well-Known Member

    Messages:
    737
    Likes Received:
    51
    Best Answers:
    33
    Trophy Points:
    150
    #2
    Two things:

    1) You need to escape your inputs to remove js/css/etc.. to avoid XSS attacks.
    2) You need to understand that since you are using files, you might have lock/race issues if 2 people submit a comment at the same time - not a major issue on low traffic websites.
     
    ThePHPMaster, Feb 2, 2016 IP
    deathshadow and NetStar like this.
  3. NetStar

    NetStar Notable Member

    Messages:
    2,290
    Likes Received:
    472
    Best Answers:
    21
    Trophy Points:
    215
    #3
    It's still an issue... because all it takes is two people submitting a comment at once which can be very likely big or small.
     
    NetStar, Feb 2, 2016 IP
  4. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,630
    Likes Received:
    724
    Best Answers:
    152
    Trophy Points:
    470
    #4
    That should be possible to circumvent with temp files and/or just having a check to see if the file is writeable (if not, postpone the write for x ms) - the method isn't very good regardless, but would at least make it a little bit more robust.
     
    PoPSiCLe, Feb 3, 2016 IP
  5. deathshadow

    deathshadow Acclaimed Member

    Messages:
    8,975
    Likes Received:
    1,635
    Best Answers:
    233
    Trophy Points:
    515
    #5
    Hence PHP's flock command:
    http://php.net/manual/en/function.flock.php

    That I'm often shocked how many people don't know about.

    All reads and writes would need to use flock before it would work, the best implementation is to have a set period of time to allow readers to wait before returning an error, looping flock until the timer expires or you achieve a lock. Just be sure said timer is well under PHP's execution limit time, and beware anything that has a lot of writes is going to start bombing and chewing up the connection limit.

    Which is why people usually use databases for this sort of thing, so that the DB engine handles stuff like that for you... well, in addition to allowing for things like indexes to speed up searching through the data.

    There are some pretty major flubs in the presented code too -- not just the broken incomplete form abusing placeholder to do a label tag's job and clearing DIV like it's still 2002, but the PHP itself has some real nasties. For example:

            $error = false;
           
            foreach($required as $field) {
                if (empty($_POST[$field])) {
                $error = true;
                }
            }
           
            if ($error) {
                    $name = $_POST['name'];
                    $email = $_POST['email'];
                    $comment = $_POST['comment'];
                    echo 'All Fields Are Required';
                } else {
    Code (markup):
    Your list of $required contains fields you try to access inside if ($error) -- which means those values may or may not exist throwing errors. I would suggest on that errror check that you break out of the loop once you know there are errors so you aren't wasting time checking extra elements when you already know it's got an error, and I'd probably avoid creating "variables for nothing" in there. Likewise empty on an array index can cause problems and errors, so you are better off on $_POST using array_key_exists instead.

    ... and @ThePHPMaster is quite right you need to santize that data. You are using vertical breaks as your delimiters in the file, so the first thing I'd be doing is either getting rid of the delimiters, or using a function that can sanitize, escape and otherwise make presentable a data line for you. While CSV may add a few characters, its relative simplicity would make it an ideal choice which is why I'd be writing out that data using a function like fputcsv.

    http://php.net/manual/en/function.fputcsv.php
     
    deathshadow, Feb 3, 2016 IP
  6. NetStar

    NetStar Notable Member

    Messages:
    2,290
    Likes Received:
    472
    Best Answers:
    21
    Trophy Points:
    215
    #6
    I don't see any valid reason for using a flat file to store data other than an xml, yaml, ..., ini (joke) for config...Need to store data? Use a database. Odds are it's already set up on your server and it's easier to use.
     
    NetStar, Feb 3, 2016 IP
  7. deathshadow

    deathshadow Acclaimed Member

    Messages:
    8,975
    Likes Received:
    1,635
    Best Answers:
    233
    Trophy Points:
    515
    #7
    Why do you consider ini a joke? It's a great way to store data in a manner where it can't accidentally be "run". I actually prefer it for configuration options since any idiot can edit it, and it's surprisingly good for storing named queries.

    Quite often it's many times easier and efficient than XML... but to be fair almost ANYTHING is more efficient than XML in terms of storage space and data processing. Hence why every time some XML fanboy calls it a "machine readable format" I have to put my hands behind my back so as not to punch them in the face... admittedly I have the same reaction to the mouth-breathing halfwit nonsense that is using base64 on anything other than a e-mail...

    Actually seen a lot of jokers calling base64 a "native format" and "efficient storage" -- something that hasn't been true since the age of mainframes with 6 bit namespaces; the ONLY reason e-mails use it in the first place; 6 O 1. Even funnier when some dipshits call it a 64 bit format, it's like no re-re, it's a 64 VALUE format because each character fits a 6 bit namespace!

    But yeah, complex data? Time to put on the big boy pants and use a DB.
     
    deathshadow, Feb 4, 2016 IP
  8. - Ariën -

    - Ariën - Peon

    Messages:
    1
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    1
    #8
    mysql have a max a big site 10000the users forums chat broke,s down. to big file flat file 0-comment 1-comment on a endless server no problemens and fast
     
    - Ariën -, Jul 3, 2019 IP
  9. JEET

    JEET Well-Known Member

    Messages:
    2,275
    Likes Received:
    118
    Best Answers:
    2
    Trophy Points:
    185
    #9
    4 things here.

    1. You are using $line_of_text = fgets($file)
    to get your comment.
    However, all those comments are not a one line comment.
    You have provided a textarea to write comment.
    Eventually, someone will write a long comment which has line breaks, and your php script which is reading the comment will break the comment.
    You will end up displaying only the first line of the comment, and rest of the comment will vanish.
    You should replace the '\n' character in the comment with something else.
    For example this will work:
    $comment = strip_tags($_POST['comment']);
    $comment= str_replace( "\n", '\n', $comment );
    The "\n" is a line break, but '\n' is not (for PHP)
    Notice the difference of "" sign and '' sign.

    When displaying, do the opposite:
    $line_of_text = str_replace( '\n', "\n", $line_of_text );
    $line_of_text = nl2br($line_of_text );
    This will display the comment properly.

    2. You are not using trim() to remove extra line breaks from the comments field.
    $comment = strip_tags($_POST['comment']);
    This means that if someone writes a comment which has extra line breaks at the top, then your display script will only show empty line, nothing else, since rest of the comment is vanishing...
    Use:
    $comment = trim(strip_tags($_POST['comment']));

    3. I think that if your website is a big one, or might grow in future, then reading a huge single comment file will take time and will slow your website.
    So instead of writing all comments in a single file, make a folder, make separate files in that folder for each ID using the ID like (1234.txt, 444.txt etc) and write your comments in those files.

    Then when showing the comment, you can simply open the file for that particular article ID and read the comments line by line.
    This will be really quick. You probably will not get more than 50 to 100 comments per article, and reading that many lines is pretty quick.

    4. Using strip_tags is ok
    But this will only hide the HTML tag like <script>
    It will not hide the js code which was inside the <script> </script tags.
    Look at this example below:

    <?php
    $s= '<script> alert(" "); </script> This is ok text ';

    //echo strip_tags($s);
    // will print - alert(" "); This is ok text

    //Instead of just that, use this preg_replace code:
    $pattern= '/\<(.*?)\>(.*?)\<(.*?)\>/is';
    $replace= '';
    echo preg_replace($pattern, $replace, $s);
    // will print - This is ok text
    ?>


    Someone who wants to do crappy things, they will put a lot of crap text between the <script> </script> tags, like a URL to a porn site etc, like window.location="some URL here";
    which you do not want to display.
    When you use that preg_replace code, you will end up removing all that crappy stuff they put inside the html tags.


    Good luck!
     
    JEET, Jul 6, 2019 IP