wp-db.php incorrect usage of strip_invalid_text() in process_fields()

[fhwebcs]

Using $wpdb->update I'm stumbling upon errors while updating multiple fields.

Source: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php#L2083

In my opinion the result of strip_invalid_text() within process_fields() is used incorrectly:

    $converted_data = $this->strip_invalid_text( $data );

    if ( $data !== $converted_data ) {
        return false;
    }

Comparing $data (multiple nested array) with returned value (array|WP_Error) from strip_invalid_text() makes no sense. So, it should be replaced by:

    $data = $this->strip_invalid_text( $data );

    if ( is_wp_error( $data ) ) {
        return false;
    }

For another example see usage of $this->strip_invalid_text() in strip_invalid_text_from_query() (https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php#L2887)

[TobiasBg]

Thanks for the report! Indeed, this looks like arrays are being compared.

Introduced in [30345] for #21212.

[TobiasBg]

Comparing arrays like this is however not a problem (see ​the PHP docs).
From the function's documentation, it's the desired operation. If text has been stripped from the $data array, the function shall return false so that the query is rejected.

Checking for that WP_Error does not make sense here, as that would only trigger if the invalid text could not be stripped from the text for some reason.

Could you therefore please explain what errors you are seeing and what input/query is triggering them?

[fhwebcs]

You are right! I'v got lost because of missing error messages (see https://core.trac.wordpress.org/ticket/32167 for related debugging improvement). In my case, the string got truncated during strip_invalid_text() and -as documented- the result value becomes false.

So, my issue can be closed. Thank you.

[TobiasBg]

No problem! Good to hear that you found the cause now!

[TiagoGouvea]

Directly related to Ticket 34703 > https://core.trac.wordpress.org/ticket/34703

[TiagoGouvea]

Ok.. more detailed information about problem here:

I'm trying to update a record, setting a field to null (but it already null), and the $converted_data are stripping the 'NULL' to 2 characters because the field are a char(2)... and then, it result in false due the $data is different of the $converted_data.

Here the dumps:

// $data
array (size=1)
  'end_estado' => 
    array (size=4)
      'value' => string 'NULL' (length=4)
      'format' => string '%s' (length=2)
      'charset' => string 'latin1' (length=6)
      'length' => 
        array (size=2)
          'type' => string 'char' (length=4)
          'length' => int 2

// $converted_data
array (size=1)
  'end_estado' => 
    array (size=4)
      'value' => string 'NU' (length=2)
      'format' => string '%s' (length=2)
      'charset' => string 'latin1' (length=6)
      'length' => 
        array (size=2)
          'type' => string 'char' (length=4)
          'length' => int 2

What you think?
Thanks

[dd32]

Hi @TiagoGouvea,

Can you just confirm whether you're passing NULL or "NULL" to the function?

Given the data you've provided, it looks like you're passing it as a string (which isn't supported, and MYSQL would attempt to set it to a literal string "NULL").

[TiagoGouvea]

Cheking here, I'm passing 'NULL' on array.. but, I have a filter that fix it. Then, in the final query the NULL will not be a string.

add_filter('query', 'wp_db_null_value');
function wp_db_null_value($query)
{
    return str_ireplace("'NULL'", "NULL", $query);
}

Ok.. I'm realizing that I did so much to force the situation here. When I leave the null value, null, the final query turn it to when updating.

What can be a best approach to set a field to null in a generic wpdb->update() ?

Thanks

[dd32]

Answered in the original ticket: comment:3:ticket:34703