[RESOLVED] I need help with my comment system

StripedTiger

<?php
mysql_connect("localhost","xxxxx","xxxxx") or die(mysql_error());
mysql_select_db("xxxxx_comments")or die(mysql_error());

if (isset($_POST['submit'])) { 

function curPageURL() {
 $pageURL = 'http';
 if ($_SERVER["HTTPS"] == "on") {$pageURL .= "s";}
 $pageURL .= "://";
 if ($_SERVER["SERVER_PORT"] != "80") {
  $pageURL .= $_SERVER["SERVER_NAME"].":".$_SERVER["SERVER_PORT"].$_SERVER["REQUEST_URI"];
 } else {
  $pageURL .= $_SERVER["SERVER_NAME"].$_SERVER["REQUEST_URI"];
 }
 return $pageURL;
}

$date=strftime("%a %b %d %Y %H:%M:%S");
$url= curPageURL();


$sql="INSERT INTO comments (username, date, comment, url)
VALUES ($username,$date,'".$_POST['comment']."',$url)"; 
mysql_query($sql);

?>


Comment Submitted
<?php
}
else
{
?>


<table width="286" height="185" align="right">
        <tr> 
          <td width="79" height="179"><div align="left">
              <form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
                <p> 
                  <textarea name="comment" cols="69" rows="7" wrap="VIRTUAL" id="comment"></textarea>
                  <input type="submit" name="submit" value="Comment">
                </p>
                </form>
            </div></td>
        </tr>
      </table>
<?php 
} 
?>

There are no errors, it will display the "Comment Submitted", but no records are created in the database.

dagon

change

mysql_query($sql);

mysql_query($sql) or die(mysql_error());

bradgrafelman

Some comments to start debugging when you examine the MySQL error:

Where is $username ever defined?
What time of column is 'date'? If it's not a DATETIME column, why not? If it is, then there's no reason to try and generate a date with PHP - MySQL has built-in functions to do this for you (e.g. NOW() or CURTIME()).
String values in SQL queries must be quoted, just like in PHP. Right now you have what seems to be 3 string values that aren't quoted in your query.
If there's a separate table for users, then you probably should be storing the id (or PRIMARY KEY) of the corresponding row in the users table. This has to do with database normalization - something you might want to look into once you learn the basics of SQL.
Your code is vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query string. Instead, it must first be sanitized with a function such as [man]mysql_real_escape_string/man.

StripedTiger

bradgrafelman;10916177 wrote:
Some comments to start debugging when you examine the MySQL error:

Where is $username ever defined?

What time of column is 'date'? If it's not a DATETIME column, why not? If it is, then there's no reason to try and generate a date with PHP - MySQL has built-in functions to do this for you (e.g. NOW() or CURTIME()).

String values in SQL queries must be quoted, just like in PHP. Right now you have what seems to be 3 string values that aren't quoted in your query.

If there's a separate table for users, then you probably should be storing the id (or PRIMARY KEY) of the corresponding row in the users table. This has to do with database normalization - something you might want to look into once you learn the basics of SQL.

Your code is vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query string. Instead, it must first be sanitized with a function such as [man]mysql_real_escape_string/man.

1 It's defined in a session included in a page that included this script.
2 It's not DATETIME, because I wanted to customize the way it looked.
3 That did the trick!
4 All my tables have id rows that auto increment.
5 Thanks for the tip, I'll look into that.

dagon

its best to store it as datetime or one of the db supported formats then format it when outputting it, otherwise you lose the ability to do things like sorting by date.

bradgrafelman

If it's stored in a session, then you should be using $_SESSION to retrieve it. More info on that superglobal here: [man]reserved.variables.session[/man]
As dagon points out, if it's a date and a time, use a DATETIME column. It makes sense, plus it allows you to work with the data much easier (e.g. by just using the NOW() or CURTIME() function in your INSERT query, for starters). When SELECT'ing data, you can easily manipulate the DATETIME column into any format you like using MySQL's DATE_FORMAT() function.
If so, then you should look into learning joins (in this case, it'd be a really simple join) so that you can store only the user id in the column (and possibly use a FOREIGN KEY syntax, if you're using InnoDB as the storage engine). There are several advantages to storing data in this manner which is one of the reasons you'll often see someone suggest to a poster that they "normalize their database schema."

StripedTiger

The comments are working now, but now I'm worried about SQL injections. Is there a way to secure my database without preventing users to post any characters in the comment field?

laserlight

StripedTiger wrote:
Is there a way to secure my database without preventing users to post any characters in the comment field?

bradgrafelman mentioned a solution in post #3:

bradgrafelman wrote:
Instead, it must first be sanitized with a function such as mysql_real_escape_string().

StripedTiger

Yeah, but wouldn't that prevent users from posting ' or " any where in their comment?

laserlight

StripedTiger wrote:
Yeah, but wouldn't that prevent users from posting ' or " any where in their comment?

No, it would not.

bradgrafelman

It would actually do the opposite - it would allow them to post single quotes. At present, since you do no escaping/sanitizing of the data, a single quote would break your SQL query string (and/or allow SQL injection attacks).

Using [man]mysql_real_escape_string/man, a single quote (among certain other characters) would be prepended with a backslash so that it won't break the SQL query and instead be included within the string as a literal single quote (quite like using \' in a single-quoted string in PHP).