Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.
Popular code excerpts may be moved to "Code Snippets" by the moderators.
I wrote a very basic blog for a lady and she wanted to ability to edit it so I made a page that lists all the blogs in the db and shows the titles with a radio button next to them, she selects the radio button and says edit then gets a page that has the title, blurb, and blog in <input>and<textarea> boxes for editing. My problem is running the update command I can't think of a way to condense it down to one quick UPDATE command so it's split up into 3 updating each one then recalling and displaying the new updated blog, any help on cleaning this up and making it more efficient would be great. oh ya, it also strips a single quote ' from any line and replaces it with ~... i think it works, haven't tested it yet. Thank you.
<?php
$title = $_POST['title'];
$blurb = $_POST['blurb'];
$blog = $_POST['blog'];
$num = $_POST['number'];
$safetitle = str_replace("'", "~", $title);
$safeblurb = str_replace("'", "~", $blurb);
$safeblog = str_replace("'", "~", $blog);
$con = mysql_connect("we all know what goes here.");
if(!$con) {
die("Could not connect to database:" . mysql_error());
}
@mysql_select_db("blogdb") or die("Could not select database: " . mysql_error());
$result = mysql_query("UPDATE blogs SET title=$safetitle WHERE blogID=$num") or die("title UPDATE error: " . mysql_error());
$result2 = mysql_query("UPDATE blogs SET blurb=$safeblurb WHERE blogID=$num") or die("Blurb UPDATE error: " . mysql_error());
$result3 = mysql_query("UPDATE blogs SET content=$safeblog WHERE blogID=$num") or die("Content UPDATE error: " . mysql_error());
?>
<html>
<body>
<?
$newquery = mysql_query("SELECT * FROM blogs WHERE blogID = '$num'") or die("Could not re-SELECT stuff: " .mysql_error());
while($row = mysql_fetch_array($newquery)) {
$newtitle = $row['title'];
$newblurb = $row['blurb'];
$newblog = $row['content'];
}
$snewtitle = str_replace("~", "'", $newtitle);
$snewblurb = str_replace("~", "'", $newblurb);
$snewblog = str_replace("~", "'", $newblog);
?>
<p align="center">Your blog has been updated.<br /> The new title is: <?php echo $snewtitle ?><br />Your new blurb is: <?php echo $snewblurb ?><br />And your new blog is: <?php echo $snewblog ?></p>
<a href="test-edit.php">Click</a> to edit another blog   |   <a href="/">Click</a> to go back to your home page.
</body>
</html>
<?php
$num = intval($_POST['number']);
$safetitle = str_replace("'", "~", $_POST['title']);
$safeblurb = str_replace("'", "~", $_POST['blurb']);
$safeblog = str_replace("'", "~", $_POST['blog']);
$con = mysql_connect("we all know what goes here.");
if(!$con) {
die("Could not connect to database:" . mysql_error());
}
@mysql_select_db("blogdb") or die("Could not select database: " . mysql_error());
$result = mysql_query("UPDATE blogs SET title='".$safetitle."', blurb='".$safeblurb."', content=".$safeblog." WHERE blogID='".$num."'") or die("title UPDATE error: " . mysql_error());
?>
<html>
<body>
<?
$newquery = mysql_query("SELECT * FROM blogs WHERE blogID = '".$num."'") or die("Could not re-SELECT stuff: " .mysql_error());
while($row = mysql_fetch_array($newquery)) {
$newtitle = $row['title'];
$newblurb = $row['blurb'];
$newblog = $row['content'];
}
$snewtitle = str_replace("~", "'", $newtitle);
$snewblurb = str_replace("~", "'", $newblurb);
$snewblog = str_replace("~", "'", $newblog);
?>
<p align="center">Your blog has been updated.<br /> The new title is: <?php echo $snewtitle ?><br />Your new blurb is: <?php echo $snewblurb ?><br />And your new blog is: <?php echo $snewblog ?></p>
<a href="test-edit.php">Click</a> to edit another blog   |   <a href="/">Click</a> to go back to your home page.
</body>
</html>
as you can see I am completely new to writing php and mysql stuff so I have no idea what the htmlentities() are or what you are talking about with authorization and injection. Sorry I know this is code cleanup, and I really appreciate the help on cleaning it up, any chance you can direct me to, or help me out with the injection, authorization, and htmlentities() bit? The stipping of the single quote ' was needed because the first blog she tried to post was titled something like "Women's blah blah blah" and that single quote threw off the whole script so I figure str_replace it before it goes in, then str_replace it before it's echoed to the page.
htmlentities() Convert all applicable characters to HTML entities. It changes all characters that has a special value in html (>, <, ', ") into a html-'safe' character. It's used mainly to stop cross-side scripting attacks.
Authorization refers to authenticating the person with the correct username and password as the 'owner' or 'administrator' of the blog. You didn't paste that code so it's probably irrelevant to this script.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
What would the proper syntax be for the htmlentities() for something like this? Thank you for the link for injections, and the authentication that I have set up is a user login page then to member home page that verifies their identity before allowing them to get to an edit or add page. Thank you again for all your help.
$result = mysql_query("UPDATE blogs SET title=$title WHERE blogID=$num") or die("Could not UPDATE: " .mysql_error());
Right? Sorry I know I seem retrded, I just want to make sure I get this right.
Thank you again for the help
EDIT: Also when I call the data back from the db do I have to do it using the htmlentities again or will it automatically fix it when it comes back out of the db to the page?
josh wrote:Its still vulnerable to mysql injection
I don't see any authorization even
Right, but let's hope the lady doesn't need to enter "O'Brien;" as an input.
For the query, use mysql_real_escape_string() to escape the SQL.
Use htmlentities() to escape the markup, at output time, not as part the SQL query. The raw value should be stored in the database, as different outputs than HTML may be necessary, where you do not want htmlentities() applied.