2
//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.

2 Answers 2

2

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..

2
  • Thanks for your answer its really very helpful. Commented Apr 7, 2020 at 19:45
  • You're welcome! And I should add that the n$ position specifier (e.g. %2$s) is not supported by wpdb::prepare().
    – Sally CJ
    Commented Apr 8, 2020 at 0:35
1

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',
    ]
);
1
  • Thanks for your answer its really very helpful. Commented Apr 7, 2020 at 19:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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