3

I have a PHP script that reads a database table and inserts all the rows into an HTML table until it's displayed all available rows as shown here:

require_once('dbconnect.php');

$sql = 
  "SELECT 
      ID, Site, Type, Requested, Quote,
      PO, CADs, MCS, DFP,
      SIM, Prereqs, Design, Report, Delivered 
    FROM Predictions";

$result = $conn->query($sql);

if ($result->num_rows > 0) {
  echo '<table class="table table-hover table-condensed">';

  while($row = $result->fetch_assoc()) {
    echo
      '<tbody>'.
        '<tr>'.
          '<td>'.$row['ID'].'</td>'.
          '<td>'.$row['Site'].'</td>'.
          '<td>'.$row['Type'].'</td>'.
          '<td>'.$row['Requested'].'</td>'.
          '<td>'.$row['Quote'].'</td>'.
          '<td>'.$row['PO'].'</td>'.
          '<td>'.$row['CADs'].'</td>'.
          '<td>'.$row['MCS'].'</td>'.
          '<td>'.$row['DFP'].'</td>'.
          '<td>'.$row['SIM'].'</td>'.
          '<td>'.$row['Prereqs'].'</td>'.
          '<td>'.$row['Design'].'</td>'.
          '<td>'.$row['Report'].'</td>'.
          '<td>'.$row['Delivered'].'</td>'.
          '<td>'.
            '<a href="#">'.
              '<span class="edit"><i class="fa fa-pencil"></i></span>'.
            '</a> | <a href="#">'.
              '<span class="delete"><i class="fa fa-times"></i></span>'.
            '</a>'.
          '</td>'.
        '</tr>'.
      '</tbody>';
  }

  echo "</table>";
}
else
  echo "0 results";

$conn->close();

That all works fine, but now I want to have what is essentially a delete button (you can see the markup above that creates the icon/link) that will populate automatically to correspond with the appropriate ID for the mysql database table. Image of table for visual idea of what I'm going for.

My delete script so far is below, but I have no idea what to put in the "WHERE id=", or how to incorporate it into my first script once it's setup properly.

<?php
require_once('dbconnect.php');

$sql = "DELETE FROM Predictions WHERE id=";

if($conn->query($sql) === TRUE)
  echo "Item deleted successfully";
else
  echo "Error deleting record; ". $conn->error;

$conn->close();

So basically I need advice to modify both of these scripts so that a delete link (or form, I don't care) is generated in the first script then applies the second script and it knows the corresponding id to use. In my search to solve this problem I saw some potential solutions using _GET, but in the same thread others said that is in fact a very bad and insecure solution.. so I'm very confused!

I'm learning PHP as I go, and I've only been going at it for about 2 days, so please have mercy :)

Blag
  • 5,818
  • 2
  • 22
  • 45
seibzehn
  • 75
  • 8

1 Answers1

3

Change this

<a href='#'><span class='delete'>

to

<a href='deletepage.php?id=" . $row["ID"] . "'><span class='delete'>

then on "deletepage.php", whatever you are going to call that page do something like

require_once('dbconnect.php');
$id = (int)$_GET['id'];
$sql = "DELETE FROM Predictions WHERE id=" . $id;    
if($conn->query($sql) === TRUE) {
    echo "Item deleted successfully";
} else {
    echo "Error deleting record; ". $conn->error;
}
$conn->close();

I don't know what driver you are using here but the preferred solution would be using a prepared statement with a parameterized query.

So pretty much you send the id via a GET parameter to your "delete page". That page takes that value, casts it to an int to avoid SQL injections (read further below), and then deletes the data. You also could instead of echoing a success there use a header to redirect them to the previous page. You could append a GET parameter to that url display a success message. (or you could always do all this on the same page and just check if the id is being sent).

Also you should have this page behind someone secure login system. You don't want any user/bot able to execute that deletepage.php.

How can I prevent SQL injection in PHP?
http://php.net/manual/en/security.database.sql-injection.php
https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet#Defense_Option_1:_Prepared_Statements_.28Parameterized_Queries.29

I'm guessing you are using mysqli so take a look at this doc for prepared statements with that driver, http://php.net/manual/en/mysqli.quickstart.prepared-statements.php.

Community
  • 1
  • 1
chris85
  • 23,846
  • 7
  • 34
  • 51
  • mysqli or PDO => use bindParam if possible, it's the best practice ;) – Blag Nov 12 '15 at 00:52
  • @Blag Yea, I put notes and links in. Not sure what driver the OP is using though, and mysqli's bindparam has an underscore; whereas PDO does not. – chris85 Nov 12 '15 at 00:54
  • 1
    yes, it was more a post to the OP than you ;) (that's why I +1 you) – Blag Nov 12 '15 at 00:58
  • Thanks for the suggestions chris85. I've implemented your code on my test files and I managed to get things nearly working. It assigns the right ID to each button and even shows the echo that the item was successfully deleted when clicked, however when I look in the database the record is still there. Any ideas? I know the user from the dbconnect.php has those privileges. Also, thanks for the additional info regarding sql injection and prepared statements, I'll take a look. – seibzehn Nov 12 '15 at 16:57
  • Try `die($sql);`. After `$sql = "DELETE FROM Predictions WHERE id=" . $id; `. Then run that on the database, does it work there? This is `mysqli`? The doc says a `delete` would return true if successful and false on failure. `Returns FALSE on failure...For other successful queries mysqli_query() will return TRUE.` – chris85 Nov 12 '15 at 17:43
  • Thank you for the continued assistance! I'm happy to report it is now working all the way through. Turns out I inserted an error as I was copying your original suggestions I put `$_GET['ID'];` instead of `$_GET['id'];` --- Of course it was a matter of the wrong case that screwed me up... Anyway, thanks again for the help! – seibzehn Nov 12 '15 at 18:20
  • Is this method safe? I was thinking the same thing but the user can see where the link redirets and can modify the id when calling the deletefile.php leading to undesired results. I was wandering if there was a better way than this. Perhaps sending an ajax POST request. I'm not trying to use much javascript so i'm still going to use that method you mentioned –  Jul 02 '17 at 20:09
  • 1
    @IchHabsDrauf It is safe in terms of injections. In terms of malicious usage, no, you should have some authentication in place. Check that the requesting user owns the record they are deleting (assuming that meets the requirements) or that the user is an admin. Oh, after re-reading answer I did post that note as well, `Also you should have this page behind someone secure login system. You don't want any user/bot able to execute that deletepage.php.` – chris85 Jul 02 '17 at 20:34
  • @chris85 Yeah I know what you mean. That's how i did it. Thanx for replying. : –  Jul 03 '17 at 11:21