franklos Posted February 14, 2009 Share Posted February 14, 2009 Hi I need to finnish of this site. Unexpected it needed a database and PHP scripts. A friend created some scripts adn I altered them to use them with the CMS I use. (came from Typo3 I use Modx) The problem is in a form that needs to enable folks to delete data. Problem is that deletion works, but the data is also deleted when clicking "no do not delete". Got the script attached. Hopefully there are some stupid errors in this script, attached! gr Frank. [attachment deleted by admin] Quote Link to comment Share on other sites More sharing options...
Psycho Posted February 15, 2009 Share Posted February 15, 2009 Looking at the code it appears that page is called after the item is selected for deletion. Then that page on first load will ask for confirmation (using a form that is posted) for the deletion and on second load will delete the file. The problem is that the very first part of the script gets the values from the $_GET variable (presumable where the item was selected for deletion) and then uses those to run the deletion. So, the deletion occurs even before the suer get a chance to give confirmtion. You need to check for the POST values from the form before runing the delete process. Try something more like this <?php if (isset($_POST['idpub'])) { if($_POST['button1']=='Yes') { include('connections.php'); $idpub = mysql_real_escape_string($_POST['idpub']); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); mysql_free_result($result); } else { //User selected NOT to delete. Redirect somewhere else } } ?> <html> <body> You really want to delete <?php echo $name; ?> <form action="../assets/snippets/nbscripts/pubdelete.php" method="post"> <table border="1" cellpadding="0" cellspacing="0" width="500"> <tr> <td> <p class="bodytext"> You have selected '<B>".$abbreviation."</B> for deletion. Do you want to continue with the delete action? </p> </td> </tr> <tr><td> </td></tr> <tr> <td> <input id="button1" type="submit" value="Yes" name="submit"> <input id="button2" type="submit" value="No" name="submit"> <input type="hidden" value="$idpub" name="idpub"> </td> </tr> </table> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
haku Posted February 15, 2009 Share Posted February 15, 2009 You will probably need a programmer from Finland if you want to Finnish your site Quote Link to comment Share on other sites More sharing options...
Psycho Posted February 15, 2009 Share Posted February 15, 2009 Corrected a couple typos in the cod I posted above: <?php if (isset($_POST['idpub'])) { if($_POST['button1']=='Yes') { include('connections.php'); $idpub = mysql_real_escape_string($_POST['idpub']); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); mysql_free_result($result); } else { //User selected NOT to delete. Redirect somewhere else } } ?> <html> <body> You really want to delete <?php echo $name; ?> <form action="../assets/snippets/nbscripts/pubdelete.php" method="post"> <table border="1" cellpadding="0" cellspacing="0" width="500"> <tr> <td> <p class="bodytext"> You have selected '<B><?php echo $abbreviation; ?></B>' for deletion. Do you want to continue with the delete action? </p> </td> </tr> <tr><td> </td></tr> <tr> <td> <input id="button1" type="submit" value="Yes" name="submit"> <input id="button2" type="submit" value="No" name="submit"> <input type="hidden" value="<?php echo $idpub; ?>" name="idpub"> </td> </tr> </table> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
franklos Posted February 16, 2009 Author Share Posted February 16, 2009 Thanks for your time. However, it seems not much has changed. Also in the statusbar for most of the actions I see the var s that are sent with the action. (pubchar, pubid etc) Do not see those with the altered script. Below the code of the pubdelete.php. I see publisher deleted, when clicking "yes" delete but also on "no" (those buttons). Also this confirmation no longer is inside the site but on a white page. So the CSS is no longer there? Hope there is some TYPO I made. <? include('connections.php'); $idpub = $_GET['idpub']; $pubchar = $_GET['pubchar']; $idpubdelete = $_GET ['idpdel']; $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql); echo "Publisher deleted!"; //exit to make sure the script stops! exit(); ?> Quote Link to comment Share on other sites More sharing options...
Psycho Posted February 16, 2009 Share Posted February 16, 2009 To be honest, your original code made little sense. There is no reason to get the values for $pubchar and $idpubdelete just to populate them into hidden fields that aren't even used. Looking at the script I posted, I see another mistake. Since I can't test against your DB I make no promise that there won't be errors. I expect the person utilizing the code to be able to debug any small issues. The problem with the original logic was that the page was deleting the record BEFORE the user ever made a selection. $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql); echo "<b>You really want to delete $name?"; Why ask them if they want to delete AFTER you delete the record? The code you posted above seems to do the same thing. You are calling the page with parameters on the query string (i.e. $_GET). You do NOT want to delete the record using the GET parameters, because the user has not made a confirmation which will be received via POST. The first script also used variables which have not been declared on the page, so I'm wondering if there is code you left out (e.g. $name & $abbreviation). Here's another update of the above code with comments. <?php if (isset($_POST['idpub'])) { //User has made a decision on the deletion confirmation if($_POST['button1']=='Yes') { //User confirmed the deletion include('connections.php'); $idpub = mysql_real_escape_string($_POST['idpub']); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); mysql_free_result($result); echo "The record has been deleted." } else { //User selected NOT to delete. Redirect somewhere else echo "The record has NOT been deleted." } //Confirmation process complete exit(); } else if (isset($_GET['idpub'])) { //Record passed on the query string for deletion //NOTE: You will need to correct this code for the right field names $idpub = mysql_real_escape_string($_GET['idpub']); //Get the name and abbreviation for the form $sql = "SELECT name, abbreviation FOM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); $record = mysql_fetch_assoc($result); $name = $record['name']; $abbreviation = $record['abbreviation']; } else { //No id passed on GET or POST - error handling echo "No record selected"; exit(); } ?> <html> <body> You really want to delete <?php echo $name; ?> <form action="../assets/snippets/nbscripts/pubdelete.php" method="post"> <table border="1" cellpadding="0" cellspacing="0" width="500"> <tr> <td> <p class="bodytext"> You have selected '<B><?php echo $abbreviation; ?></B>' for deletion. Do you want to continue with the delete action? </p> </td> </tr> <tr><td> </td></tr> <tr> <td> <input id="button1" type="submit" value="Yes" name="submit"> <input id="button2" type="submit" value="No" name="submit"> <input type="hidden" value="<?php echo $idpub; ?>" name="idpub"> </td> </tr> </table> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
franklos Posted February 17, 2009 Author Share Posted February 17, 2009 Hi, you are right about the minimum ability to debug etc. I am currently trying to study PHP from here http://devzone.zend.com/article/627-PHP-101-PHP-For-the-Absolute-Beginner. So i hope to understand more aout them curley brackets, question marks, etc in the comming time. however it seems difficult (but fun) to me. Thanks again, Frank. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 17, 2009 Author Share Posted February 17, 2009 Hi one last question, $idpub = mysql_real_escape_string($_POST['idpub']); gives a login error Access denied for user 'hike'@'localhost' (using password: NO) Googles and found it could be a connection problem, however the line just before this line connects to the database. Anyway its a challenge, thanks, Frank. Quote Link to comment Share on other sites More sharing options...
Psycho Posted February 17, 2009 Share Posted February 17, 2009 That line would not produce a login error - at least I can't see how it could. In any event you definitely need it. You must be especially careful when using ANY user input in a query. That command will escape the value of $_POST['idpub'] so that it is safe for a database query. Not using it will open you up to SQL injection which could allow a malicious user to access or destroy your database. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 17, 2009 Author Share Posted February 17, 2009 here are the errors: And the include('connections.php'); is right before that line. Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: Access denied for user 'hike'@'localhost' (using password: NO) in /usr/home/pianonl/public_html/assets/snippets/nbscripts/readdir.php on line 29 Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: A link to the server could not be established in /usr/home/pianonl/public_html/assets/snippets/nbscripts/readdir.php on line 29 Warning: mysql_query() [function.mysql-query]: Access denied for user 'hike'@'localhost' (using password: NO) in /usr/home/pianonl/public_html/assets/snippets/nbscripts/readdir.php on line 33 Warning: mysql_query() [function.mysql-query]: A link to the server could not be established in /usr/home/pianonl/public_html/assets/snippets/nbscripts/readdir.php on line 33 Access denied for user 'hike'@'localhost' (using password: NO) Quote Link to comment Share on other sites More sharing options...
haku Posted February 17, 2009 Share Posted February 17, 2009 I haven't read this whole thread, but that error appears when you don't have an open connection to the database. So go back and look at your database connection code, and double check that the username, password, and host are all correct. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 17, 2009 Author Share Posted February 17, 2009 Hi Haku, the connection is done through connection.php and runs fine for all the other scripts. Problem seems the mysql_real_escape_string() . But i am a complete newbie, thanks anyway Quote Link to comment Share on other sites More sharing options...
haku Posted February 18, 2009 Share Posted February 18, 2009 That error means that you have no active connection. It doesn't matter if it works for other scripts, its not working for this one. Quote Link to comment Share on other sites More sharing options...
gizmola Posted February 18, 2009 Share Posted February 18, 2009 I will go you one further on the error: If you actually *read it* you will see that it is trying to connect as a user with NO PASSWORD! Quote Link to comment Share on other sites More sharing options...
franklos Posted February 18, 2009 Author Share Posted February 18, 2009 <starts sweating>Hi, made a mistake that script seems to be okay, have I still got the right of a futher question? Deletion still does not function so I guess it must be in the pubdelete.php. Probably full off mistakes but I am trying hard. <? $idpub = $_GET['idpub']; include('connections.php'); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql); echo "Publisher deleted!"; //exit to make sure the script stops! exit(); ?> <off for a shower> thanks Quote Link to comment Share on other sites More sharing options...
franklos Posted February 18, 2009 Author Share Posted February 18, 2009 <sweating more> Could it just be that I am completly dumb. SHould the above script be divided in two scripts? One the deletion form and one doing the deletion action? <starts sweating>Hi, made a mistake that script seems to be okay, have I still got the right of a futher question? Deletion still does not function so I guess it must be in the pubdelete.php. Probably full off mistakes but I am trying hard. <? $idpub = $_GET['idpub']; include('connections.php'); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql); echo "Publisher deleted!"; //exit to make sure the script stops! exit(); ?> <off for a shower> thanks Quote Link to comment Share on other sites More sharing options...
Psycho Posted February 18, 2009 Share Posted February 18, 2009 It depends upon the process flow you are trying to achieve. I typically have my forms pages post to the same page that creates the form. That way I can validate the user's data and, if there are errors, easily redisplay the form with the input they had entered. In this situation, I don't have clue what the problem is. You don't show what is in the pubdelete.php file. Also, the snippet you posted above has the exact same problem I explained before. It would delete the record before the user ever had a chance to confirm. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 18, 2009 Author Share Posted February 18, 2009 Hi, the file posted is the pudelete.php. As said I am a total newbie to PHP and the scripts I use ( add pub(blisher), mod(ify) publisher) all have the same (apparently) unsafe method of handeling data. If you want you can see what I am doing eh trying to do on www.pianoquartet.nl/test for deletion you would need a login which off course I can not provide here. Anyway when clicking delete (if logged in) the page containing the delete submission button. The source shows the hidden input type with the correct value. But deletion is not done. I will try to see what I can do. A learning process. Thanks Quote Link to comment Share on other sites More sharing options...
gizmola Posted February 18, 2009 Share Posted February 18, 2009 franklos, First off, of course you can continue to ask questions, that's the reason phpFreaks exists! In looking at your delete code, it is basic, but looks about right to me. I'm not 100% sure what's in your connections.php at this point, but it needs to make a mysql connection, and it looks to me like that connection may not be available when you actually do the query. You should check the result of the mysql_query() function and if it fails, you should return the mysql_error() so you can see what's going on. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 18, 2009 Author Share Posted February 18, 2009 Hi Gizmola, I know it is all basic and maybe someday it will get up to standard. This was to be a simple site but things ran out of control. Anyway here' s my connections.php. Let me know whats wrong and how to debug if you find the time or feel like it. <?php $sqlConn = mysql_connect('localhost','user','password'); if (!$sqlConn) { die ('unable to connect' . mysql_error()); } $db = mysql_select_db('database', $sqlConn); //sets encoding to utf8 mysql_query("SET character_set_client = utf8;"); mysql_query("SET character_set_results = utf8;"); ?> As for the codes that I gracefully use, written above. It looks like whatever I click it returns the "User selected NOT to delete" button. I have got the feeling the $idpub = mysql_real_escape_string($_GET['idpub']); does not pickup a value. Thanks all, Frank. Quote Link to comment Share on other sites More sharing options...
Maq Posted February 18, 2009 Share Posted February 18, 2009 One of the best debugging tools is echo. You should echo out your variables and queries to ensure they hold the correct values. For example: echo $idpub . " "; $sql = "DELETE FROM publisher WHERE idpub=$idpub"; echo $sql; $result = mysql_query($sql) or die(mysql_error()); In the query ($sql) you should also surround $idpub with single quotes if the datatype is NOT and integer. And finally use or die(msyql_error()) for any possible query errors, if there are it should output a descriptive error message to the browser. Quote Link to comment Share on other sites More sharing options...
franklos Posted February 19, 2009 Author Share Posted February 19, 2009 Hi, so I changed the pubdelete.php with the echo $sql, like so <? include('connections.php'); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; echo $sql; $result = mysql_query($sql); echo "Publisher deleted!"; //exit to make sure the script stops! exit(); ?> On clicking "ÿes" (delete publisher) that returns: DELETE FROM publisher WHERE idpub=Publisher deleted! so the idpub is not passed on to the pudelete.php? While on the page with the buttons the source shows this: [code <input id="button1" type="submit" value="Yes" name="submit"> <input id="button2" type="submit" value="No" name="submit"> <input type="hidden" value="123" name="idpub"> idpub is the id from that publisher table, so integer. Quote Link to comment Share on other sites More sharing options...
Maq Posted February 19, 2009 Share Posted February 19, 2009 so the idpub is not passed on to the pudelete.php? Ding Ding Ding! We have a winner! You need to use the POST method instead of the GET method. $idpub = $_POST['idpub']; Is your form tag on this page... something like ??? </pre> <form method="POST" action="<?php%20echo%20%24_SERVER%5B'PHP_SELF'%5D;%20?>">< Quote Link to comment Share on other sites More sharing options...
franklos Posted February 19, 2009 Author Share Posted February 19, 2009 Hi an "almost" winner then, now get ' the record has NOT been deleted" I don t see where I should put your bit of code in the following. What a mountain to climb for me... <?php $idpub = $_POST['idpub']; session_start(); if (isset($_POST['idpub'])) { //User has made a decision on the deletion confirmation if($_POST['button1']=='Yes') { //User confirmed the deletion include('connections.php'); $idpub = mysql_real_escape_string($_POST['idpub']); $sql = "DELETE FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); mysql_free_result($result); echo "The record has been deleted."; } else { //User selected NOT to delete. Redirect somewhere else echo "The record has NOT been deleted."; } //Confirmation process complete exit(); } else if (isset($_GET['idpub'])) { //Record passed on the query string for deletion //NOTE: You will need to correct this code for the right field names include('connections.php'); $idpub = mysql_real_escape_string($_GET['idpub']); //Get the name and abbreviation for the form $sql = "SELECT name, abbreviation FROM publisher WHERE idpub=$idpub"; $result = mysql_query($sql) or die(mysql_error()); $record = mysql_fetch_assoc($result); $name = $record['name']; $abbreviation = $record['abbreviation']; } else { //No id passed on GET or POST - error handling echo "No record selected"; //exit(); } ?> <html> <body> <form method="POST" action="<?php echo $_SERVER['PHP_SELF']; ?>"> <table border="1" cellpadding="0" cellspacing="0" width="500"> <tr> <td> <p class="bodytext"> You have selected '<B><?php echo $abbreviation; ?></B>' for deletion. Do you want to continue with the delete action? </p> </td> </tr> <tr><td> </td></tr> <tr> <td> <input id="button1" type="submit" value="Yes" name="submit"> <input id="button2" type="submit" value="No" name="submit"> <input type="hidden" value=<?php echo $idpub;?> name="idpub"> </td> </tr> </table> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
Maq Posted February 19, 2009 Share Posted February 19, 2009 name="idpub"> Line 68 should be: Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.