29

I'm trying to learn the proper way to use prepared statements to avoid SQL injections etc.

When I execute the script I get a message from my script saying 0 Rows Inserted, I expect this to say 1 Rows Inserted and of course update the table. I'm not entirely sure on my prepared statement, as I've done some research and I mean it varies from example to example.

When I'm updating my table do I need to declare all the fields or is it ok to just update one field??

Any information would be very helpful.

index.php

<div id="status"></div>

    <div id="maincontent">
    <?php //get data from database.
        require("classes/class.Scripts.inc");
        $insert = new Scripts();
        $insert->read();
        $insert->update();?>

       <form action="index2.php" enctype="multipart/form-data" method="post" name="update" id="update">
              <textarea name="content" id="content" class="detail" spellcheck="true" placeholder="Insert article here"></textarea>
        <input type="submit" id="update" name="update" value="update" />
    </div>

classes/class.Scripts.inc

public function update() {
    if (isset($_POST['update'])) {
        $stmt = $this->mysqli->prepare("UPDATE datadump SET content=? WHERE id=?");
        $id = 1;
        /* Bind our params */                           
        $stmt->bind_param('is', $id, $content);
        /* Set our params */
        $content = isset($_POST['content']) ? $this->mysqli->real_escape_string($_POST['content']) : '';

        /* Execute the prepared Statement */
        $stmt->execute();
        printf("%d Row inserted.\n", $stmt->affected_rows);

    }                   
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
user0129e021939232
  • 6,205
  • 24
  • 87
  • 140

3 Answers3

45
$stmt = $this->mysqli->prepare("UPDATE datadump SET content=? WHERE id=?");
/* BK: always check whether the prepare() succeeded */
if ($stmt === false) {
  trigger_error($this->mysqli->error, E_USER_ERROR);
  return;
}
$id = 1;
/* Bind our params */
/* BK: variables must be bound in the same order as the params in your SQL.
 * Some people prefer PDO because it supports named parameter. */
$stmt->bind_param('si', $content, $id);

/* Set our params */
/* BK: No need to use escaping when using parameters, in fact, you must not, 
 * because you'll get literal '\' characters in your content. */
$content = $_POST['content'] ?: '';

/* Execute the prepared Statement */
$status = $stmt->execute();
/* BK: always check whether the execute() succeeded */
if ($status === false) {
  trigger_error($stmt->error, E_USER_ERROR);
}
printf("%d Row inserted.\n", $stmt->affected_rows);

Re your questions:

I get a message from my script saying 0 Rows Inserted

This is because you reversed the order of parameters when you bound them. So you're searching the id column for the numeric value of your $content, which is probably interpreted as 0. So the UPDATE's WHERE clause matches zero rows.

do I need to declare all the fields or is it ok to just update one field??

It's okay to set just one column in an UPDATE statement. Other columns will not be changed.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • What is the first argument on line 10 for? – Anonymous Penguin Apr 25 '14 at 21:30
  • @AnnonomusPerson: one letter for each parameter. 's' for string, 'i' for integer, etc. Read http://php.net/manual/en/mysqli-stmt.bind-param.php – Bill Karwin Apr 25 '14 at 22:06
  • `@BillKarwin` For me `$status variable` is showing `integer` value . I used `echo $status` and the output is `1`. – Nimmagadda Gowtham Jul 24 '16 at 09:19
  • 1
    @NimmagaddaGowtham, when you echo a `true` value, PHP prints `1`. – Bill Karwin Jul 24 '16 at 16:24
  • how does `$stmt->bind_param('si', $content, $id)` work before you declare the value of `$content`? – stib May 21 '19 at 01:05
  • 1
    @stib, it binds to the variables by reference. It does not copy their value at the time you bind to them. Then you can modify the value, and `execute()` sends the value to MySQL at that time. It sends whatever the current value of the variable is at the time you `execute()`. This means you can bind variables to a prepared statement and then run a loop, modifying the values and using `execute()` many times inside the loop. No need to re-bind the variables during every loop iteration in that case. – Bill Karwin May 21 '19 at 01:07
13

In fact, prepared statements are not that complex as it's shown in the other answer. Quite contrary, a prepared statement is the most simple and tidy way to execute a query. Take, for example, your case. You need only three lines of code!

$stmt = $this->mysqli->prepare("UPDATE datadump SET content=? WHERE id=?");
$stmt->bind_param('si', $content, $id);
$stmt->execute();
  1. Prepare your query with placeholders
  2. Then bind variables (a hint: you can safely use "s" for any variable)
  3. And then execute the query.

As simple as 1-2-3!

Note that checking every function's result manually is just insane, it would only bloat your code without any benefit. Instead you should configure mysqli to report errors automatically once for all. To do so, add the following line before mysqli_connect()/new mysqli:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

the result will be pretty much the same as with trigger_error but without an single extra line of code! As you can see, the code could be very simple and concise, if used properly.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
-1

I want to clean up Bill Karwin's awesome code

$stmt = $this->mysqli->prepare("UPDATE datadump SET content=? WHERE id=?") or die ($this->mysqli->error);

$id = 1;

// Bind our params
// BK: variables must be bound in the same order as the params in your SQL.
// Some people prefer PDO because it supports named parameter.
$stmt->bind_param('si', $content, $id) or die ($stmt->error);

// Set our params
// BK: No need to use escaping when using parameters, in fact, you must not, 
// because you'll get literal '\' characters in your content. */
$content = (string)$_POST['content'] ?: '';

/* Execute the prepared Statement */
$status = $stmt->execute() or die ($stmt->error);


printf("%d Row inserted.\n", $stmt->affected_rows);

I recommend using "or die" instead of if clause I recommend forcing a variable type to take values:

// If id brings value: '12abc', PHP automatically stops it at 12
$id = (int)$_ POST ["id"];
  • 2
    [I don't recommend using "or die"](https://stackoverflow.com/questions/15318368/mysqli-or-die-does-it-have-to-die) – Your Common Sense May 20 '20 at 13:37
  • 1
    You haven't cleaned up the code, instead you made it worse and you introduced new vulnerabilities. See YCS's answer how to do it properly. – Dharman May 20 '20 at 14:54