0

I have a page that brings up a users information and the fields can be modified and updated through a form. Except I'm having some issues with having my form update the database. When I change the update query by hardcoding it works perfectly fine. Except when I pass the value through POST it doesn't work at all.

   if (isset($_POST['new']))
    {
   $result1 = pg_query($db, 
    "UPDATE supplies.user SET 

        id = '$_POST[id_updated]',  
        name = '$_POST[name_updated]',  
        department = '$_POST[department_updated]',
        email = '$_POST[email_updated]',
        access = '$_POST[access_updated]'    

     where id = '$_POST[id_updated]'");
  if (!$result1)
 {
   echo "Update failed!!";
} else
{
     echo "Update successful;";
}

I did a vardump as an example early to see the values coming through and got the appropriate values but I'm surprised that I get an error that the update fails since technically the values are the same just not being hardcoded..

UPDATE supplies.user SET name = 'Drake Bell', department = 'bobdole',
 email = 'blah@blah.com', access = 'N'  where id = 1 

I also based the form on this link here for guidance since I couldn't find much about PostGres Online Guide

Dosh1
  • 1
  • 2
    They looks awfully insecure, you should never put post data directly into an SQL query without escaping it. Prepared statements solve this problem elegantly... – erik258 Dec 30 '15 at 22:11
  • 1
    Regardless of why this isn't working, this is [so bad](http://bobby-tables.com/). You should [**Never trust user input**](http://stackoverflow.com/questions/2794016/what-should-every-programmer-know-about-security). **Edit:** Get proper [error output](http://php.net/manual/en/function.pg-last-error.php) – FirstOne Dec 30 '15 at 22:11
  • 1
    Never build SQL using string substitution, where the input is coming from and end users. Use bind variables – James Dec 30 '15 at 22:12

2 Answers2

0

Try dumping the query after the interpolation should have happened and see what query you're sending to postgres.

Better yet, use a prepared statement and you don't have to do variable interpolation at all!

erik258
  • 14,701
  • 2
  • 25
  • 31
0

Do not EVER use data coming from external sources to build an SQL query without proper escaping and/or checking. You're opening the door to SQL injections.

You should use PDO, or at the very least pg_query_params instead of pg_query (did you not see the big red box in the manual page of pg_query?):

$result1 = pg_query($db, 
    "UPDATE supplies.user SET 
        id = $1,  
        name = $2,
        department = $3,
        email = $4,
        access = $5
     WHERE id = $6",
    array(
     $_POST[id_updated],  
     $_POST[name_updated],  
     $_POST[department_updated],
     $_POST[email_updated],
     $_POST[access_updated],    
     $_POST[id_updated]));

Also, when something goes wrong, log the error (pg_last_error()).

By the way, UPDATE whatever SET id = some_id WHERE id = some_id is either not really useful or not what you want to do.

jcaron
  • 17,302
  • 6
  • 32
  • 46
  • Gotcha thanks, I actually switched to PDO and the form works better now and now I know it's much more secure. I didn't know much about PDO originally but now I understand why its better to use. – Dosh1 Jan 05 '16 at 18:50