Goedemiddag,

Ik ben bezig met een delete functie waarbij ik uit 3 tabellen gegevens wil wissen. Eerst heb ik het ID van de bepaalde speler opgezocht in mijn database en daarna wil ik alle gegevens met die ID verwijderen. Dit is mijn code maar het werkt niet, kan iemand mij helpen?

<?php
require('connection.php');
$player = $_POST['player_name'];
$query = "SELECT player_id FROM player WHERE player_name = '$player';";
$resultaat = mysqli_query($connectie, $query);
$rij = mysqli_fetch_array($resultaat);
$queryverwijderplayer = "DELETE FROM player WHERE player_id = $rij['player_id'];";
echo $queryverwijderplayer;
?>
Max Amelsfort op 07/11/2018 07:41:55
Ik heb nu gezorgd dat de value van de vorige pagina het ID van de speler is in plaats van de naam, daarmee heb ik dit gekregen.

Okay, maar waarom heeft dat id nog de naam player_name? Dat is op zijn zachtst gezegd misleidend. Geef alles zelf-uitleggende namen.

<?php 
require('connection.php');
$player = mysqli_real_escape_string( $connectie, $_POST['player_name']);
$queryverwijderplayer = "DELETE FROM player WHERE player_id = $player;";
$queryverwijderclub = "DELETE FROM club WHERE player_id = $player;";
$queryverwijdertime = "DELETE FROM time WHERE player_id = $player;";
$resultaatplayer = mysqli_query($connectie, $queryverwijderplayer);
$resultaatclub = mysqli_query($connectie, $queryverwijderclub);
$resultaattime = mysqli_query($connectie, $queryverwijdertime);
header('Location: beveiligdepagina.php');
exit();
?>

Uh oh. Je gebruikt hier weliswaar real_escape_string(), maar dit is niet veilig zonder dit te combineren met quotes.

real_escape_string() is géén wondermiddel. Als er niets te escapen valt, wordt er ook niets ge-escaped. Stel dat je user 6 wilt verwijderen, maar dit combineert met "OR 1=1". Voer je hier real_escape_string() op uit dan blijft dit "6 OR 1=1". Ik hoef je waarschijnlijk niet te vertellen wat er gebeurt als je deze query uitvoert:
DELETE FROM player WHERE player_id = 6 OR 1=1

(spoiler: alle players worden verwijderd)
Het feit dat je daarna(ast) allerlei data uit losse tabellen moet verwijderen baart mij ook zorgen. Dit impliceert min of meer dat dit geen echte relationele database is, en alle tabellen als los zand aan elkaar hangen :/.
Thomas van den Heuvel op 07/11/2018 15:48:35

[quote="Max Amelsfort op 07/11/2018 07:41:55"]Ik heb nu gezorgd dat de value van de vorige pagina het ID van de speler is in plaats van de naam, daarmee heb ik dit gekregen.

Okay, maar waarom heeft dat id nog de naam player_name? Dat is op zijn zachtst gezegd misleidend. Geef alles zelf-uitleggende namen.

Uh oh. Je gebruikt hier weliswaar real_escape_string(), maar dit is niet veilig zonder dit te combineren met quotes.
[/quote]

Hoe moeten de quotes er dan omheen?



[size=xsmall]Toevoeging op 07/11/2018 16:17:50:[/size]

- Ariën - op 07/11/2018 09:08:31

Ja, maar gebruik van die() is niet netjes.
Wij sterven toch ook niet als we wat fout doen? ;-)

Ikzelf gebruik de Object-georienteerde versie van MySQLi (met pijltjes, waaronder: $db->query(.....) ). In een MySQL extended class overschrijf ik de query() functie en voeg daar standaard foutafhandeling toe (als Exception).

Vanaf dan heb ik bij elke query een foutafhandeling zonder dat ik ze hoef te verpakken met if-statements.


Ik heb "die" overal gebruikt omdat ik daarmee zorg dat de rest van de code ook niet wordt uitgevoerd als de verbinding of query niet goed is. Zo heb ik dat ook geleerd
Zo doe je dat....

Dus de query wordt uiteindelijk bijv.
DELETE FROM player WHERE player_id = '8'

