Alle backticks verwijderen zoals Arjen zegt, leest veel makkelijker en zou naar verluid beter zijn (vraag me nu wel niet waarom:$)

Nog een tip
Maak van die $_GET['id'] eens mysql_real_escape_string ( $_GET['id] )
Pas de function mysql_real_escape_string() toe bij elke GPC (get-post-cookie) voor je queries.

Dat voorkomt SQL injecties > veiligheid

Dit toch gewoon??

PHP Code:

<?php

$query = mysql_query("DELETE FROM diplomas WHERE id ='". htmlspecialchars(mysql_real_escape_string($_GET['id'])) ."' AND diplomavan = '". htmlspecialchars(mysql_real_escape_string($_SESSION['TCID'])) ."'") or die(mysql_error()); 
                         
if($query)
    { 
        echo'<tr bgcolor="#D8F8CF"><td colspan="7"><small><b>Diploma succesvol verwijderd</b></small></td></tr>'; 
        echo '<meta http-equiv="refresh" content="1;URL=/mijncv/diplomas/">'; 
    } 
else 
    { 
        echo'<tr><td colspan="7"><small><b><font color="red">Diploma niet gevonden in de database of diploma is niet behaald door jou</font></b></small></td></tr>'; 
    }
?>

Je $_GET['id'] variabele is altijd een getal naar mijn weten, dus je kan ook bovenaan je script het volgende zetten:

PHP Code:

if(!is_numeric($_GET['id']) OR !is_numeric($_SESSION['TCID'])) {
exit('Er is een fout opgetreden!');
} 


Hierdoor hoef je geen htmlspecialchars (is sowieso overbodig, gebruik je meer bij de output om te zorgen dat men je layout niet verkloot) en een mysql_real_escape_string te gebruiken in je mysql query (het is natuurlijk altijd goed om dit te gebruiken, ook al is het in sommige gevallen overbodig).

Nja, mijn mening luidt dat je geen enkele mysql_real_escape_string te weinig kan zetten.
Het zal je applicatie er niet echt op doen vertragen;-)

Origineel gepost door Arjen Rademaker

Je $_GET['id'] variabele is altijd een getal naar mijn weten, dus je kan ook bovenaan je script het volgende zetten:

PHP Code:

if(!is_numeric($_GET['id']) OR !is_numeric($_SESSION['TCID'])) {
echo 'Er is een fout opgetreden!';
exit();
} 


Hierdoor hoef je geen htmlspecialchars (is sowieso overbodig) en een mysql_real_escape_string te gebruiken in je mysql query (het is natuurlijk altijd een goed om dit te gebruiken, ook al is het in sommige gevallen overbodig).

Het werkt ook op de manier zoals ik het heb geplaatst.. Lijkt me een beetje onnodig om dat te bekritiseren terwijl je zelf overbodige code plaatst als een echo voor een lege exit. exit dan gewoon als;

PHP Code:

<?
exit('ID Variabele is niet nummeriek');
?>

Maar genoeg daarover.

Ik bekritiseer niets, ik draag alleen alternatieven aan. Doe er mee wat je wilt, het is goed bedoeld.
Een voordeel van een vroege check of de id en tcid wel numeriek zijn is dat je geen onnodige query hoeft uit te voegen.

Bedoel het ook niet zo hoor. Ieder zijn eigen manier van scripten, en dat is wat het vak zo fijn maakt.

Misschien heb je ook ergens wel een punt met die query uitsparen.

Origineel gepost door Arjen Rademaker

Je $_GET['id'] variabele is altijd een getal naar mijn weten, dus je kan ook bovenaan je script het volgende zetten:

Je kraamt onzin uit en je werkt zo antwoorden weg die wel juist zijn. $_GET en $_POST kunnen aangeraakt worden door iedere bezoeker van een website. Het MOET beveiligd worden! Beveilig je die dus niet dan is een SQL injection met de ogen dicht nog wel mogelijk.

Meer informatie over beveiligen:
http://www.phpfreakz.nl/downloadz/we...king_huide.pdf

Origineel gepost door Arek van Schayk

Je kraamt onzin uit en je werkt zo antwoorden weg die wel juist zijn. $_GET en $_POST kunnen aangeraakt worden door iedere bezoeker van een website. Het MOET beveiligd worden!

http://www.phpfreakz.nl/downloadz/we...king_huide.pdf

Je leest de zin verkeerd denk ik (sorry voor de onduidelijkheid). Ik bedoel dat een match in de database betekent dat de $_GET['id'] altijd een getal is (immers is de id waarschijnlijk een integer met een primary key). Hierdoor kan je voordat je de query uitvoert eerst controleren of de $_GET['id'] wel een getal is, anders is het al bij voorbaat onnodig om een query uit te voeren aangezien het nooit een resultaat oplevert (en hierdoor is het onnodig om nog andere checks uit te voeren in de query, echter is het wel goed om standaard mysql_real_escape_string te gebruiken).

Beide opties waren juist, echter vind ik het wat overbodig om dan de tweede optie te posten.

Verzorgt alleen maar onduidelijkheden en zulke reacties in dit topic waardoor er alleen maar meer verwarring ontstaat..

Bij nader inzien geef ik je gelijk als het gaat om het kader van het zo vriendelijk en efficiënt mogelijk opstellen van je codering, zo werk ik namelijk ook. Alleen is het vaak al een hele kunst om de gebruikers hier uit te leggen dat ze de input moeten beveiligen tegen dit soort kwaad heden laat staan ze uitleggen dat ze dit efficiënter kunnen opstellen en waarom.

Daarnaast kan ik je alleen nog even de tip geven dat je niet teveel gebruik moet maken van is_numeric(), 1e4 is namelijk ook numeriek waardoor de functie true zal terug geven.

De ctype functies zijn hier wel geschikt voor, zo controleer je met ctype_digit iedere char van een string of het een digit is.

"Checks if all of the characters in the provided string, text , are numerical."

http://nl2.php.net/ctype
http://nl2.php.net/ctype_digit

@Robin: Ik vind het echt onzin wat je nu zegt. Topics zijn ook bedoeld als naslagwerk voor andere mensen. De meeste mensen die wat willen leren vinden het juist fijn dat er alternatieven worden aangedragen om zo eventuele andere problemen (zoals snelheidsproblemen bij grote websites) op te lossen of te doen verminderen. Dat jij hiervoor niet openstaat is jouw probleem, echter zul je dan weinig leren...

@Arek: Je hebt helemaal gelijk!

@Arjen

Je geeft zelf aan dat je moet openstaan voor verbeteringen.

Als ID (in dit verhaal) wil je natuurlijk geen numerieke waardes zoals 1e4 of 11000.234 waar is_numeric() true op terug geeft. Ctype_digit ziet de 1'e'4 en de 11000'.'234 niet als digit dus zal hij false terug geven. Dat is wat je wilt dus dat is wat je dient te gebruiken.

Het beste wat je vandaag kan doen om nummers te identificeren is volgens mij een simpele preg_match:

Code:

<?php

if ( preg_match("/^-?[0-9]+$/", $_GET['id']) AND preg_match("/^-?[0-9]+$/", $_SESSION['TCID']) ) {

    // query om te zien of deze rij ook daadwerkelijk bestaat mét mysql_real_escape_string.

} else {

    // gepaste boodschap

}

?>

Zo is alles veilig lijkt me

Natuurlijk is dat niet het beste, dat is juist het meest omslachtig omdat PHP hier functies voor heeft.

De preg_match is wel sneller als de is_numeric en ctype_digit, op dat punt mag je jezelf gelijk geven :)
Aanvullend bericht:
Uhmmm sorry, preg_match() is alleen sneller als je het goed gebruikt en de integer niet langer is als 3 tekens lang ;)