views:

522

answers:

5

For some reason, JavaScript/PHP wont delete my data from MySQL! Here is the rundown of the problem.


I have an array that displays all my MySQL entries in a nice format, with a button to delete the entry for each one individually. It looks like this:

<?php

      include("login.php");

     //connection to the database
     $dbhandle = mysql_connect($hostname, $username, $password)
      or die("<br/><h1>Unable to connect to MySQL, please contact support at [email protected]</h1>");

     //select a database to work with
     $selected = mysql_select_db($dbname, $dbhandle)
       or die("Could not select database.");

     //execute the SQL query and return records
     if (!$result = mysql_query("SELECT `id`, `url` FROM `videos`"))
     echo 'mysql error: '.mysql_error();

     //fetch tha data from the database
     while ($row = mysql_fetch_array($result)) {
        ?>

       <div class="video"><a class="<?php echo $row{'id'}; ?>" href="http://www.youtube.com/watch?v=&lt;?php echo $row{'url'}; ?>">http://www.youtube.com/watch?v=&lt;?php echo $row{'url'}; ?></a><a class="del" href="javascript:confirmation(<? echo $row['id']; ?>)">delete</a></div>

<?php }

//close the connection
mysql_close($dbhandle);
?>

The delete button has an href of javascript:confirmation(<? echo $row['id']; ?>) , so once you click on delete, it runs this:

<script type="text/javascript">
<!--
function confirmation(ID) {
    var answer = confirm("Are you sure you want to delete this video?")
    if (answer){
     alert("Entry Deleted")
     window.location = "delete.php?id="+ID;
    }
    else{
     alert("No action taken")
    }
}
//-->
</script>

The JavaScript should theoretically pass the 'ID' onto the page delete.php. That page looks like this (and I think this is where the problem is):

<?php

include ("login.php");

mysql_connect($hostname, $username, $password)
 or die("Unable to connect to MySQL");

mysql_select_db ($dbname)
or die("Unable to connect to database");

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` ='.$id.'");
echo ("Video has been deleted.");
?>

If there's anyone out there that may know the answer to this, I would greatly appreciate it. I am also opened to suggestions (for those who aren't sure).

Thanks!

+10  A: 

Hi,

In your delete.php script, you are using this line :

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` ='.$id.'");

The $id variable doesn't exists : you must initialize it from the $_GET variable, like this :

$id = $_GET['id'];

(This is because your page is called using an HTTP GET request -- ie, parameters are passed in the URL)

Also, your query feels quite strange : what about this instead :

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` = '$id' ");

ie, removing the '.' : you are inside a string already, so there is nothing to concatenate (the dot operator in PHP is for concatenation of strings)


Note :

  • if this works on some server, it is probably because of register_globals
    • For more informations, see Using Register Globals
    • But note that this "feature" has been deprecated, and should definitly not be used !
      • It causes security risks
      • And should disappear in PHP 6 -- that'll be a nice change, even if it breaks a couple of old applications
  • your code has a big SQL injection hole : you should sanitize/filter/escape the $id before using it in a query !
    • If you video.id is a string, this means using mysql_real_escape_string
      • If you where using the mysqli or PDO extensions, you could also take a look at prepared statements
    • with an integer, you might call intval to make sure you actually get an integer.


So, in the end, I would say you should use something that looks like this :

$id = $_GET['id'];
$escaped_id = mysql_real_escape_string($id);
$query = "DELETE FROM `videos` WHERE `videos`.`id` = '$escaped_id'";
// Here, if needed, you can output the $query, for debugging purposes
mysql_query($query);

Hope this helps !

Pascal MARTIN
Sweet! It worked!Thank you! =]
Michal Kopanski
A: 

You're trying to delimit your query string very strangely... this is what you want:

mysql_query('DELETE FROM `videos` WHERE `videos`.`id` ='.$id);

But make sure you sanitize/validate $id before you query!

Edit: And as Pascal said, you need to assign $id = $_GET['id'];. I overlooked that.

brianreavis
Thanks, you were on the right track, but the query wasn't working, even with the $_GET request.The correct query was `mysql_query("DELETE FROM `videos` WHERE `videos`.`id` = $id ");`
Michal Kopanski
sorry, it got all screwed up, here it is:mysql_query("DELETE FROM `videos` WHERE `videos`.`id` = $id ");
Michal Kopanski
okay, so i don't how to add code into comments, here is a last attempt:<code>mysql_query("DELETE FROM `videos` WHERE `videos`.`id` = $id ");</code>
Michal Kopanski
A: 

In your delete.php you never set $id.

You need to check the value in $_REQUEST['id'] (or other global variable) and ONLY if it's an integer, set $id to that.

EDIT: Oh, also you need to remove the periods before and after $id in the query. You should print out your query so you can see what you're sending to the sql server. Also, you can get the SQL server's error message.

JasonWoof
Thanks, I got it working now!
Michal Kopanski
A: 

You add extra dots in the string. Use

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` ='$id'");

instead of

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` ='.$id.'");

Also check how do you get the value of $id.

NawaMan
A: 

Thanks everyone. I used Pascal MARTIN's answer, and it comes to show that I was missing the request ($_GET) to get the 'id' from the precious page, and that some of my query was incorrect.

Here is the working copy:

<?php

include ("login.php");

$id = $_GET['id'];

mysql_connect($hostname, $username, $password)
 or die("Unable to connect to MySQL");

mysql_select_db ($dbname)
or die("Unable to connect to database");

mysql_query("DELETE FROM `videos` WHERE `videos`.`id` = $id ");
echo ("Video ".$id." has been deleted.");
?>

Thanks again!

Michal Kopanski