<?php
$player = mysqli_real_escape_string( $connectie, $_POST['player_name']);
$queryverwijderplayer = "DELETE FROM player WHERE player_id = '".$player."'";
?>

Zelf raad ik aan om de mysqli_real_escape_string() toe te voegen in de opdracht zelf. Dan heb je direct een goed overzicht of je niks vergeten bent. Het zou vervelend zijn als je toch ergens overheen leest, een rotte querie hebt die misbruik wordt, en dat eventuele persoonsgegevens op straat ligt.


<?php
$queryverwijderplayer = "DELETE FROM player
WHERE player_id = '".mysqli_real_escape_string($connectie, $_POST['player_name'])."'";
?>

En over je die(). Die moet je nooit gebruiken. De functie die() laat je script direct stoppen, en dat is nooit de oplossing. Je layout wordt opeens afgebroken, en eventuele belangrijke script daarna worden ook niet meer geladen.

Ikzelf gebruik de object-georiënteerde versie van MySQLi, en pas de foutafhandeling zo toe:
<?php
class Database extends mysqli {

var $totalQueries = 0;

function query($query)
{
$result = parent::query($query);
if($this->error) {
throw new Exception(mysqli_error($this), mysqli_errno($this));
}
return $result;
}
}
?>

Waarna ik dus De class initieer en de queries uitvoer met:
<?php
$db = new Database(DB_HOST, DB_USER, DB_PASS, DB_NAME);
$db->query("SELECT dit, dat, FROM dingen");
?>

Als er wat fout gaat in de query, dan krijg je mooi een exception voor de kiezen die je met de exception_handler kan aanpassen (of je werkt met try-catch blokken)
Een niet gevangen exception resulteert altijd in een fatal error. Op het moment dat je een weg inslaat met exceptions dan is het ook zaak dat je try-catch blokken om die functionaliteit zet... Of je implementeert dus die custom exception handler, maar het effect is dan hetzelfde als een die(), immers:
Execution will stop after the exception_handler is called.

En een try-catch eigenlijk ook, immers:
When an exception is thrown, code following the statement will not be executed, and PHP will attempt to find the first matching catch block.
Die exceptionhandler heb ik ooit eens gebouwd omdat ik die wildgroei aan try-catch blokken zat was in de losse scripts. Maar nu ik dit via een 'single-point of entry' afhandel, en dus alles via index.php laat binnensluizen zou die try-catch daar kunnen worden geplaatst. De afhandeling voor de exception gaat bij mij via een template die een templateparser verwerkt.

Maar goed, in mijn voorbeeld is de afhandeling sowieso centraal geregeld, zonder dat je je hele applicatie hoeft om te bouwen naar statements om je queries.
Precies, op het moment dat je jezelf de try-catch architectuur aanmeet is dat van invloed, of liever gezegd, dit wordt allesbepalend, op de algehele structuur van je applicatie.

Dit kun je met geen fatsoen in gaan bouwen in een verzameling van scripts die deze aanpak niet volgt, tenzij je dus de hele opbouw hier op afstemt, in welk geval je gewoon beter opnieuw kunt beginnen.

En al helemaal als het fundament (de database) nogal krakkemikkig is.
Misschien dan toch bij die database beginnen, want je kunt dit beperken tot één DELETE als je de relaties goed instelt:


<?php
$queryverwijderplayer = "DELETE FROM player WHERE player_id = $player;";
$queryverwijderclub = "DELETE FROM club WHERE player_id = $player;";
$queryverwijdertime = "DELETE FROM time WHERE player_id = $player;";
?>
Maar is deleten echt nodig? Hiermee verdwijnen mogelijk al je koppelingen, mocht je die hebben gemaakt.
Ikzelf raad aan om alles te flaggen als verwijderd. Zo voorkom je ook een 'f*ck up' als je per ongeluk het verkeerde verwijdert.
- Ariën - op 07/11/2018 18:38:30
Ikzelf raad aan om alles te flaggen als verwijderd. Zo voorkom je ook een 'f*ck up' als je per ongeluk het verkeerde verwijdert.

Inderdaad, dat had ik hierboven ook al aangehaald.

Reageren