MySQL/PHP: Regel in tabel updaten

[Bastiaan]

Ik ben mij sinds kort aan het verdiepen in de wereld van MySQL. Nou loop ik al tegen genoeg uitdagingen en hindernissen aan, maar ik kom er nu één tegen waar ik geen raad mee weet en niet vooruitkom met meer dan een weekend aan Google, (vooral) Stackoverflow of verschillende basic tutorials zoals deze.

Als proefomgeving ben ik mijn kilometerregistratie aan het omzetten vanaf een spreadsheet naar een database. Dit is gewoon privé-administratie, en het doel is dus vooral ter lering ende vermaak.
Ik heb de gevraagde waarden uit een database in een tabel zichtbaar middels een eenvoudige query. Eén van de functies welke ik wil toevoegen is het 'verwijderen' (mind the quotes) van een regel middels een button welke in iedere regel terugkomt. Dat gaat redelijk goed, maar het vreemde is dat wanneer ik klik/submit...

PHP:

if(isset($_POST['submit'])){
    $conn->query("UPDATE auto_kmreg SET actief='0' WHERE kilometers='183145'");
    $conn->close();
}

...er redelijk voor zorgt dat de regel met die waarde wordt verwijderd (lees: onzichtbaar is), maar dat...

PHP:

if(isset($_POST['submit'])){
    $conn->query("UPDATE auto_kmreg SET actief='0' WHERE kilometers='$kilometers'");
    $conn->close();
}

...helemaal niets lijkt te doen. Ik zeg daarbij "redelijk" aangezien ik de pagina nog handmatig moet refreshen voor het resultaat, maar dat is een zorg voor later. Ik begrijp echter niet helemaal wat ik nu kan doen om te zorgen dat de betreffende regel wordt verwijderd aan de hand van die variabele $kilometers.

Dit stukje is nu opgenomen boven de head van het document en getuige het feit dat het met een vaste waarde functioneert bij een submit van de button lijkt die plaatsing niet een oorzaak te zijn. In datzelfde document wordt de variabele op andere plekken wel herkend en kan er mee gewerkt/gerekend worden.

Kan iemand me een duwtje in de goede richting geven?

Overigens: ik realiseerde me later dat natuurlijk een tweede auto op exact dezelfde kilometerstand zou kunnen staan bij een tankbeurt, dus ik schakel over op de ID's in plaats van kilometers. Voor nu maakt dat echter geen oplossing voor het probleem.

[Stroopwafels]

Waar haal jij de variable $kilometers vandaan?

[isNull]

Maak gebruik van prepared statements. Zie dit voorbeeld.

[ Voor 3% gewijzigd door isNull op 06-04-2015 18:44 ]

[TommieW]

Wellicht een beetje off-topic, maar mogelijk ook een oplossing:
Je lijkt nu je variabele direct in de query te gooien. Als iemand nu er voor zorgt dat de variabele iets is als "; DROP ALL TABLES;", zal er aardig wat kapot kunnen gaan. Ik weet dat het voor privégebruik is, maar als je het doet, doe het dan goed.

Beetje spuit11, maar dat maakt niet uit.

[Bastiaan]

Iets verderop in het document is de tabel opgebouwd:

