updated ubuntu from version 14 to 16. was mysql and now in pdo

Question

The code was done in mysql and now i converted to PDO. Atleast i think so. Im not that good of a coder and just learning. But this code was working while in mysql and since i changed over to PDO it doesnt do its job. It is entended to show gold and how long ago the gold was scanned. Its for a game called kingsofchaos . Some code isnt shown but there is a greasemonkey(javascript) code that gets the info and the php does the rest for show and database data. This is the link that this code is for. http://www.kingsofchaos.com/battlefield.php?start=0 Now the GM gets the info required and far as i know if gold is shown then it updates the database, Havent got that far yet to test it. Cause normally the ??? is when this php is supposed to show the gold and how long ago it was scanned by another user. Ok i hope i left enough info for this but if not just let me know and ill do my best to provide.

<?php
require_once("ban.php");
if ($login==0) { die(); }

$list = $_POST['list'];
$list = str_replace("[/d]", "", $list);
$list = explode("[d]", $list);
array_shift($list);

$servername = $config['sqlserver'];
$dbname = $config['sqldb'];
$db = new PDO("mysql:host=$servername;dbname=$dbname",  $config['sqluser'], $config['sqlpass']);
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

for ($i=0; $i<count($list); $i++) {

$start = strpos($list[$i],"u=")+2;
$end = strpos($list[$i],"*");
$user = trim(substr($list[$i],$start,$end-$start));
/* $user = mysql_real_escape_string(trim(substr($list[$i],$start,$end-$start))); */

$start = strpos($list[$i],"g=")+2;
$end = strpos($list[$i],"*o=0*");
$gold = trim(substr($list[$i],$start,$end-$start));
//print_r($gold);
/* $gold = mysql_real_escape_string(trim(substr($list[$i],$start,$end-$start))); */

$start = strpos($list[$i],"t=")+2;
$end = strpos($list[$i],"--");
$size = trim(substr($list[$i],$start,$end-$start));
/* $size = mysql_real_escape_string(trim(substr($list[$i],$start,$end-$start))); */

$start = strpos($list[$i],"s=")+2;
$sid = trim(substr($list[$i],$start,-1));

try{
    $growth = $db->prepare("INSERT INTO `growth` (id, name, size, date) VALUES ('$sid', '$user', '$size', '".time()."')");
    $growth->execute();
//From this part to //end doesnt work yet, From information ive gotten from you all, i have got all the other parts working far as i can tell..
    if ($gold == "???") {
        $stats = $db->prepare("SELECT gold, goldage FROM `stats` WHERE id='$sid' AND name='$user' ");
        $stats->execute();
        $stats = $stats->fetch(PDO::FETCH_ASSOC);
        //$stats = mysql_fetch_array($stats);           
        $gold2 = number_format($stats['gold']);
        if (!$gold2) { $gold2 = "???"; }
        if (!$stats['goldage']) { $gold2 = "???"; $goldage = "never updated"; } else { $goldage = duration(time()-$stats['goldage'],1)." ago"; } echo $user.$goldage.";".$gold2." Gold*";
//This is supposed to loop through 20 names on each page and show gold values and how long ago for each. 
//end
    } else {
        $check = $db->prepare("SELECT COUNT(*) FROM `stats` WHERE id='$sid' AND name='$user' LIMIT 1");
        $check->execute();
        //$check->fetchAll();
        $check = $check->fetch(PDO::FETCH_ASSOC);
        //$check = mysql_fetch_array($check);
        //$check = $check->rowCount();
        //$check = $check['COUNT(*)'];
        if ($check<1) {
            $query = $db->prepare("INSERT INTO `stats` (id, name, size, gold, goldage) VALUES ('$sid', '$user', '$size', '$gold', '".time()."')");
            $query->execute();
        } else {
            $query = $db->prepare("UPDATE `stats` SET size='$size', gold='$gold', goldage='".time()."' WHERE id='$sid' AND name='$user'");
            $query->execute();
        }
    }
}
catch(PDOException $e) {
    echo "Error: " . $e->getMessage();
}
}

function duration($seconds,$max_periods)
{
$periods = array("year" => 31536000, "month" => 2419200, "week" => 604800, "day" => 86400, "hour" => 3600, "minute" => 60, "second" => 1);
$i = 1;
foreach ( $periods as $period => $period_seconds )
{
    $period_duration = floor($seconds / $period_seconds);
    $seconds = $seconds % $period_seconds;
    if ( $period_duration == 0 )
    {
        continue;
    }
    $duration[] = "{$period_duration} {$period}" . ($period_duration > 1 ? 's' : '');
    $i++;
    if ( $i >  $max_periods )
    {
        break;
    }
}
return implode(' ', $duration);
}
$db = null;

