[PHP/MYSQL]Velden verwijderen uit tabel

[Leon-]

Ik ben bezig een nieuwsberichten systeem te maken, dit gaat stroef maar lukt tot nu toe.
Zo gigantisch veel weet ik (nog) niet van php, invoegen van bericht in de database lukt allemaal prima.
Maar nu moet ik via het admin panel ook berichten kunnen verwijderen, daarvoor heb ik onderstaand script geschreven:

code:

<?

//Berichten verwijderen

include('connect.inc.php');
$opdracht = "SELECT * FROM nieuws ORDER BY id DESC";
$result = mysql_query($opdracht);

while ($row = mysql_fetch_array($result)) { 
    ?>
<table>
    <tr>
        <td>Verwijder bericht:</td>
        <td><? echo $row['id']; ?><td>
        </tr>
        <tr>
        <td></td>
        <td><form method="post"><input type="submit" name="submit" value="verwijder"></form></td>
    </tr>
</table>
<?
if(isset($submit)) {
    
        mysql_query('DELETE FROM nieuws WHERE id = "'.$row['id'].'"');
    }
}
    ?>

Wanneer ik nu op de button "verwijder" klik verwijderd hij niet alleen het ID waar de button achter staat maar alles, hij leegt de hele tabel.
Ik kom er niet uit, ik heb ook al een aantal andere scripts bekeken maar daar kon ik ook niet veel wijzer uit worden.
Hopelijk kunnen jullie mij een aantal tips geven in de goede richting

[ Voor 15% gewijzigd door Leon- op 02-11-2005 18:11 ]

[NMe]

Dit staat letterlijk in onze FAQ: P&W FAQ - PHP:Mijn POST en GET forms werken niet (meer)
Je checkt op $submit, een variabele die alleen bestaat als register_globals op on staat (wat niet de bedoeling is!) Verder heb je een beetje een vreemde gedachtengang. Je hebt zoveel forms als je rijen hebt in je database, maar geen enkele van je forms geeft een waarde door, ze hebben allemaal alleen maar een submit knop.

Je kan veel beter een systeem maken waarbij je met checkboxes werkt, en je alle aangevinkte checkboxes gebruikt. Je hebt daarbij dan maar één submit knop nodig, en de checkboxes zet je zo neer dat ze arrays gebruiken.

Ga ook eens [google=sql injection attack] trouwens, dat scheelt je straks een hoop ellende.

[Twan V]

Een aanvullende tip: als je een functie (of in dit geval een scriptregel) hebt die 1 regel uit de database moet verwijderen, voeg dan aan het eind ' LIMIT 1 ' toe.

Lijkt misschien overbodig, maar als je een foutje maakt of iemand je script hackt, wordt er maar een rij verwijderd, inplaats van dat de hele tabel geleegd wordt.

[MuddyMagical]

Omdat je de IF structuur in de While lus hebt gezet die de pagina opbouwt wordt elke $row['id'] verwijderd.

Beter zou dit zijn:

PHP:

<?

//Berichten verwijderen

include('connect.inc.php');
$opdracht = "SELECT * FROM nieuws ORDER BY id DESC";
$result = mysql_query($opdracht);