PHP:

    $sql = "SELECT * FROM auto_kmreg WHERE actief='1' ORDER BY Datum DESC";
    $result = $conn->query($sql);

    if ($result->num_rows > 0) {
        while($row = $result->fetch_array()) {
            $datum = $row['datum'];
            $kilometers = $row['kilometers'];
            $getankt = $row['getankt'];
            $ppl = $row['ppl'];
            $opmerking = $row['opmerking'];
                                
            $tankkosten = $getankt * $ppl;
                                
            echo '
                <tr>
                    <td class="col-lg-2"><b>' . (date("d-m-Y", strtotime($datum))) . '</b></td>
                    <td class="col-lg-2">' . number_format($kilometers, 0, ',', ' ') . '</td>
                    <td class="col-lg-2">' . number_format($getankt, 2, ',', ' ') . '</td>
                                        
etc...

En volgens mij begrijp ik ook nu ineens waarom het misloopt. Ik moet in mijn UPDATE-query die $kilometers opnieuw definieren, denk ik.

Edit: met de prepared statements wil ik inderdaad nog aan de gang, maar even één tandje tegelijk. Bedankt in ieder geval voor de tip; ik weet al dat het geen onbelangrijke is.

[ Voor 9% gewijzigd door Bastiaan op 06-04-2015 18:48 ]

[Fish]

real_escape_string niet vergeten --> http://www.sanwebe.com/20...update-using-prepared-stm

[Cartman!]

Je kan ook een tool als Adminer of phpMyAdmin downloaden/installeren en daar de query in testen, dat is soms net wat sneller testen dan in code

[ByteMe_]

Gebruik je de nieuwere MySQLi driver of de oude MySQL? Het werkt allebei, maar MySQLi is meer OOP.

[ Voor 5% gewijzigd door ByteMe_ op 06-04-2015 19:24 ]

[Douweegbertje]

RonaldDesigns schreef op maandag 06 april 2015 @ 19:24:
Gebruik je de nieuwere MySQLi driver of de oude MySQL? Het werkt allebei, maar MySQLi is meer OOP.

Op wie reageer je, en wat probeer je nu eigenlijk te zeggen..?

[Fish]

het mysql is depricated m.a.w. liever niet meer gebruiken. en de mysqli gebruiken.

http://php.net/manual/en/migration55.deprecated.php

[Martindo]

Ik zou even gaan kijken naar PDO en het maken van prepared queries waar je het gevaar van sql injecties mee weghaalt.

[ajakkes]

Ik gok dat

print "(".$kilometers.")";

Toevoegen boven de if submit regel op je pagina er uit ziet als ()

En fijn dat iedereen je eerst een perfecte manier van database query schrijven wil leren voordat je je eerste update statement uit kan voeren.

Zolang DROP TABLE != DROP LIFE hoef je van mij geen prepared statement te gebruiken. Verdrink eerst het kalf maar een keer.

[krvabo]

Martindo schreef op maandag 06 april 2015 @ 20:49:
Ik zou even gaan kijken naar PDO en het maken van prepared queries waar je het gevaar van sql injecties mee weghaalt.

Ja, dat lost echt z'n probleem op.
Naast het feit dat mysqli ook prepared statements ondersteunt.
En naast het feit dat 'real_escape_string' ook voldoet.

PDO is leuk als je vooral enterprisey code wil schrijven die vooral maar alles moet kunnen. Voor zoiets is het alleen maar onhandig. (En niet -nodig-)

---

@TS:
Je zult inderdaad (met een POST) de kilometers-variabele moeten meegeven, met bijvoorbeeld een hidden value. De manier waarop je checkt of er een POST is gedaan kan echter problemen opleveren. Sommige / veel / zoniet alle browsers zetten alleen de waarde 'submit' als je op de submitknop klikt, of de button selecteert met tab en enter drukt. Als jij een text input hebt, en je drukt daarbij op enter, zonder aan de knop te zitten, dan zal die check falen.

Het maakt met alleen een delete-knop waarschijnlijk niet zoveel uit, maar zodra je eender ander welk script maakt dan kun je hier tegenaan lopen.

Overigens is het ook best practice om variabelen buiten een querystring te houden, danwel door prepared statements (nette manier), ofwel sprintf, ofwel strings en variabelen aan elkaar plakken (maakt de queries dan weer beter leesbaar, maar is micronominaal trager). Let er altijd op dat je dus wel je variabelen veilig maakt. Dit kun je doen met mysqli_real_escape_string (of mysql_real_escape_string), integers casten of door de intval() functie halen, of dus prepared statements te gebruiken.
Op de manier waarop je nu werkt, tenzij er code mist, is je pagina kwetsbaar voor hacks.

[Bastiaan]

Voor nu zit alles achter een beveiligde omgeving; voor wat betreft security volstaat dat wel even.

ajakkes schreef op maandag 06 april 2015 @ 20:55:
Ik gok dat

print "(".$kilometers.")";

Toevoegen boven de if submit regel op je pagina er uit ziet als ()

En fijn dat iedereen je eerst een perfecte manier van database query schrijven wil leren voordat je je eerste update statement uit kan voeren.

Dat klopt; er verschijnt inderdaad niets anders dan (). Dan zit ik nu in ieder geval in de goede hoek te werken. Vooruitgang is er nog niet echt - anders had ik dat ook laten weten - maar met trial and error komen we er vanzelf.

Ik probeer nu verder zoveel mogelijk gebruik te maken van mysqli en (inmiddels) de prepared statements.
Voor wat betreft het aanleren van de perfecte manier; dat ben ik met je eens en lees ik ook even overheen. Wordt zeker niet minder gewaardeerd overigens, maar inderdaad eerst de absolute basics maar eens met goed fatsoen onder de knie krijgen.

[RedHat]

Heb je geen typefouten gemaakt in de benaming van het veld? Je kunt een var_dump($row) doen om te kijken welke waarden in de array opgeslagen zijn.

[ajakkes]

Als je een formulier verzend komen de ingevulde waardes in $_GET of $_POST terecht. Print_r($_GET);

[RedHat]

ajakkes schreef op maandag 06 april 2015 @ 21:24:
Als je een formulier verzend komen de ingevulde waardes in $_GET of $_POST terecht. Print_r($_GET);

PHP:

 while($row = $result->fetch_array()) {

[Mebus]

Zet anders dit eerst bovenaan je script:

PHP:

ini_set('display_errors', 1); error_reporting(E_ALL);

Wie weet krijg je dan wel wat extra regels tekst op je pagina

[Bastiaan]

De omschakeling naar ID's in plaats van kilometers heb ik vast doorgevoerd en inmiddels werk ik via de volgende manier:

De betreffende button heeft een actie gekregen:

PHP:

<form action="remove_registration.php?id=' . $id . '" method="POST">
<button type="submit" class="btn btn-outline btn-danger btn-xs tooltip-demo" data-toggle="tooltip" data-placement="left" title="Gegevens verwijderen." type="submit" name="submit"><i class="fa fa-trash-o"></i></button>
</form>

Het formulier waar de actie naartoe wijst gebruikt het ID vervolgens om het actieve veld op '0' te zetten:

PHP:

$url = "$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";
$parts = parse_url($url);
parse_str($parts['query'], $query);
$id = $query['id'];
$conn->query("UPDATE auto_kmreg SET actief='0' WHERE id='$id'");
header("Location: index.php");

Inmiddels ben ik dus een stuk verder gekomen, maar het is nog verre van wenselijk. Natuurlijk nog slordig en zo lek als een mandje, maar ik ben momenteel ook aan het kijken om het met ajax opgelost te krijgen. Uiteindelijk komen we er dus wel...

Bedankt voor ieders input! De functionele tips heb ik gelijk meegenomen. De tips rondom security houd ik nog even vast en pak ik in een later stadium mee.

[Mebus]

PHP:

$_GET['id']

En klaar.

En misschien is het handig om eerst dit helemaal goed uit te werken (wat meer basis php leren?) voordat je met ajax begint

[fommes]

als id geen string is hoef je er geen quotes omheen te zetten, verder zou ik $id = (int) $_GET['id'] gebruiken.

Nu is het voor persoonlijk gebruik, maar als meerdere mensen er gebruik van zouden moeten maken dan moet je wel opletten dat je ook zorgt dat je zou checken of diegene die het record wilt updaten dat ook mag.

[Bastiaan]

Ik heb me in een eerder stadium met userlogins beziggehouden en dat functioneert dan wél weer prima en met relatieve eenvoud... Die check zit er dadelijk dus ook op.

De $_GET heb ik er ook tussengeplaatst. Ziet er gelijk veel netter uit zo:

PHP:

$conn->query("UPDATE auto_kmreg SET actief='0' WHERE id=$_GET[id]");
header("Location: index.php");

[fommes]

het voordeel van typecasten $var = (int) $_GET['id']; is dat je zeker weet dat het een integer is, dus dat iemand niet ;'drop table' mee kan geven. weer een stukje veiliger. actief='0' zal waarschijnlijk ook actief=0 moeten zijn

[Cartman!]

Bastiaan schreef op dinsdag 07 april 2015 @ 17:30:
Het formulier waar de actie naartoe wijst gebruikt het ID vervolgens om het actieve veld op '0' te zetten:

PHP:

$url = "$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";
$parts = parse_url($url);
parse_str($parts['query'], $query);
$id = $query['id'];
$conn->query("UPDATE auto_kmreg SET actief='0' WHERE id='$id'");
header("Location: index.php");

Hoe je in dat stukje code uiteindelijk tot $_GET['id'] komt is natuurlijk waanzin, misschien kun je beter eerst bij de basis beginnen?

De functionele tips heb ik gelijk meegenomen. De tips rondom security houd ik nog even vast en pak ik in een later stadium mee.

Zodat je dezelfde fouten blijft maken? Astjeblieft, leer het direct goed aan voor jezelf want je gaat er een keer keihard nat op en dan baal je dat je niet hebt geluisterd.

krvabo schreef op maandag 06 april 2015 @ 21:02:
PDO is leuk als je vooral enterprisey code wil schrijven die vooral maar alles moet kunnen. Voor zoiets is het alleen maar onhandig. (En niet -nodig-)

Mwa, niet mee eens. PDO ondersteund named parameters en dat is heel erg handig. Syntax komt ook grotendeels overheen dus waar jij "enterprisey" vandaan haalt snap ik niet zo. Mocht je t nog simpeler willen kun je altijd Doctrine DBAL gaan gebruiken

[ Voor 21% gewijzigd door Cartman! op 07-04-2015 19:23 ]

[Bastiaan]

Dat het blijkbaar zo kort kan wist ik niet eerder, zo simpel is het helaas. Had ik het geweten, was dat punt natuurlijk niet eens aan bod gekomen. Dat betekent ook niet dat ik (mja; achteraf dan) begrijp wat het in beide gevallen doet.

Zoals ik als eerst al aangaf is het nogal zoeken naar duidelijk uitleg en praktische voorbeelden. Ik kom ze (behalve W3Schools, en dat is ook niet om over naar huis te schrijven) amper tegen op één of twee centrale plekken. Kant-en-klare broncode lezen heb ik weinig problemen mee, maar om het om te zetten naar eigen invulling is wat lastiger. Vandaar dat ik me hierin juist wil verdiepen. Dat mijn stappen niet de meest logische volgorde hebben geloof ik direct, maar met de opzet waar ik mee ben begonnen zijn dat gewoonweg de praktische zaken waar ik tegenaan loop (in het kader van "je moet ergens beginnen").

Mocht je een goede tip hebben voor wat betreft de door jou genoemde basis en van waar ik verder kan werken: roep! Ik sta voor alles open. Scheelt misschien ook nog één of twee topics in de toekomst

[Cartman!]

Je kan googlen op "php get query string parameter" om maar iets te noemen Ik geef je bij deze mee dat beveiliging nooit vergeten mag worden, ook niet als "het maar voor oefenen is" want je leert het jezelf verkeerd aan. Je huidige project is denk ik erg geschikt als oefening, typ dus vooral dingen in google in die je wilt doen om tot meer informatie te komen. Topics openen is overigens niet slecht, als je maar zelf moeite gedaan hebt om tot een oplossing te komen

[pedorus]

Bastiaan schreef op dinsdag 07 april 2015 @ 18:32:
De $_GET heb ik er ook tussengeplaatst. Ziet er gelijk veel netter uit zo:

PHP:

$conn->query("UPDATE auto_kmreg SET actief='0' WHERE id=$_GET[id]");
header("Location: index.php");

Dit ziet er helaas uit als een verschrikking vanuit security oogpunt. Minstens 3 dingen gaan fout:

1. GET hoort niets te veranderen, maar doet het wel. Als je iets wil veranderen, hoor je POST te gebruiken. Dit voorkomt dat er bijvoorbeeld records verloren gaan als de zoekmachine langskomt.
2. id wordt zomaar geinsert, waardoor je sql injectie kan krijgen.
3. Er is geen check of een request van de eigen site afkomt door een ingelogde gebruiker, XSRF is mogelijk.

Stel dat je een hidden variabele token in je form hebt die uniek is voor de user:

PHP:

if (!isset($_SESSION["token"]))
    $_SESSION["token"] = bin2hex(openssl_random_pseudo_bytes(10)); // iets statisch voor deze user
echo '<input type="hidden" name="token" value="'.$_SESSION["token"].'">'

Dan krijg je zoiets om die 3 problemen op te lossen:

PHP:

if (!isset($_SESSION["token"]) || $_POST["token"] !== $_SESSION["token"]) {
    $conn->close();
    die("invalid request");
}

$stmt = $conn->prepare("UPDATE auto_kmreg SET actief='0' WHERE id=?");
$stmt->bind_param("i", $_POST["id"]); // "i" want id is een integer type, s voor string, d voor double
$stmt->execute();
$stmt->close();
$conn->close();
header("Location: index.php"); // fout, in theorie wil je een absoluut path voor 100% compatibiliteit

Gek genoeg zijn er ook frameworks die dit soort dingen voor je doen

[Mebus]

Bastiaan schreef op dinsdag 07 april 2015 @ 22:31:
.

phphulp.nl is een goed begin, zelf 8-10 jaar geleden mijn kennis daar vandaan gehaald Zelf niet meer echt veel actief op phphulp maar zo nu en dan kom ik er nog weleens online.

[Bastiaan]

Mja, het is nogal een rotschop maar het mag nog best even gezegd worden: jullie hebben gelijk

Nog heel veel avonden (diep in nachten), energie, inlezen en inkloppen is wordt alles met PDO opgebouwd. Met name qua security is het goed geweest toch gelijk met PDO en prepared statements te beginnen.

En eerlijk is eerlijk: als ik nu mijn eigen broncodes teruglees is het één groot WTF-festijn