?>

You should read the php manual about PDO (or a tutorial about it) to well understand what are *prepared statements*. — Casimir et Hippolyte, Jul 11 '17 at 20:53
[Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)***. Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! — Jay Blanchard, Jul 11 '17 at 20:56
learn about the DATETIME column type as well. It looks like a lot of calculation to get from seconds to what you actually need — Ivo P, Jul 11 '17 at 21:01
Is there something missing from the line `$growth->execute(); */` — Nigel Ren, Jul 11 '17 at 21:05
I did have some code commented out but when i pasted it all in here i just forgot to remove that comment line. But from where If ($gold == ???) . Im not sure i have any of the select parts or insert parts right. And yea kinda figured that there was possibilities for sql injection. So if can lead me in the right direction to fix all that on this php page. Greatly appreciated with it an thx to all that helps. If it were mysql i can easily stop sql injections but since the update on vps i have to use PDO and im not that familiar with PDO and how it all works. — Erebus, Jul 11 '17 at 21:17

Ivo P · Answer 1 · 2017-07-11T21:19:58.323

I am not sure about what the question is, but I have remarks on your code:

 $growth = $db->prepare("INSERT INTO `growth` (id, name, size, date) VALUES ('$sid', '$user', '$size', '".time()."')");
    $growth->execute();

Make use of the possibilities of pdo here:

 $growth = $db->prepare("INSERT INTO `growth` 
                       (id, name, size, date) 
                       VALUES (:sid, :user, :size, '".time()."')");
    $growth->bindValue(':sid', $sid);
    $growth->bindValue(':user', $user);
    $growth->bindValue(':size', $size);

    $growth->execute();

This also goes for the other queries. This way pdo will make sure your input is escaped properly. So a username like "a'o" will not break the query, or do harm

And should the query be in a loop: you can prepare it only once (before starting the loop) and just bind the correct values and execute inside the loop

Addendum for comments:

$check = $db->prepare("SELECT COUNT(*) counted
                       FROM `stats` 
                       WHERE id=:sid AND name=:usr 
                      -- limit 1 will give only 1!
                       LIMIT 1");
$check->bindValue(':sid', $sid);
$check->bindValue(':usr', $user);

        $check->execute();
        while($row = $check->fetch(PDO::FETCH_ASSOC)) {
           if ($row['counted']<1) {
               // do the binding thing here again
               $query = $db->prepare("INSERT INTO `stats` (id, name, size, gold, goldage) VALUES ('$sid', '$user', '$size', '$gold', '".time()."')");
               $query->execute();
           } else {
               $query = $db->prepare("UPDATE `stats` SET size='$size', gold='$gold', goldage='".time()."' WHERE id='$sid' AND name='$user'");
               $query->execute();
           }
       }

fails:

you limit the query to 1 row only. remove limit 1 to get more
you overwrite $check. I use $row above
You cannot compare $check with < 1: it is an array. Use $row['counted']

Ok ive added what you have suggested for the $growth part. But that is working now but this part and on is what aint working. $stats = $db->prepare("SELECT gold, goldage FROM `stats` WHERE id='$sid' AND name='$user' "); $stats->execute(); $stats = $stats->fetch(PDO::FETCH_ASSOC);This part of the code is to return alot of data back for each user that show on each page. There are 20 that show for each page. And it only returns the first user at top and not the other 19 below them. Sorry if i bunched up alot of words here and hope you can read it. Its my first time using site — Erebus, Jul 11 '17 at 21:07
you are only fetch-ing the result once. So you get the first row. Do it in a loop. I will add to answer — Ivo P, Jul 11 '17 at 21:13
ok the $check parts are now working, i still havent got the code above it working yet which is $stats and $gold statements not working but will try some of this you have done for me with it. and again to all that helps with this greatly appreciated. — Erebus, Jul 11 '17 at 21:32
The recipe is simple: from the original query you replace $var with :somelabel and you make sure you removed the ' ' as well. Then you add the bindValue() lines, mentioning the :somelabel and the variable — Ivo P, Jul 11 '17 at 22:36

updated ubuntu from version 14 to 16. was mysql and now in pdo

1 Answers1