PHP PDO issue

Discussion in 'PHP' started by stephan2307, Feb 6, 2021.

0
  1. #1
    Got an odd one.

    Just started working on a new project. Currently working on a user class.

    The user class will have a save function where it updates the relevant tables related to the user account. The function looks like this:

    

 function save() {
            global $pdo;
           
            $sql = 'UPDATE user_account SET ';
           
            $counter = 0;
            foreach ( $this->details as $key=>$field) {
                if ( $key != 'ua_id') {
                    if ($counter > 0 ) {
                        $sql .= ', ';
                    }
                   
                    $sql .= ''. $key.'=:'.$key;
                    $counter++;
                }
            }
           
            $sql .= ' WHERE ua_id=:ua_id LIMIT 1';
       
            $stmt =$pdo->prepare($sql);

            foreach ( $this->details as $key=>$field) {
                if ($key == 'ua_id') {
                    $field = (int)$field;
                }
                $stmt->bindParam(':'.$key,$field);
            }
          
            $stmt->execute();           
        }
    PHP:
    $this->details is basically an array of all the fields of the user_account table.
    The problem that I have, is that the query executes but no row is affected so no changes are made. $stmt->errorInfo(); shows no error. When I copy the query generated in the save function and then replace the placeholders with the values, it works perfectly fine.

    I know it will be a small mistake that I made but I can't tell where.

    Any suggestion what I can try?
     
    stephan2307, Feb 6, 2021 IP
  2. stephan2307

    stephan2307 Well-Known Member

    Messages:
    1,277
    Likes Received:
    33
    Best Answers:
    7
    Trophy Points:
    150
    #2
    hmm interesting that if I don't use bindParam for the ua_id and put it like this 
    
$sql .= ' WHERE ua_id='.$this->details['ua_id'].' LIMIT 1';
    PHP:
    then it all works fine.

    Any idea why that is?
     
    stephan2307, Feb 6, 2021 IP
  3. JEET

    JEET Notable Member

    Messages:
    3,832
    Likes Received:
    502
    Best Answers:
    19
    Trophy Points:
    265
    #3
    echo the output of this line:

    $field = (int)$field;
    echo $field; exit;

    see what it echos.
     
    JEET, Feb 8, 2021 IP
  4. sarahk

    sarahk iTamer Staff

    Messages:
    28,807
    Likes Received:
    4,534
    Best Answers:
    123
    Trophy Points:
    665
    #4
    This seems simpler

    $sql = 'UPDATE `user_account` SET ';

$keys = array_keys($this->details);
$counter = count($keys) - (in_array('ua_id', $keys)? 1:0);

$sql .= implode(', ', array_map(
    function ($k) { return sprintf("%s = :%s", $k, $k); },
    array_keys($this->details)
));

$sql .= ' WHERE ua_id=:ua_id LIMIT 1';
            // if ua_id isn't in $this->details then we have to find it and add it or the update won't work

$stmt =$pdo->prepare($sql);

$stmt->execute($this->details);
    PHP:
     
    sarahk, Feb 8, 2021 IP
  5. stephan2307

    stephan2307 Well-Known Member

    Messages:
    1,277
    Likes Received:
    33
    Best Answers:
    7
    Trophy Points:
    150
    #5
    I did a var_dump and it was an integer.

    I changed bindParam to bindValue and it seemed to have fixed the issue. What id the actual difference between the two?
     
    stephan2307, Feb 9, 2021 IP
    JEET likes this.
  6. malky66

    malky66 Acclaimed Member

    Messages:
    3,997
    Likes Received:
    2,248
    Best Answers:
    88
    Trophy Points:
    515
    #6
    malky66, Feb 9, 2021 IP
    JEET likes this.
  7. JEET

    JEET Notable Member

    Messages:
    3,832
    Likes Received:
    502
    Best Answers:
    19
    Trophy Points:
    265
    #7
    @malky66

    So in his original example code, he is binding everything to $field
    meaning, all fields in his table got updated with one single value? The last value of $field
     
    JEET, Feb 9, 2021 IP
  8. stephan2307

    stephan2307 Well-Known Member

    Messages:
    1,277
    Likes Received:
    33
    Best Answers:
    7
    Trophy Points:
    150
    #8
    Yeah that what I think happened. Echoing out $field while I was looping over them wouldn't have helped as it would have bound it to the last value of field.
     
    stephan2307, Feb 10, 2021 IP
    JEET likes this.
  9. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #9
    I find this code... troubling. One of the whole reasons mysql_ functions went the way of the dodo was to make us STOP slopping variables into query strings.Providing the fields dynamically like this is just begging for trouble and is something I would HIGHLY advise against.

    PLEASE tell me $this->details isn't coming from client-side!
     
    deathshadow, Feb 14, 2021 IP
  10. SpacePhoenix

    SpacePhoenix Well-Known Member

    Messages:
    197
    Likes Received:
    28
    Best Answers:
    2
    Trophy Points:
    155
    #10
    Prepared statements should be used for absolutely every query, no matter where the source of the data being used is. Can you guarantee that a source of data will never be from a user? I know I couldn't. By using prepared statements with absolutely every query, you don't risk accidentally creating a security risk if the source of data for a query gets changed to user submitted data at a later date
     
    SpacePhoenix, Feb 14, 2021 IP