while ($row = mysql_fetch_array($result)) 
{ 
    ?>
    <table>
        <tr>
            <td>Verwijder bericht:</td>
            <td><? echo $row['id']; ?>&nbsp;<input type="checkbox" name="check_<? echo($row['id']); ?>"><td>
        </tr>
        <tr>
            <td>$nbsp;</td>
            <td><form method="post"><input type="submit" name="submit" value="verwijder"></form></td>
        </tr>
    </table>
    <?
}
if(isset($submit))
{
    while($row = mysql_fetch_array($result))
    {
        if(isset($_REQUEST['check_'.$row['id']])
        {
            mysql_query('DELETE FROM nieuws WHERE id = "'.$row['id'].'"');
        }
    }
}
?>

Disclaimer: Heb al wat gedronken en dit ff snel in elkaar geflanst. SQL injections zijn nog steeds mogelijk en ik garandeer niets over de werking...

Maar kijk idd even naar die SQL injection attacks.

[ Voor 38% gewijzigd door MuddyMagical op 02-11-2005 23:24 ]

[NMe]

PHP:

            <td>Verwijder bericht:</td>
            <td><? echo $row['id']; ?> <input type="checkbox" name="check_<? echo($row['id']); ?>"><td>

check_123? Waarom niet check[123]? Een array is veel handelbaarder...

edit:

Even wat aandachtiger naar die code gekeken, en die gaat niet werken. Ten eerste gebruik je $row buiten de array, terwijl je die er alleen binnen zou moeten gebruiken. Daarnaast heb je nu nog steeds een submit button per checkbox, en ook ga je nog uit van register_globals.

PHP:

//Berichten verwijderen

include('connect.inc.php');
$opdracht = "SELECT * FROM nieuws ORDER BY id DESC";
$result = mysql_query($opdracht);

?>
  <form method="post" action="<?=$_SERVER['PHP_SELF'];?>">
    <table>
<?
while ($row = mysql_fetch_array($result))  { 
?>
        <tr>
            <td>Verwijder bericht:</td>
            <td><input type="checkbox" name="check[<?=$row['id'];?>]" /> <?=$row['titel'];?><td>
        </tr>
<?
}
?>
        <tr>
            <td>$nbsp;</td>
            <td><input type="submit" name="submit" value="verwijder" /></td>
        </tr>
    </table>
  </form>
<?
if(strtolower($_SERVER['REQUEST_METHOD']) == 'post')
{
    $ids = implode(',', $_POST['check']);
    mysql_query('DELETE FROM nieuws WHERE id IN ('.$ids.')');
}

Nu heb je nog steeds last van mogelijkheid tot SQL injectie, maar die mag je er zelf uit proberen te halen.

Overigens ga ik wel uit van het bestaan van een veld genaamd 'titel'. Het lijkt me handiger om voor de gebruiker de titel te laten zien bij het verwijderen.

[ Voor 79% gewijzigd door NMe op 02-11-2005 23:42 ]

[MuddyMagical]

-NMe- schreef op woensdag 02 november 2005 @ 23:26:
[...]

check_123? Waarom niet check[123]? Een array is veel handelbaarder...

Even wat aandachtiger naar die code gekeken, en die gaat niet werken. Ten eerste gebruik je $row buiten de array, terwijl je die er alleen binnen zou moeten gebruiken. Daarnaast heb je nu nog steeds een submit button per checkbox, en ook ga je nog uit van register_globals.

PHP:

//Berichten verwijderen

include('connect.inc.php');
$opdracht = "SELECT * FROM nieuws ORDER BY id DESC";
$result = mysql_query($opdracht);

?>
  <form method="post" action="<?=$_SERVER['PHP_SELF'];?>">
    <table>
<?
while ($row = mysql_fetch_array($result))  { 
?>
        <tr>
            <td>Verwijder bericht:</td>
            <td><input type="checkbox" name="check[<?=$row['id'];?>]" />&nbsp;<?=$row['titel'];?><td>
        </tr>
<?
}
?>
        <tr>
            <td>$nbsp;</td>
            <td><input type="submit" name="submit" value="verwijder" /></td>
        </tr>
    </table>
  </form>
<?
if(strtolower($_SERVER['REQUEST_METHOD']) == 'post')
{
    $ids = implode(',', $_POST['check']);
    mysql_query('DELETE FROM nieuws WHERE id IN ('.$ids.')');
}

Nu heb je nog steeds last van mogelijkheid tot SQL injectie, maar die mag je er zelf uit proberen te halen.

Niet voor niets een disclaimer erbij gezet...

[Leon-]

Bedankt allen Je script werkt trouwens niet -NMe-

*gaat zoeken op php.net naar strtolower (wtf is dat ) hehe*

Kzal die injection attack es opzoeken

[ Voor 61% gewijzigd door Leon- op 03-11-2005 23:36 ]

[NMe]

De mogelijkheid tot SQL injectie eruit halen lost je probleem niet op hoor, dat was gewoon een zijdelingse opmerking over de veiligheid van je script.

[Leon-]

-NMe- schreef op donderdag 03 november 2005 @ 23:33:
De mogelijkheid tot SQL injectie eruit halen lost je probleem niet op hoor, dat was gewoon een zijdelingse opmerking over de veiligheid van je script.

Ja heb had mijn post al geëdit ^^

[NMe]

Wat bedoel je met "werkt niet"? Krijg je een error? Zo ja: welke? Zo nee: wat gaat er dan fout?

[Leon-]

-NMe- schreef op donderdag 03 november 2005 @ 23:39:
Wat bedoel je met "werkt niet"? Krijg je een error? Zo ja: welke? Zo nee: wat gaat er dan fout?

Ik zal de volgende keer wat duidelijker zijn

http://www.lebo-design.nl/gw/admin/delete.php, wanneer ik op "submit" klik doet het script niks.

code:

strtolower
(PHP 3, PHP 4 , PHP 5)

strtolower -- Maak een string volledig in kleine letters
Beschrijving
string strtolower ( string str)


Geeft string met alle alfabetische karakters geconverteerd naar kleine letters. 

Weet dat 'alfabetisch' wordt bepaald door de huidige locale. Dit betekent bijvoorbeeld dat in de default "C" locale, karakters als umlaut-A (ĩ niet zullen worden geconverteerd.

Aha

[NMe]

Vervang de regels 29 en 30 eens door dit:

PHP:

    $ids = implode(',', $_POST['check']);
    $sql = 'DELETE FROM nieuws WHERE id IN ('.$ids.')';
    mysql_query($sql);
    echo $sql;

Dan zie je waar het fout gaat.

Ik heb zelf al een vermoeden: vervang regel 29 door dit, dan moet het werken:

PHP:

    $ids = implode(',', array_keys($_POST['check']));

[Leon-]

Het werkt! Super!

Maar nu moet ik het script nog begrijpen, is ook belangrijk anders kom ik niet verder

PHP:

if(strtolower($_SERVER['REQUEST_METHOD']) == 'post') 
{ 
    $ids = implode(',', array_keys($_POST['check']));

Wat doet dit stukje precies?

Met strtolower haalt hij alle hoofdletter uit het form? dus stond er origineel TEST maakt hij er nu test van?

implode(',', array_keys($_POST['check']));

Hier voegt hij de array samen? en krijgt het id?

[NMe]

Leon- schreef op vrijdag 04 november 2005 @ 00:07:
Het werkt! Super!

Maar nu moet ik het script nog begrijpen, is ook belangrijk anders kom ik niet verder

Mooi dat je dat zelf al zegt.

Met strtolower haalt hij alle hoofdletter uit het form? dus stond er origineel TEST maakt hij er nu test van?

Nee, strtolower zet de waarde van $_SERVER['REQUEST_METHOD'] om naar kleine letters. Daarna vergelijk je het met de tekst "post". In principe is die strtolower overbodig en kun je ook direct vergelijken, maar ik wist niet meer zeker over REQUEST_METHOD altijd lowercase of uppercase is, en aangezien ik geen tijd had om het op te zoeken heb ik het maar zo opgelost. In principe kun je ook dit gebruiken:

PHP:

if ($_SERVER['REQUEST_METHOD'] == 'post')

of

PHP:

if ($_SERVER['REQUEST_METHOD'] == 'POST')

...afhankelijk van wat nu de standaard waarde is. Dat zou je dus zelf even moeten opzoeken. Of je kan die strtolower natuurlijk laten staan.

implode(',', array_keys($_POST['check']));

Hier voegt hij de array samen? en krijgt het id?

Implode neemt inderdaad alle indices in je array en voegt ze samen, in dit geval met een komma ertussen. Wanneer je een checkbox doorstuurt naar een andere pagina, dan wordt de waarde alleen doorgegeven als deze checkbox ook echt aangevinkt is. Daar maak ik hier gebruik van. Aangezien ik check[$id] als naam heb gekozen voor elke checkbox, wordt $_POST['check'] op zichzelf ook een array. Implode voegt deze array samen. Check je de checkboxes met de id's 1, 4 en 12, dan bevat $ids uiteindelijk de string '1,4,12'. Die kun je dan weer heel mooi in die IN-clausule gebruiken in je query.