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);

    }                   
}
6
  • Did you try it to see what happens? Commented Aug 19, 2013 at 14:38
  • 3
    real_escape_string() is not necessary for prepared statements
    – Danijel
    Commented Aug 19, 2013 at 14:41
  • 3
    In your SQL, the first parameter is content and the second is ID. When you call bind_param, you've reversed the order - try swapping the order around in bind_param
    – andrewsi
    Commented Aug 19, 2013 at 14:41
  • @PatrickKostjens I did try this and it doesn't effect the table at all Commented Aug 19, 2013 at 14:41
  • Verify that you not updating with same content, and from an existing id. Otherwise, effectively MySQL have right : nothing to update. Commented Aug 19, 2013 at 14:43

3 Answers 3

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.

6
  • What is the first argument on line 10 for? Commented Apr 25, 2014 at 21:30
  • @AnnonomusPerson: one letter for each parameter. 's' for string, 'i' for integer, etc. Read php.net/manual/en/mysqli-stmt.bind-param.php Commented Apr 25, 2014 at 22:06
  • @BillKarwin For me $status variable is showing integer value . I used echo $status and the output is 1. Commented Jul 24, 2016 at 9:19
  • 1
    @NimmagaddaGowtham, when you echo a true value, PHP prints 1. Commented Jul 24, 2016 at 16:24
  • 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. Commented May 21, 2019 at 1: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.

-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
  • 2
    I don't recommend using "or die" Commented May 20, 2020 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
    Commented May 20, 2020 at 14:54

Not the answer you're looking for? Browse other questions tagged or ask your own question.