[PHP] Hulp nodig

[Anoniem: 613047]

Beste Tweaker's


Ten eerste ik ben nieuw op Tweakers dus ik weet niet of dit helemaal goed geplaatst is..

Mijn probleem is ik ben bezig om een dagelijkse login points systeem te maken dat een gebruiker 1 punt krijgt en daar later dingen mee kan kopen...

maar zodra mijn punt 1 is dan krijg ik de error dat ik onvoldoende points heb kan iemand mij hier bij helpen?

PHP:

<?php

$pakket = $_GET['pakket'];

$getMySQL = mysql_query('SELECT * FROM users WHERE id = "'.$_SESSION["user"]["id"].'"');
while ($data = mysql_fetch_array($getMySQL)) {

    if ($pakket == '1') { 
        if ($data['login_points'] <= '1') {
            echo 'Je hebt onvoldoende Login Points!';
        } else {
            mysql_query('UPDATE users SET login_points = login_points - "1" WHERE id = "'.$_SESSION["user"]["id"].'"');
            mysql_query('UPDATE users SET vip_points = vip_points + "2" WHERE id = "'.$_SESSION["user"]["id"].'"');
            header('location: /me');
        }
    } elseif ($pakket == '2') {
        if ($data['login_points'] <= '2') {
            echo 'Je hebt onvoldoende Login Points!';
        } else {
            mysql_query('UPDATE users SET login_points = login_points - "2" WHERE id = "'.$_SESSION["user"]["id"].'"');
            mysql_query('UPDATE users SET vip_points = vip_points + "5" WHERE id = "'.$_SESSION["user"]["id"].'"');
            header('location: /me');
        }
    } elseif ($pakket == '3') {
        if ($data['login_points'] <= '4') {
            echo 'Je hebt onvoldoende Login Points!';
        } else {
            mysql_query('UPDATE users SET login_points = login_points - "4" WHERE id = "'.$_SESSION["user"]["id"].'"');
            mysql_query('UPDATE users SET vip_points = vip_points + "12" WHERE id = "'.$_SESSION["user"]["id"].'"');
            header('location: /me');
        }
    } elseif ($pakket == '4') { 
        if ($data['login_points'] <= '8') {
            echo 'Je hebt onvoldoende Login Points!';
        } else {
            mysql_query('UPDATE users SET login_points = login_points - "8" WHERE id = "'.$_SESSION["user"]["id"].'"');
            mysql_query('UPDATE users SET vip_points = vip_points + "18" WHERE id = "'.$_SESSION["user"]["id"].'"');
            header('location: /me');
        }
    } elseif ($pakket == '5') {
        if ($data['login_points'] <= '16') {
            echo 'Je hebt onvoldoende Login Points!';
        } else {
            mysql_query('UPDATE users SET login_points = login_points - "16" WHERE id = "'.$_SESSION["user"]["id"].'"');
            mysql_query('UPDATE users SET vip_points = vip_points + "34" WHERE id = "'.$_SESSION["user"]["id"].'"');
            header('location: /me');
        }
    } else {
        header('location: /me');
    }
}


?>

[Onbekend]

Het probleem zit in regel 8 t/m 10:

if ($pakket == '1') {
if ($data['login_points'] <= '1') {
echo 'Je hebt onvoldoende Login Points!';

[Anoniem: 613047]

Onbekend schreef op vrijdag 08 augustus 2014 @ 22:31:
Het probleem zit in regel 8 t/m 10:

if ($pakket == '1') {
if ($data['login_points'] <= '1') {
echo 'Je hebt onvoldoende Login Points!';

kan je me vertellen wat er fout is?

[Morrar]

Ik weet niet wat je als pakket invult of wat er precies uit MySQL komt, maar als in MySQL staat dat je 1 punt hebt, dan zal $data['login_points'] <= '1' inderdaad waar zijn en dus krijg je die error (let op: kleiner of *gelijk* aan 1).

Paar algemene tips:
- echo de query om te testen of deze correct is.
- run de query handmatig in je DBMS (bijvoorbeeld phpMyAdmin) en zie wat je terug krijgt.
- test eventueel je if-structuur met een variabele.

[ Voor 39% gewijzigd door Morrar op 08-08-2014 22:38 ]

[Onbekend]

Experimenteer eens met de waarde van $data['login_points'] zou ik zeggen.
Je kan het beste het eerste stukje code in een nieuw php-script zetten en allerlei waarden in voor $data['login_points'] invullen. Bijvoorbeeld tekst of een cijfer of iets anders....

[Anoniem: 613047]

Zover ik weet werkt de variable login_points gewoon wel. want als ik mijn login_points op 2 zeg en ik probeer pakket 1 te kopen dan lukt het wel maar zodra ik 1 login_point heb lukt het niet

[Morrar]

Anoniem: 613047 schreef op vrijdag 08 augustus 2014 @ 22:38:
Zover ik weet werkt de variable login_points gewoon wel. want als ik mijn login_points op 2 zeg en ik probeer pakket 1 te kopen dan lukt het wel maar zodra ik 1 login_point heb lukt het niet

Dat klopt toch ook? Je test op login points <= 1, dus bij 1 punt krijg je een error (1 is gelijk aan 1)

Moet je niet gewoon testen login points < 1? Iemand moet immers minimaal 1 punt hebben om tot aanschaf over te kunnen gaan...


P.S. Haal ook de quotes weg rond de 1, die zijn overbodig.

[ Voor 15% gewijzigd door Morrar op 08-08-2014 22:44 ]

[Fish]

Geef in je echo ook even aan welke stap het is, dat debugged makkelijker
evt met de waarden

[ Voor 12% gewijzigd door Fish op 08-08-2014 22:40 ]

[Ulster Seedling]

Misschien moet je PHP's comparison operators even bestuderen:

quote: http://php.net/manual/en/language.operators.comparison.php

Example	Name	Result
$a < $b	Less than	TRUE if $a is strictly less than $b.
$a <= $b	Less than or equal to	TRUE if $a is less than or equal to $b.

Met andere woorden: 1 <= 1 is waar (true), omdat 1 gelijk is aan 1. 1 < 1 is echter niet waar (false), omdat 1 niet kleiner is dan 1.

[ Voor 14% gewijzigd door Ulster Seedling op 08-08-2014 22:48 ]

[Anoniem: 613047]

Ulster Seedling schreef op vrijdag 08 augustus 2014 @ 22:47:
Misschien moet je PHP's comparison operators even bestuderen:


[...]


Met andere woorden: 1 <= 1 is waar (true), omdat 1 gelijk is aan 1. 1 < 1 is echter niet waar (false), omdat 1 niet kleiner is dan 1.

Dit heeft me geholpen dankjewel

[Ulster Seedling]

Mocht je nog op zoek zijn naar Nederlandstalige tutorials over PHP, dan kan ik je phptuts.nl aanraden. Hier staan een aantal goede tutorials op (zoals de PHP Beginnershandleiding). De site is al een tijdje niet meer bijgewerkt, maar de tutorials zijn nu nog relevant (gezien we nog steeds bij PHP 5 zijn).

[NMe]

Nou we dat gehad hebben zal ik eens schieten op de rest van je code want er is echt een hoop dat een stuk beter kan (en in de meeste gevallen moet). Zoek om te beginnen eens op wat SQL-injection is. Ik weet niet 100% zeker of $_SESSION nou wel of niet in sommige gevallen kwetsbaar is daarvoor, maar je kan hoe dan ook beter prepared statements gebruiken. Daarbij zijn de mysql_-functies deprecated en zou je zeker als je alles net aan het leren bent beter kunnen overstappen op de mysqli_-functies of -classes of eventueel PDO.

Dan is er nog het feit dat je in het begin een query doet waar maar één resultaat uitkomt en je tóch een while gebruikt in plaats van een if. En over het merendeel van je verdere code kan ik alleen maar zeggen: DRY. Het belangrijkste hier is dat je in bijna elk pad waarin je code kan raken vanaf een bepaald punt altijd dezelfde location-header set. Als dat altijd de bedoeling is, haal hem dan zo ver mogelijk naar buiten toe.

Met bovenstaande in het achterhoofd kan je code veel korter:

PHP:

$pakket = (int)$_GET['pakket'];
$userId = (int)$_SESSION["user"]["id"];

$required_vip_points = array(
    1 => 2,
    2 => 5,
    3 => 12,
    4 => 18,
    5 => 34,
);

$getMySQL = mysql_query('SELECT * FROM users WHERE id = "'.$userId.'"');
if ($data = mysql_fetch_array($getMySQL)) {

    $failed = false;
    $required_login_points = 1 << ($pakket - 1); // << is een bit shift, dit kan ook met simpele machten van 2
    if ($data['login_points'] < $required_login_points) {
        $failed = true;
    } else {
        // je hebt maar een query nodig, ook als je twee velden wil veranderen
        mysql_query('UPDATE users SET login_points = login_points - '.$required_login_points.', vip_points = vip_points + '.$required_vip_points[$pakket].' WHERE id = "'.$userId.'"');
    }
    if ($failed) {
        echo 'Je hebt onvoldoende Login Points!';
    } else {
        header('location: /me');
        die(); // altijd een die doen na het zetten van een location header
    }
}

^^ dit zou de hele code uit je startpost moeten kunnen vervangen.

Ik heb overigens bewust toch mysql_ in plaats van mysqli_ gebruikt omdat je nog wat andere wijzigingen nodig hebt buiten dit stukje van je code om om mysqli te laten werken.

Ulster Seedling schreef op vrijdag 08 augustus 2014 @ 22:55:
De site is al een tijdje niet meer bijgewerkt, maar de tutorials zijn nu nog relevant (gezien we nog steeds bij PHP 5 zijn).

Dat zegt niks over hoe relevant de tutorials nog zijn. PHP 5.0 is ruim 10 jaar oud. Met name PHP 5.3 heeft nogal wat quality of life improvements toegevoegd, waaronder namespaces, closures en betere garbage collection. Ook 5.4 en 5.5 hadden nog wat leuke toevoegingen (traits, short array syntax, try/catch/finally-support). Iemand die nu moet beginnen met PHP leren zou ik willen aanraden om een boek te zoeken over PHP 5.5 of als dat niet lukt, 5.4.

[ Voor 27% gewijzigd door NMe op 09-08-2014 10:58 ]

[Onbekend]

Zie het volgende niet als afkraken van de code, maar als tips om jezelf te verbeteren.

PHP:

$pakket = $_GET['pakket'];

Controleer eerst of de parameter 'pakket' is meegegeven. Daarna moet je controleren of de waarde van het juiste type is. Je gaat er nu vanuit dat dit een normaal geheel getal is, maar iemand kan ook bijvoorbeeld gewone tekst of een veel te hoog getal meegeven, of iets totaal anders zoals een kommagetal of iets met exponenten.

PHP:

$getMySQL = mysql_query('SELECT * FROM users WHERE id = "'.$_SESSION["user"]["id"].'"');

Zoals NMe al heeft aangegeven is dit een ideaal punt voor een sql-injection. Verder wordt afgeraden om "SELECT *" te gebruiken maar om gewoon de kolommen mee te geven. Bijvoorbeeld "SELECT login_points ".
Daarna ga je er vanuit dat de query correct is uitgevoerd en dat het resultaat in $getMySQL staat. Voordat je met de waarde $getMySQL verder gaat werken moet je dus controleren of $getMySQL niet False is. Dat doe je bijvoorbeeld zo:

PHP:

if ($getMySQL !== False) {
    die('Query error');
}

PHP:

if ($pakket == '1')
elseif ($pakket == '2')
elseif ($pakket == '3')

Het is een nummer, dus gewoon aftesten zonde quotes.
Mocht je verschillende waarden willen aftesten, gebruik dan een switch-statement. Vooral bij lange lange if-elseif-elseif conbinaties is een switch overzichtelijker en vaak sneller.

Zoiets dus:

PHP:

switch ($pakket) {
    case 1:
        echo "Doe iets";
        break;
    case 2:
        echo "Doe iets";
        break;
    case 3:
        echo "Doe iets";
        break;
    default:
        echo "Doe niets";
        break;
}

PHP:

mysql_query('UPDATE users SET login_points = login_points - "1" WHERE id = "'.$_SESSION["user"]["id"].'"');
mysql_query('UPDATE users SET vip_points = vip_points + "2" WHERE id = "'.$_SESSION["user"]["id"].'"');

Hier update je 2x de zelfde tabel. Dit kan ook in 1 statement en dat is ook sneller.
UPDATE users SET login_points = login_points - 1, vip_points = vip_points + 2 WHERE ......


Nu heb je hier ook een mooi voorbeeld waarin je eigenlijk transacties moet gaan gebruiken.
Stel dat de gebruiker binnen 0,1 seconde 100x op de knop drukt zodat 100x het php-script (tegelijkertijd) gaat draaien.
Eerst wordt 100x keer mysql_query('SELECT * FROM users WHERE id = "'.$_SESSION["user"]["id"].'"'); uitgevoerd, en voor elk resultaat wordt het zelfde aantal 'login_points' dus opgehaald.

Maar het script gaat wel 100x mysql_query('UPDATE users SET login_points = login_points - "1" WHERE id = "'.$_SESSION["user"]["id"].'"'); en mysql_query('UPDATE users SET vip_points = vip_points + "2" WHERE id = "'.$_SESSION["user"]["id"].'"'); uitvoeren.
Het resulaat is dus dat de waarde van login_points 100 lager wordt, en de waarde van vip_points 200 hoger....
Door een transactie te gebruiken vanaf de eerste SELECT t/m de laatste UPDATE voorkom je dit probleem, want een tegelijkertijd draaiend script zal wachten totdat de transactie is voltooid.

Zoiets dus:

PHP:

try {
    // Start transactie:
    beginTransaction();

    // Doe van alles:
    mysql_query('SELECT * FROM users WHERE id = "'.$_SESSION["user"]["id"].'"');
    mysql_query('UPDATE users SET login_points = login_points - "1" WHERE id = "'.$_SESSION["user"]["id"].'"');
    mysql_query('UPDATE users SET vip_points = vip_points + "2" WHERE id = "'.$_SESSION["user"]["id"].'"');

    // En verzend alles:
    commit();
} catch (Exception $e) {
    // Er is iets fout gegaan, dus doe geen update:
    rollback();
}

[Barryvdh]

NMe schreef op vrijdag 08 augustus 2014 @ 23:13:
Dat zegt niks over hoe relevant de tutorials nog zijn. PHP 5.0 is ruim 10 jaar oud. Met name PHP 5.3 heeft nogal wat quality of life improvements toegevoegd, waaronder namespaces, closures en betere garbage collection. Ook 5.4 en 5.5 hadden nog wat leuke toevoegingen (traits, short array syntax, try/catch/finally-support). Iemand die nu moet beginnen met PHP leren zou ik willen aanraden om een boek te zoeken over PHP 5.5 of als dat niet lukt, 5.4.

En niet per ongeluk over PHP 6
Wel: http://www.phptherightway.com/#databases

[Sgreehder]

NMe schreef op vrijdag 08 augustus 2014 @ 23:13:
...

Goede bedoelingen daargelaten, de gemiddelde beginner kan dit soort terminologie voorlopig niet in context plaatsen (en terecht, want DRY draagt iets geheel anders uit - duplication of knowledge - en laat zich pas gelden op schaal).

Overigens, injectie vang je met de oorspronkelijke mysql extensie het beste af als volgt:

PHP:

mysql_query("SELECT * FROM users WHERE id = '" . mysql_real_escape_string($_SESSION["user"]["id"]) . "'");

Ik zou verder stellen dat een switch of zelfs de oorspronkelijke code de leesbaarheid hier wel ten goede komt.

[ Voor 0% gewijzigd door Sgreehder op 09-08-2014 01:14 . Reden: quotes ook altijd ]

[NMe]

Sgreehder schreef op zaterdag 09 augustus 2014 @ 01:10:
[...]

Goede bedoelingen daargelaten, de gemiddelde beginner kan dit soort terminologie voorlopig niet in context plaatsen (en terecht, want DRY draagt iets geheel anders uit - duplication of knowledge - en laat zich pas gelden op schaal).

DRY mag je toepassen op elk niveau van software-ontwikkeling tussen de architectuur en het daadwerkelijke code-kloppen.

Overigens, injectie vang je met de oorspronkelijke mysql extensie het beste af als volgt:

PHP:

mysql_query("SELECT * FROM users WHERE id = '" . mysql_real_escape_string($_SESSION["user"]["id"]) . "'");

Voor strings, ja. Voor integers is de cast die ik toepas ook voldoende en iets (doch marginaal) sneller.

Ik zou verder stellen dat een switch of zelfs de oorspronkelijke code de leesbaarheid hier wel ten goede komt.

IMO niet? Het enige dat het mogelijk wat tricky maakt is regel 16, maar als je moeite hebt met het lezen van bitshifts kun je die ook herschrijven naar:

PHP:

$required_login_points = max(1, 1 * pow(2, $pakket - 1));

...of vanaf PHP 5.6:

PHP:

$required_login_points = max(1, 1 * 2 ** ($pakket - 1));

Ik vind de bitshift makkelijker lezen.

[Sgreehder]

NMe schreef op zaterdag 09 augustus 2014 @ 11:11:
Voor strings, ja. Voor integers is de cast die ik toepas ook voldoende en iets (doch marginaal) sneller.

Voor dat soort marginale snelheidsverbeteringen zou je prepared statements in dit geval ook links moeten laten liggen

[Anoniem: 26306]

NMe schreef op vrijdag 08 augustus 2014 @ 23:13:
Ik weet niet 100% zeker of $_SESSION nou wel of niet in sommige gevallen kwetsbaar is daarvoor, ...

Ik wel. Alle variabelen in $_SESSION worden door de applicatie zelf toegewezen. Je hoeft het dus niet als user input te beschouwen zolang je er geen variabelen in stopt die wél van de user vandaan komen. $_SESSION is dus "veilig" zolang je niet iets doet als $_SESSION['uid'] = $_POST['uid']. Desondanks kun je er zoals je al zegt maar beter voor zorgen dat je sowieso alles met PDO doet want dan hoef je je er al helemaal geen zorgen meer om te maken.

[NMe]

Sgreehder schreef op zaterdag 09 augustus 2014 @ 15:55:
[...]

Voor dat soort marginale snelheidsverbeteringen zou je prepared statements in dit geval ook links moeten laten liggen

Daarom is het ook niet de reden dat ik het doe. Ik vind persoonlijk een integercast fijner omdat ik dan niet per ongeluk een string in een integerveld stop in de database en dáár dan weer errors van krijg. Integerconversie + rangecheck vind ik leesbaarder dan escapen + ctype_digit en consorten.

Linksom of rechtsom: met prepared statements heb je er sowieso geen last van en hoef je alleen over de checks na te denken, niet om het escapen.

[Sgreehder]

NMe schreef op zaterdag 09 augustus 2014 @ 16:19:
[...]

Daarom is het ook niet de reden dat ik het doe. Ik vind persoonlijk een integercast fijner omdat ik dan niet per ongeluk een string in een integerveld stop in de database en dáár dan weer errors van krijg. Integerconversie + rangecheck vind ik leesbaarder dan escapen + ctype_digit en consorten.

Linksom of rechtsom: met prepared statements heb je er sowieso geen last van en hoef je alleen over de checks na te denken, niet om het escapen.

Tja, en juist met binds voorkom je dat (PDO::PARAM_INT, etc). Hoef je ook niet tegen PHP te vechten,