Ik gebruik onderstaand script om een bestand aan te roepen omzodoende een record te kunnen verwijderen na bevestiging.
Het benodigde id wordt meegegeven zie ik in de urlbalk van de pop-up, maar het lijkt of er verder niets mee wordt gedaan.
Ook krijg ik de melding:
Warning: mysqli_fetch_row() expects parameter 1 to be mysqli_result, boolean given on line 51.
Ik snap nog niet heel erg veel van je code omdat je html, js en php door elkaar gebruikt, maar in elk geval zijn de volgende zaken niet helemaal lekker: (nu wel)
Als je dan toch zo graag php binnen je html wilt printen, sluit dan je php af voor je html blok, en open deze daarna weer, en gebruikt php binnen de elementen die je wilt printen....
Thomas - 11/01/2015 15:37 (laatste wijziging 11/01/2015 16:24)
Moderator
Ik zou dit ook anders aanpakken.
Wat je uiteindelijk wilt doen (wat het effect van de verwijder-operatie zou moeten zijn) is het verwijderen van een record en een daarmee geassocieerd geupload bestand. Dit zouden de enige twee handelingen moeten zijn, ik zou dit niet combineren met output of wat dan ook, dat maakt het alleen maar minder goed leesbaar.
Daarnaast wil je waarschijnlijk dat deze operatie in zijn geheel, of in zijn geheel niet uitgevoerd wordt. Daarom is het misschien niet zo'n verstandig idee om eerst de afbeelding te verwijderen, en dan pas (proberen) het record te verwijderen... Als je database-tabel de InnoDB-engine gebruikt, kun je gebruik maken van een transactie. De enige informatie die je hoeft te versturen om een record te verwijderen is het id - de rest van de informatie kun je bij het verwijderen eerst uit de database uitlezen. Een betere volgorde zou dus zijn (mits je tabel van de InnoDB-engine gebruik maakt):
- start transactie
- lees record met id X uit met de toevoeging FOR UPDATE, en sla deze info op in een variabele
- verwijder het record met id X, als de query om wat voor reden fout gaat zou het script gestaakt moeten worden (exception, die(), exit)
- indien de query succesvol was, voer nog andere acties uit om e.e.a. op te ruimen met behulp van de eerder onthouden record-informatie
- commit transactie
Dit zorgt ervoor dat het bestand alleen verwijderd wordt als de query slaagt, en als er ergens onderweg iets fout gaat tijdens de transactie wordt de transactie teruggedraaid en wordt er dus ook niets verwijderd.
Dan nog iets anders: je code in je huidige vorm is niet echt veilig omdat je geen exit na je header() aanroep zet.
Zelfs wanneer $controle de waardefalseheeft, wordt de query uitgevoerd! Probeer dit maar eens uit.
Headers zijn output, dus tenzij je script iets afdrukt, worden deze pas aan het einde van je script (als output) verstuurd. En in jouw geval buffer je alle output (ob_start()), dus dan wordt sowieso het versturen van output uitgesteld tot het einde van je script.
Tenzij je dus direct na je header() aanroep aangeeft dat je script "klaar" is door middel van een exit statement, wordt de rest van het script gewoon doorlopen. Het is een (wijdverbreide) misconceptie dat header('Location: ...') je direct fysiek doorstuurt. Dit gebeurt normaal pas aan het einde van je script, tenzij je dus direct "ophangt" met exit.
EDIT: Tot slot is je query vatbaar voor SQL-injectie omdat je niet controleert of $_GET['id'] (of $_POST['id']) een getal bevat. Dit valt onder input filtering. Waarschijnlijk is het een goede gewoonte om alle DATA in je SQL te escapen met de daarvoor bestemde _real_escape_string() functie (al heeft dit geen effect voor numerieke waarden). Dit valt onder output escaping.
Ofwel dat (input filtering + gebruikmaking van een _real_escape_string() functie), of je maakt gebruik van prepared statements, maar die zijn in MySQLi nogal brak naar mijn mening. (En zelfs dan zou je eigenlijk -waar van toepassing- je input moeten filteren!)
In onze MySQLi tutorial wordt (onder andere) uitgebreid aandacht besteed aan security (maar ook aan transacties en dergelijke). Ik stel voor dat je deze eens aandachtig doorneemt. Hier staat heel veel informatie in.
EDIT2: Declaraties van classes zou je in aparte bestanden moeten zetten. In plaats van het includen/requiren van al deze bestanden kun je wellicht beter gebruik maken van een soort van autoloader. Dit zorgt ervoor dat enkel die bestanden worden betrokken bij een script die het script nodig heeft. Niet meer, niet minder. Daarnaast wordt je code korter (geen lijst van requires of wat dan ook).
Ook zou je wat meer object georienteerd kunnen gaan werken, hiermee kun je een script beter compartimenteren in acties in plaats van (geneste) if-elseif-else statement, wat je code gigantisch onoverzichtelijk maakt (hiermee kun je ook code en html (output) beter scheiden).
Gesponsorde links
Je moet ingelogd zijn om een reactie te kunnen posten.