1

I have a comment section on my index.php page. (like I have displayed here) I have a delete link that sends you to the delete page. I want to be able to delete that specific ID when it is clicked. index.php

 <div class='commentnest'>
                <p id='id'> ".$row['id']."</p>
                <p id='user'> ".$row['user'].":</p>
                <p id='comment'>".$row['usercomment']." </p>
                <p id='time'>".$row['timestamp']."</p>
                <a href='delete.php?=".$row['id']."'>Delete</a>
        </div>

delete.php page

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}
$sql = "DELETE FROM commenttable WHERE id=id";
if (mysqli_query($conn, $sql)) {
    echo "Record deleted successfully";
} else {
    echo "Error deleting record: " . mysqli_error($conn);
}
mysqli_close($conn);
?> 

every time I send a specific comment there, it deletes all the comments. I know it's because I'm not stating which ID to select, but i'm having a hard time figuring that out.

Thanks for any help!

splash58
  • 26,043
  • 3
  • 22
  • 34
kenny
  • 438
  • 4
  • 14

1 Answers1

1

All records are deleted because id=id will be always true unless id is NULL. You need to properly pass it from your HTML:

<a href='delete.php?id=".$row['id']."'>Delete</a>

and then properly use it in the query:

$sql = "DELETE FROM commenttable WHERE 
  id='".mysqli_escape_string($conn,$_REQUEST["id"])."'" ;

if the id is always an integer, you can write simpler:

$sql = "DELETE FROM commenttable WHERE 
  id=".(int)$_REQUEST["id"]);
Sasha Pachev
  • 5,162
  • 3
  • 20
  • 20
  • 1
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)*** Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Jun 09 '16 at 21:02
  • mysqli_escape_string() or casting the user input to int removes the possibility of SQL injection. While prepared statements can also protect against SQL injection, this is not the only way to do it. – Sasha Pachev Jun 09 '16 at 21:04
  • 1
    Escaping the string, while a good idea, can be defeated. Anyhow - props to you and your running family! You're all faster than I am! ¯\\_(ツ)_/¯ – Jay Blanchard Jun 09 '16 at 21:07
  • *All records are deleted because id=id will be always true unless id is NULL* - You are wrong. They are deleted because for every row field id is equal itself – splash58 Jun 09 '16 at 21:33
  • `create table test.t1(id int, n int);insert into t1 values (1,1), (NULL,2); delete from test.t1 where id = id;select * from test.t1;` - you will see that the second record is still there. It is possible for `id=id` to be false, and it is achieved when `id` is NULL. – Sasha Pachev Jun 09 '16 at 22:20