Notice: wpdb::prepare was called incorrectly. The query argument of wpdb::prepare() must have a placeholder

Question

//Get Services and save to database first.
for($i = 0; $i < count($_POST["wc_service_id"]); $i++) {
    $wc_service_id          = $_POST["wc_service_id"][$i];
    $wc_service_name        = $_POST["wc_service_name"][$i];

    $wc_service_code        = $_POST["wc_service_code"][$i];
    $wc_service_qty         = $_POST["wc_service_qty"][$i];
    $wc_service_price       = $_POST["wc_service_price"][$i];

    $insert_query =  "INSERT INTO `".$computer_repair_items."` VALUES(NULL, '".$wc_service_name."', 'services', '".$post_id."')";
     $wpdb->query(
            $wpdb->prepare($insert_query)
    );
    $order_item_id = $wpdb->insert_id;


    $insert_query =  "INSERT INTO `".$computer_repair_items_meta."` 
                        VALUES(NULL, '".$order_item_id."', 'wc_service_code', '".$wc_service_code."'), 
                        (NULL, '".$order_item_id."', 'wc_service_id', '".$wc_service_id."'),
                        (NULL, '".$order_item_id."', 'wc_service_qty', '".$wc_service_qty."'), 
                        (NULL, '".$order_item_id."', 'wc_service_price', '".$wc_service_price."')";
    $wpdb->query(
            $wpdb->prepare($insert_query)
    );
}//Services Processed nicely

With code above its working completely fine only problem i am getting this Notice: when debug mode is on. And i shouldn't have any notices.

Notice: wpdb::prepare was called incorrectly. The query argument of wpdb::prepare() must have a placeholder.

Is there any suggestion? I know removing prepare( would take away the problem. But i want to keep the query prepared if you think its not necessary please explain.

Also adding %s, %d for first query, is good. But what for 2nd Query where i have 3 values?

Thanks in advance for reply.

Answer 1

You don't need to remove the prepare(), but you just need to do it properly.

And please check the $wpdb->prepare() reference for the function's syntax etc., but basically, instead of (wrapped for brevity):

$insert_query = "INSERT INTO `".$computer_repair_items."`
    VALUES(NULL, '".$wc_service_name."', 'services', '".$post_id."')";
$wpdb->query(
    $wpdb->prepare($insert_query)
);

you should do it like this: (presuming $computer_repair_items is a valid table name)

$insert_query = $wpdb->prepare(
    "INSERT INTO `$computer_repair_items` VALUES(NULL, %s, 'services', %d)",
    $wc_service_name, // 2nd parameter; replaces the %s (first placeholder)
    $post_id          // 3rd parameter; replaces the %d (second placeholder)
);
$wpdb->query( $insert_query );

I.e. You must pass at least two parameters to $wpdb->prepare():

1. The first parameter is the SQL command and should be in the same format as accepted by the PHP's sprintf() function, i.e. using placeholders such as %s for strings and %d for integers.

2. The second parameter should be the replacement value for the first placeholder in the SQL command mentioned above, i.e. the first parameter. So in the example I gave, the second parameter is $wc_service_name which the value replaces the %s in the SQL command. But note that you should not wrap the placeholder in quotes, so '%s' and "%s" for examples, are incorrect and just use %s.

3. Depending on your SQL command, you can also have the third, fourth, fifth, etc. parameters, just as in the example I gave where I have a third parameter which is $post_id for the %d placeholder.

And actually, for single INSERT operations, you could simply use $wpdb->insert():

// Make sure you replace the column NAMES.
$wpdb->insert( $computer_repair_items, [
    'column'  => $wc_service_name,
    'column2' => 'services',
    'column3' => $post_id,
] );

For multiple inserts, you can try something like:

$insert_values = [];
foreach ( [
    [ 'wc_service_code', $wc_service_code ],
    [ 'wc_service_id', $wc_service_id ],
    [ 'wc_service_qty', $wc_service_qty ],
    [ 'wc_service_price', $wc_service_price ],
] as $row ) {
    $insert_values[] = $wpdb->prepare( "(NULL, %d, %s, %s)",
        $order_item_id, $row[0], $row[1] );
}

$insert_query = "INSERT INTO `$computer_repair_items_meta`
    VALUES " . implode( ',', $insert_values );
$wpdb->query( $insert_query );

Or just call $wpdb->insert() multiple times..

Answer 2

Your code is extremely unsafe.

You're not using $wpdb->prepare() properly. You can't just use it on a string as a magic fix. There is nothing in this line of code that protects you from SQL injection attacks, because you're taking submitted values from $_POST and inserting them directly into an SQL query:

"INSERT INTO `".$computer_repair_items."` VALUES(NULL, '".$wc_service_name."', 'services', '".$post_id."')";

The whole point of $wpdb->prepare(), as documented, is to safely insert variables into an SQL query. To do this you need to provide it 2 things:

1. An SQL string with placeholders where the variables need to go.
2. The variables that will go into those placeholders, separately.

So instead of:

$insert_query = "INSERT INTO `".$computer_repair_items."` VALUES(NULL, '".$wc_service_name."', 'services', '".$post_id."')";
$wpdb->query(
    $wpdb->prepare($insert_query)
);

You need to do this:

$insert_query = "INSERT INTO `{$computer_repair_items}` VALUES( NULL, %s, 'services', %s )";

$wpdb->query(
    $wpdb->prepare( $insert_query, $wc_service_name, $post_id );
);

Note that:

• $wpdb->prepare() can't insert the table name from a variable, but the table name should not be coming from an unsafe source. I'm assuming that you've defined in manually in your code earlier.
• The first %s will be replaced with a safely escaped version of $wc_service_name, with quotes added automatically.
• The second %s will be replaced with a safely escaped version of $post_id, with quotes added automatically, just because this is what your original code did. If that column is actually an integer column, then use %d instead of %s.

All that being said, $wpdb actually has a method for INSERT queries that automatically prepares the query for you. For our example you would use it like this:

$wpdb->insert(
    $computer_repair_items,
    [
        $wc_service_name,
        $post_id,
    ],
    [
        '%s',
        '%s',
    ]
);