0

I have a code which should delete a selected ID row from table by loading a remove.php file with SQL query. The problem is that it doesn't work but I get no errors or notices. My variable seems to get transfered to remove.php in URL just fine, as '<a href="remove.php?='.$Fvalue.'">' gets interpreted as

remove.php?=1

for example, where 1 is primary id of the row.

Here is all the code related to the question only:

EDIT:

Rendered HTML for products table with REMOVE button:

    <!-- Products table -->

    <table class="table">
        <thead>
        <tr>
            <th scope='col'>id </br ></th><th scope='col'>name </br ></th><th scope='col'>price </br ></th><th scope='col'>amount </br ></th><th scope='col'>category_name </br ></th>    </tr>
        </thead>
        <tbody>

        <tr><td data-id="1"><a href="remove.php?remove_id=1"> REMOVE</a></td>
<td>iPhone 7</td>
<td>800</td>
<td>15</td>
<td>Smartphones</td>
</tr><tr>
<td data-id="42"><a href="remove.php?remove_id=42"> REMOVE</a></td>
<td>Motorola </td>
<td>3000</td>
<td>5</td>
<td>Smartphones</td>
</tr><tr><td data-id="2"><a href="remove.php?remove_id=2"> REMOVE</a></td><td>Macbook Pro 2015</td>
<td>1300</td><td>10</td>
<td>Computers</td></tr><tr><td data-id="4"><a href="remove.php?remove_id=4"> REMOVE</a></td>
<td>Dell XPS</td>
<td>1400</td>
<td>6</td><td>Computers</td>
</tr><tr><td data-id="41"><a href="remove.php?remove_id=41"> REMOVE</a></td>
<td>CHROMEBOOK</td>
<td>5600</td>
<td>8</td>
<td>Computers</td></tr></tbody>

Updated PHP:

 <?php
    foreach ($newArray as $value) {
        echo '<tr>';
        foreach ($value as $key => $Fvalue) {

            $remove = $value['id'] = " REMOVE";
            if($value[$key] == $value['id']) {
                echo '<td data-id="'.$Fvalue.'">' . '<a href="remove.php?remove_id='.$Fvalue.'">' . $remove . '</a>' . '</td>';  // will show all values.
            } else {
                echo '<td>' . $Fvalue . '</td>';
            }
        }
        echo '</tr>';
    }
    ?>

remove.php

<?php
require_once ("navigation.php");
require_once("database_connection.php");
$id = !empty($_GET['remove_id']) ? $_GET['remove_id'] : null;
if($id != null) {
    $deleteProducts = "DELETE FROM `products` WHERE `id` = '.$id.'";
    mysqli_query($dbc, $deleteProducts);
}

Looking for any help to me spot any problems as I get no errors and have no idea why the code is not deleting a row in the table. Thanks.

Limpuls
  • 856
  • 1
  • 19
  • 37
  • What error are you getting? – Mureinik Mar 20 '19 at 20:25
  • @Mureinik LOL. Literally mentioned in the title and two times in OP that I get no error :D That's the problem. No errors, but code doesn't do anything. Not removing from DB – Limpuls Mar 20 '19 at 20:26
  • $id value is 0 or 1 ad The result of isset And Not The value in $_GET['$Fvalue'] – scaff Mar 20 '19 at 20:27
  • `remove.php?something=.$Fvalue ` – Alex Mar 20 '19 at 20:27
  • @Limpuls. Eap. I somehow skipped the "no" when reading. Odd. – Mureinik Mar 20 '19 at 20:27
  • My web development skills are admittedly very rusty, and were never well developed to begin with, but doesn't this design basically mean that anyone that figures out the url to that remove.php file can delete arbitrary records (based on a guessed id value) at any time? – Uueerdo Mar 20 '19 at 22:25
  • @Uueerdo As I remember there is a way to check if a user came to the url directly by typing or from clicking a link. And if he came to remove.php directly, he will get redirected back. The other think is that this page will be strictly restricted only for admin. I will use sessions to check if the current logged in user is admin. I hope somebody can give some clearer and deeper explanation and advices for securing this remove.php file – Limpuls Mar 20 '19 at 22:29
  • @Uueerdo https://stackoverflow.com/questions/33999475/prevent-direct-url-access-to-php-file – Limpuls Mar 20 '19 at 22:33
  • @Limpuls ah, that makes sense; I wasn't aware of those mechanisms. – Uueerdo Mar 21 '19 at 18:09

