CODEWITHSUNDEEP
phpsql
Mark Jones
Mark Jones

Reputation: 193

delete value from database using link

I have a page that shows comments, "comments.php", and I include the page on any other page that I want comments to show. I am trying to implement a way to delete comments if needed. Each comment has an auto-increment "commentID". Right now I'm using an action, and then just using a link to call the action.

When I hover over the link, the URL looks correct, but when I click it, the page refreshes and nothing happens. Any ideas?

Action:

if ($_POST['action'] == 'delete') {
    $sql = "delete from " . $db_prefix . "comments where commentID = " . (int)$_GET['id'];
    mysql_query($sql) or die('error deleting user: ' . $sql);
header('Location: ' . $_SERVER['HTTP_REFERER']);
}

Show comments and show link to delete: (unnecessary code has been left out)

echo '<a href="/comments.php?action=delete&id=' . $result['commentID'] . '">delete</a>

What am I doing wrong?

Upvotes: 2

Views: 65

Answers (1)

Script47
Script47

Reputation: 14540

In your code you're mixing $_POST with $_GET.

Try this,

 ?php
 if ($_GET['action'] == 'delete') {
    if (!ctype_digit($_GET['id'])) {
        exit("ID has to be an int.");
    }
    $id = intval($_GET['id']);

    $sql = "DELETE FROM `" . $db_prefix . "comments` WHERE `commentID` = " . $id;

    mysql_query($sql) or die('error deleting user: ' . $sql);

    header('Location: ' . $_SERVER['HTTP_REFERER']);
}
?>

Your link also shows action=delete so you should be checking if $_GET action equals delete.

Edit 1

Your code is prone to SQL injection, you are still using MySQL even though it has been deprecated, you should use either MySQLi or PDO with prepared statements.

Not to mention your $_GET data is being passed on to the query without being sanitized, you should start using intval it would make it better and prevent XSS. Please read up on the function intval and ctype_digit to get a better understanding at what it does.

Edit 2

Scrap $_SERVER['HTTP_REFERER']

How reliable is HTTP_REFERER?

Edit 3

As noted in comments:

"If you're using the same file for everything, just omit the file name ?action=delete&id"

which would explain the 404 you mentioned in comments.

Upvotes: 4

Related Questions