-1

I am attempting to create a form that, when submitted, will update the values that were inserted into the form and leave the unanswered values unchanged.

My idea was to create a loop that would set all the $_POST keys and values into an update statement. The names of each key correspond with each column in my table and so it should work out.

This is what I came up with:

$query = "UPDATE accounts SET ";

    foreach($_POST as $field => $value) {
        if ($field != null && $value != 'Update Information!'){
            $query .= "{$field} = {$value}";
            $query .= ", ";
    }
}
$query .= "WHERE id = {$current_user["id"]}";

The issue I am running into is the last line of the loop. The loop inserts a comma at the end of each loop which is fine until the last value where it messes up the UPDATE statement.

Is there anyway to exclude the comma on the last loop? Thanks!

zach
  • 27
  • 7
  • 1
    `$query = rtrim($query, ', ');` is the simplest solution – RiggsFolly Jun 05 '16 at 21:46
  • 2
    BUT Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared statement and parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Jun 05 '16 at 21:47
  • @RiggsFolly `substr()` is more effective than `rtrim()`, as it requires no comparisons to be done. – Johannes H. Jun 05 '16 at 21:51
  • 1
    @JohannesH. In PHP5.6 and PHP7 the difference is unmeasurable even with `microtime()` – RiggsFolly Jun 05 '16 at 22:06

2 Answers2

1

I prefer to use an array and implode for tasks like this.

$farray = array();
foreach($_POST as $field => $value) {
    $farray[] = "$field = '" . mysqli_real_escape_string($conn, $value) . "'";
}
$query .= implode(', ', $farray);
Barmar
  • 741,623
  • 53
  • 500
  • 612
0

Just delete the , it after the loop using $query = substr($query, 0, -2).


However your approach is dangerous in the first place. NEVER EVER use user input direclty in an SQL query. Escape it properly using mysql_real_escape_string() or use prepared statements (recommended). Imagine

$_POST['something'] = "foo; DROP DATABASE; UPDATE accounts SET id = 5";

Also note that the mysql_* interface is outdated - you should switch to mysqli_* or PDO.

Johannes H.
  • 5,875
  • 1
  • 20
  • 40
  • 1
    Even then your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared statement and parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Jun 05 '16 at 21:44
  • I am a beginner so I just started learning prepared statements but I will definitely use mysql_real_escape_string(). Thanks! – zach Jun 05 '16 at 22:08