2 Answers2

3

You have to pass a key associated with the value you are sending;

<a href="remove.php?remove_id='.$Fvalue.'">' . $remove . '</a>

Notice the remove_id part.

Now, in your PHP you can retreive this value;

$id = !empty($_GET['remove_id']) ? $_GET['remove_id'] : null;
if($id !== null) {
    $deleteProducts = "DELETE FROM `products` WHERE `id`='{$id}'";
    mysqli_query($dbc, $deleteProducts);
}

In my code here, I also fixed an issue you had in your code. You had $id = (isset($_GET['$Fvalue']));, which will always set $id to true or false, not to the ID that you had passed.

You can also clean up your HTML variables by using a double quote instead of a single quote.

if($value[$key] == $value['id']) {
    echo "<td data-id='{$Fvalue}'><a href='remove.php?remove_id={$Fvalue}'>{$remove}</a></td>";  // will show all values.
} else {
    echo "<td>{$Fvalue}</td>";
}

BIG NOTE: Little Bobby says you may be at risk for SQL Injection Attacks. Learn about Prepared Statements with parameterized queries.

GrumpyCrouton
  • 8,486
  • 7
  • 32
  • 71
  • Thanks, I totally forgot about that. Unfortunately, it still doesn't work. No errors this time too – Limpuls Mar 20 '19 at 20:32
  • @Limpuls Can you show me how the HTML is actually rendered by using inspect element? – GrumpyCrouton Mar 20 '19 at 20:33
  • @Limpuls Did you implement my whole answer? – GrumpyCrouton Mar 20 '19 at 20:36
  • Updated OP. Check t – Limpuls Mar 20 '19 at 20:40
  • And yeah, I know about Little Bobby and prepared statement. I started simply and had no time to implement that. Will update my security later, even though it's not necessary as this is not commercial project, just personal :) – Limpuls Mar 20 '19 at 20:44
  • @Limpuls It's easier and faster to just do it right the first time. You could use a PDO class like my [GrumpyPDO](https://github.com/GrumpyCrouton/GrumpyPDO) class where your entire query would be `$db->run('DELETE FROM products WHERE id=?', [$id])` and it would be much more secure. – GrumpyCrouton Mar 20 '19 at 20:48
  • True, that's a good practice to keep as now I always stick to core functionality and always promise myself that little details as validation and security will be improved later. However, I don't think this is related to my question and problem. Whether it's secure or not, PDO or not, this should be working and it's not:) Looking for more help. – Limpuls Mar 20 '19 at 20:50
  • @Limpuls The HTML is being rendered properly, did you implement my PHP code? If so, can you do `var_dump($id)` in the PHP script and let me know what the result is? – GrumpyCrouton Mar 20 '19 at 20:59
  • Yeah I did as you can see in OP. `var dump($id);` outputs `string(2) "42"` so it's all good. It starts to look like there is somthing wrong with my DB. But other queries in other files works just fine. And this DELETE query is also correct and simple. Any ideas to debug DB query and stuff like that? – Limpuls Mar 20 '19 at 21:03
  • Solved. The problem was my MySQL query. I was concatenating the `$id` variable in `WHERE` clause, but just having single quotes, like this `WHERE id = '$id'` solved the problem. Weird, huh? :D Update your answer with my added fix and I will accept it :) – Limpuls Mar 20 '19 at 21:16
  • @Limpuls that is already in my answer, though I surrounded it with curly brackets, it's the same thing `\`id\`='{$id}'` (as long as the string is surrounded by double quotes) . Also, switching to parameterized queries would have also solved your issue which is also in my answer – GrumpyCrouton Mar 20 '19 at 22:17
0

Using the get method, you need key-value pairs in your url.

remove.php?id=1

Where id is the key and 1 is the value.

DigiLive
  • 1,093
  • 1
  • 11
  • 28