wp-cli / search-replace-command

Searches/replaces strings in the database.
MIT License
57 stars 45 forks source link

Warning: unserialize() expects parameter 1 to be string, array given #122

Closed nickdaugherty closed 2 years ago

nickdaugherty commented 5 years ago

Bug Report

Describe the current, buggy behavior

[Suppressed] Warning: unserialize() expects parameter 1 to be string, array given in wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php on line 85

Additionally, this results in warnings like this:

[Suppressed] Notice: unserialize(): Error at offset 0 of 24 bytes in wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php on line 85

Describe how other contributors can replicate this bug

I'm not 100% sure the conditions that trigger this, we've just noticed it while running search/replaces like this:

wp --path=/var/www/ search-replace http://foo.com https://foo.bar --all-tables --dry-run

Describe what you would expect as the correct outcome

No suppressed warnings because only strings get passed into unserialize().

Let us know what environment you are running this on

wp cli info
OS:    Linux 4.9.0-9-amd64 #1 SMP Debian 4.9.168-1 (2019-04-12) x86_64
Shell:    /bin/bash
PHP binary:    /<redacted>/php
PHP version:    7.3.5
php.ini used:    /<redacted>/php.ini
WP-CLI root dir:    phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:    phar://wp-cli.phar/vendor
WP_CLI phar path:    /home/<redacted>
WP-CLI packages dir:    
WP-CLI global config:    
WP-CLI project config:    
WP-CLI version:    2.1.0

Provide a possible solution

This bug was introduced here:

https://github.com/wp-cli/search-replace-command/commit/babbfbfcc1d36aff2f8b54f7b491bded98bde5d3

It would seem that while trying to address PHPCS styles, the logic was changed.

Was:

if ( is_string( $data ) && false !== ( $unserialized = @unserialize( $data ) ) ) {

Is now:

$unserialized = @unserialize( $data );
if ( is_string( $data ) && false !== $unserialized ) {

The is_string() check needs to happen before unserialize() is called, not after.

squelchdesign commented 3 years ago

I'm getting the same. Started when I upgraded to PHP8.

rwagner00 commented 2 years ago

Getting the same error on PHP8 as well, which I resolved by replacing the is_string($data) check with the significantly more robust WP Core function is_serialized($data). Does anyone know of a reason this wouldn't be a good change?

schlessera commented 2 years ago

The reported bug was fixed via https://github.com/wp-cli/search-replace-command/commit/d7a6a73fcddd80ca8953fb32269fb36aae934851