I've written this very basic content management page. There is a form on the left and a field on the right that contains the user's posts. The page allows only 10 posts at a time. Any new post deletes the oldest current post. Very basic. There are two scripts. The first is the html page. The second is the form capturing script. I extend a challenge to anybody who would like to improve this code. (AKA give me pointers.) The html page: <?php include_once("includes.php"); // 1. Create a database connection $connection = mysql_connect(DB_SERVER, DB_USER, DB_PASS); if (!$connection) { die("Database connection failed: " . mysql_error()); } // 2. Select a database to use $db_select = mysql_select_db(DB_NAME, $connection); if (!$db_select) { die("Database selection failed: " . mysql_error()); } ?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title>Page of Posts</title> <link rel="stylesheet" type="text/css" href="pageofposts.css" /> </head> <body> <!-- Begin Wrapper --> <div id="wrapper"> <!-- Begin Header --> <div id="header"> <p>Basic functionality started Wednesday October 26, 2011 and finished Saturday October 29, 2011.</p> </div> <!-- End Header --> <!-- Begin Left Column --> <div id="leftcolumn"> <form action="formcapture.php" method="post"> <p> Title: <input type="text" name="title" value=""><br> Name: <input type="text" name="author" value="-Anonymous"><br> <textarea name="thetext" rows="10" cols="38"></textarea><br> <input type="submit" value="Post"><input type="reset" value="Clear"> </p> </form> <p>USER BEWARE: Enter text in the above box and sign whatever name you wish. Your message will appear immediately in the column on the right. There is one catch. There can only be a maximum of 10 posts on a page. Once the limit is reached, any new post deletes the oldest post on the page.</p> </div> <!-- End Left Column --> <!-- Begin Right Column --> <div id="rightcolumn"> <?php // Perform database query $query = "select * from posts where ten<10"; $result = mysql_query($query, $connection); if (!$result) { die("Database query failed: " . mysql_error()); } $xx = 0; while($row = mysql_fetch_array($result)) { $time[$xx] = $row["time_col"]; $text[$xx] = $row["text_col"]; $sig[$xx] = $row["sig_col"]; $post_num[$xx] = $row["ten"]; $title[$xx] = $row['title_col']; $xx++; } $total_posts = count($post_num); $yy = $total_posts; for ($yy=count($post_num); $yy>0; $yy--) { echo "<div id=\"post\">"; echo "<h1>".$title[($yy-1)]."</h1>"; echo "<p class=\"comment\">".$text[($yy-1)]."</p>"; echo "<p class=\"sig\">".$sig[($yy-1)]."<br>".$time[($yy-1)]."</p></div>"; $query = "delete from posts where ten>9"; $result = mysql_query($query, $connection); if (!$result) { die("Database query failed: " . mysql_error()); } } ?> <!-- ================== TEMPLATE FOR POSTS ================== <div id="post"> <h1>The Date</h1> <p class="comment"></p> <p class="sig"></p> </div> --> </div> <!-- End Right Column --> </div> <!-- End Wrapper --> </body> </html> <?php // 5. Close connection mysql_close($connection); ?> PHP: The formcapture.php script: <?php include_once("includes.php"); // 1. Create a database connection $connection = mysql_connect(DB_SERVER, DB_USER, DB_PASS); if (!$connection) { die("Database connection failed: " . mysql_error()); } // Select a database to use $db_select = mysql_select_db(DB_NAME, $connection); if (!$db_select) { die("Database selection failed: " . mysql_error()); } // Get the values from the form in pageofposts.php $title = $_POST['title']; $thetext = $_POST['thetext']; $author = $_POST['author']; $time = date('j M Y')."<br>". date('g:i a'); $zero = 0; // Send entered values to database. if(isset($thetext) && isset($author)) { // Reverse the order of the table $result = mysql_query("alter table posts order by ten desc", $connection); if (!$result) { die("Database query failed (Reverse the order of the table): " . mysql_error()); } // Add 1 to each 'ten' value $result = mysql_query("update posts set ten=ten+1", $connection); if (!$result) { die("Database query failed (Add 1 to each 'ten' value): " . mysql_error()); } // Send new value to table $result = mysql_query("insert into posts (time_col, sig_col, text_col, ten, title_col) values ('{$time}', '{$author}', '{$thetext}', '{$zero}', '{$title}')", $connection); if (!$result) { die("Database query failed (Send new value to table): " . mysql_error()); } // Reset the order of the table $result = mysql_query("alter table posts order by ten desc", $connection); if (!$result) { die("Database query failed (Reset the order of the table): " . mysql_error()); } } // Close connection mysql_close($connection); // Redirect back to pageofposts.php. header("Location: pageofposts.php"); exit; ?> PHP:
No replies... In that case can any experienced PHP programmer give me pointers? This was my very first script from scratch. I'd appreciate very much an experienced programmer telling me about any inefficiencies in my code or even if my scripts have any structural flaws. Anything?
I will provide some feedback, but take it for what its worth. I have experience but I am certainly not an expert yet! The thing I want to talk about is what you are using for your database. $connection = mysql_connect(DB_SERVER, DB_USER, DB_PASS); PHP: mysql as used in the above code is no longer under active development for PHP and generally should only be used to maintain legacy code or similar. I think most would agree that you should be using either mysqli or prepared statments/PDO. I use mysqli and it works good for my needs, but I am going to start using prepared statements on my upcoming projects. I think the real experts will probably say that you should be using prepared statements exclusively, but I cannot offer advice on that right now because I don't use them yet. So lets stick with mysqli for now. Other more experienced coders can probably talk more about this. The next thing I see is your form variables: $title = $_POST['title']; $thetext = $_POST['thetext']; $author = $_POST['author']; $time = date('j M Y')."<br>". date('g:i a'); $zero = 0; PHP: What I have learned is that you have to treat every thing that is being put into one of your forms as a possible bomb that could destroy your website. Basically, anything a user puts into your website has to be cleaned and checked before you can let it get any further. A good hacker, or even a bad one, can write a mysql query right into one of your forms that can influence your database. So you have to make sure you put your form input through the ringer before you let it move onto the database. Here is one way: $title = mysqli_real_escape_string($connection, trim($_POST['title'])); $thetext = mysqli_real_escape_string($connection, trim($_POST['thetext'])); $author = mysqli_real_escape_string($connection, trim($_POST['author'])); $time = date('j M Y')."<br>". date('g:i a'); $zero = 0; PHP: mysqli_real_escape_string will escape dangerous characters that could be used to for a mysql query. This way you make it much harder for someone to mess with your database through forms. This is just one of probably many ways to sanitize your input in at least some fashion. What is your xx variable for? Rather, what are you under the impression it is for?
Hey absentx thanks for the feedback! Use mysqli or prepared statements huh? I'll definitely look into those. Clean out any entered form data with mysqli_real_escape_string and trim functions. I'm assuming that will take care of the no apostrophes or quote marks bug. The $xx variable, you mean the one in the while loop? Lol now that you mention it I can see it really isn't doing anything. I tried using a for loop before I was using the while loop so it is left over code that I didn't delete. Thanks for pointing it out!
Yeah we always have to assume that the data being entered into our forms is bad, so try and do everything possible to make sure it is good safe data before entering it into your database or using it for whatever purpose you need it. mysqli_real_escape_string will add a backslash before certain characters (thus "escaping" them) from the PHP Manual: Now, if you are entering some data that has apostrophe's or any of the other characters that mysqli_real_escape_string affects, then you can use stripslashes or similar to display the data properly. So if you enter Bob's Guns into the database, mysqli_real_escape_string would make that Bob\'s Guns and enter it into the db. Now lets display it: $store = $row['store_name']; echo $store //result would be "Bob\'s Guns" //that is kind of ugly PHP: so we do it like this: $store = stripslashes($row['store_name']); echo $store //result would be "Bob's Guns" //that's what we want PHP: