Code is updating all db rows when only one should be triggered by link

Question

I've come up completely empty-handed on how to make this work.

What I have is a table listing all results from the database, at the side of each row I have a button that -hopefully- allows people to "retire" that result with a simple 0/1 check. Now, the link itself works, but it submits every single button on the page, even though just one has actually been clicked.

I did try this at first as a form with a single submit button for each row; same result. You can see that there are a series of buttons (edit, transfer, breed) and then the (retire) button that should trigger an update marking a column in the db with a '1', from being a '0.'

How can I resolve this?

Image for assistance in visualization; the last button on the far right of each row should run this update when clicked.

Showing visualization: https://i.stack.imgur.com/CulHt.png

Showing SQL (which is correct): https://i.stack.imgur.com/hMbKg.png

echo '<a href="edit_horse.php?id='. $horseID .'" class="btn btn-sm btn-icon btn-light me-2" data-bs-toggle="tooltip" data-bs-placement="top" title="Edit"><i class="fas fa-pencil-alt"></i></a>';
echo '<a href="transfer_horse.php?id='. $memberID .'&horse='. $horseID .'" class="btn btn-sm btn-icon btn-light me-2" data-bs-toggle="tooltip" data-bs-placement="top" title="Transfer"><i class="fas fa-exchange-alt"></i></a>';
echo '<a href="breed_horse.php?id='. $memberID .'&horse='. $horseID .'" class="btn btn-sm btn-icon btn-light me-2" data-bs-toggle="tooltip" data-bs-placement="top" title="Breed"><i class="fas fa-venus-mars"></i></a>';

$retireSQL = "SELECT * FROM horses WHERE id = '$horseID'";
$retire = mysqli_query($sqlconnect, $retireSQL);

if(mysqli_num_rows($retire) != 0){
  $retire_link = '<a class="btn btn-sm btn-icon btn-light me-2" href="my_horses.php?page='. $page .'&id='. $memberID .'&horse='. $horseID .'&action=retire" data-bs-toggle="tooltip" data-bs-placement="top" title="Retire"><i class="fas fa-heart-broken"></i></a>';
}

if(isset($horseID)) {
  if (isset($action)) {
    if ($action == 'retire') {
      $retire_insertSQL = "UPDATE horses SET retired = 1 WHERE id = '$horseID'";
        $retire_insert = mysqli_query($sqlconnect,$retire_insertSQL) or die(mysqli_error($sqlconnect));
        echo $retire_insertSQL;
      }
    }
  }

  echo $retire_link;

Lurking didn't land you on [SQL parameterization](https://stackoverflow.com/q/60174/1422451)? :) — Parfait, Jul 30 '22 at 01:05
What `datatype` do you use for your `retired`? And also check if there is no trigger or an on update changes in you database. — Ibrahim Hammed, Jul 30 '22 at 01:07
Please show how `$horseID` and `$action` is set after button click. From `$_POST`? — Parfait, Jul 30 '22 at 01:08
@IbrahimHammed it's a simple INT(2) column. The changes are working, it's that it's updating ALL of the rows shown on the page (pagination is set to 50 rows, so all 50 on the page get updated to retired when you click just one button). — NatashyaVee, Jul 30 '22 at 01:11
@Parfait I think this is where the process is failing is that I'm attempting to do it all in a single page; which is causing some complexity. The query itself (ran in phpmyadmin) works fine and only targets a single horse. However, with how I've structured it, it's triggering every result on the page. I did have it getting from $_POST as well as the structure above, but neither worked (or, they both work, but submit the result to rows that aren't triggered via their ID). — NatashyaVee, Jul 30 '22 at 01:13
@Parfait The URL you are pushed to is an addendum to the current page (my_horses.php); which then becomes: /my_horses.php?page=1&id=1&horse=3078&action=retire (adding in horseID and action). — NatashyaVee, Jul 30 '22 at 01:16
echo your `$retire_insertSQL` before updating db and `exit();` to see what query its actually running. — Ibrahim Hammed, Jul 30 '22 at 01:17
@IbrahimHammed -- attached a new image above in the main question showing the query. (UPDATE horses SET retired = 1 WHERE id = 'VAR') — NatashyaVee, Jul 30 '22 at 01:19
Please clarify your specific problem or provide additional details to highlight exactly what you need. As it's currently written, it's hard to tell exactly what you're asking. — Community, Jul 30 '22 at 12:10

score 0 · Answer 1 · answered Jul 30 '22 at 02:35

0

I figured it out! I was calling the same variable established in the while loop, and comparing it to itself instead of the actual horseID I established and pushed to a URL. So, this is the working code, below:

<?php
// in the header of the page I ran a fresh $_GET against the horseID that I established in the link via variables
$horse_set_id = @($_GET['horse']);

echo '<a id="'. $horseID .'" class="btn btn-sm btn-icon btn-light me-2" href="my_horses.php?page='. $page .'&id='. $memberID .'&horse='. $horseID .'&action=retire" data-bs-toggle="tooltip" data-bs-placement="top" title="Retire"><i class="fas fa-heart-broken"></i></a>';
                                                    
if (isset($action)) {
  if ($action == 'retire' && $horse_set_id == $horseID) {
                                                     
  $retire_insertSQL = "UPDATE horses SET retired = 1 WHERE id = '$horse_set_id'";
                                                            
  $retire_insert = mysqli_query($sqlconnect,$retire_insertSQL) or die(mysqli_error($sqlconnect));
  echo $retire_insertSQL;
  }
}
?>

answered Jul 30 '22 at 02:35

NatashyaVee

27
6

Then, mark the solution yourself – Ibrahim Hammed Jul 30 '22 at 10:21
Pleased you figured it out! It would be worth casting the horse_set_id to an integer (if it is indeed a number) otherwise your query could be modified by a web user in various clever ways (e.g. to drop the database). – halfer Jul 31 '22 at 13:38
@halfer -- can you give me some more detail on what you'd do here? I'm all self-taught so I really don't know much about securing the queries. Would love some advice if you have some to share! – NatashyaVee Jul 31 '22 at 19:08
Assuming a horseID is a number, you could do `$horse_set_id = (int) @($_GET['horse']);`. Then in the worst case scenario your horseID would be zero, forced by the integer casting (casting a non-number to int results in 0). – halfer Jul 31 '22 at 20:38
To understand the problem, consider what would happen if someone sent the query string `?horse=';DROP TABLE horses; --`. This would result in a two-statement query: `UPDATE horses SET retired = 1 WHERE id = ''; DROP TABLE horses; --123'` (with a commented out bit at the end). Your SQL run function `mysqli_query()` might throw an error, it might not - but one should not take the risk. – halfer Jul 31 '22 at 20:40
Ideally you should look at [parameter binding](https://stackoverflow.com/a/16612474), which you could do after you get your current code safe. – halfer Jul 31 '22 at 20:42
If you have other query string input in your app, make sure you go through it meticulously, and at least use the `(int)` trick for integer values. Strings are harder, and it's honestly better to use parameter binding for all value injection. – halfer Jul 31 '22 at 20:47

Code is updating all db rows when only one should be triggered by link

1 Answers1