wp-cli / search-replace-command

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

WP CLI running PHP 8.1 can't unserialize empty PHP 8.0 objects #191

Closed MarkBerube closed 5 months ago

MarkBerube commented 8 months ago

Bug Report

Describe the current, buggy behavior

On PHP 8.1 there was a significant change on how data is deserialized into objects and it can cause fatal errors on search & replace. In PHP 8.0 and older if you deserialized an empty mysqli_result object, no big deal. However in PHP 8.1 the typehinting is far more strict & if you provide a null value on a object that is typed like int you'll find this error on deserialization:

PHP Fatal error:  Uncaught TypeError: Cannot assign null to property mysqli_result::$current_field of type int in phar:///usr/local/bin/wp/vendor/wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php:90

This has potential to block anybody from upgrading to PHP 8.1+ with wp-cli since it is really quite common to find mysqli_result objects in WP DB due to plugins.

Describe how other contributors can replicate this bug

To replicate this bug, bring down the following docker environment: https://gist.github.com/MarkBerube/3c79f30551cb650d70b0822afed04cb0

run the following

docker-compose up -d

docker-compose run --rm wpcli wp core install --url=dontmatter.co --title=nop --admin_user=blah --admin_email=blah@blah.com

docker compose exec -T  db mysql -uwordpress -pwordpress <<< "insert into wordpress.wp_options (option_name,option_value) VALUES ('Foobar','O:13:\"mysqli_result\":5:{s:13:\"current_field\";N;s:11:\"field_count\";N;s:7:\"lengths\";N;s:8:\"num_rows\";N;s:4:\"type\";N;}');"

docker-compose run --rm wpcli wp search-replace 'mysqli_result' 'mysqlir_result' --all-tables

should produce the following stdout:

PHP Fatal error:  Uncaught TypeError: Cannot assign null to property mysqli_result::$current_field of type int in phar:///usr/local/bin/wp/vendor/wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php:90

Describe what you would expect as the correct outcome

No PHP fatal errors on search & replace.

Let us know what environment you are running this on

docker-compose run --rm wpcli wp cli info
                                                                                                                                                               0.0s
OS: Linux 6.3.13-linuxkit #1 SMP PREEMPT_DYNAMIC Thu Sep  7 07:54:49 UTC 2023 x86_64
Shell:  
PHP binary: /usr/local/bin/php
PHP version:    8.1.25
php.ini used:   
MySQL binary:   /usr/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.5-MariaDB, for Linux (x86_64) using readline 5.1
SQL modes:  
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:   /var/www/html
WP-CLI packages dir:    
WP-CLI cache dir:   /home/www-data/.wp-cli/cache
WP-CLI global config:   
WP-CLI project config:  
WP-CLI version: 2.9.0

Provide a possible solution The problem is serializing between older PHP versions, specially with objects like mysqli_result that are strongly typed now. Don't expect there to be an easy way to clean all objects that folks leave in their DB but maybe the simple ones could be targeted via regex

Provide additional context/Screenshots original report that was found: https://github.com/php/php-src/issues/10893 potential fix for future: https://github.com/php/php-src/pull/6587

danielbachhuber commented 8 months ago

Thanks for the great bug report, @MarkBerube !

Provide a possible solution

Another option to consider: We could potentially skip mysqli_result objects for PHP 8.1 and throw a warning.

I'm not sure of the use case to search/replace inside of a mysqli_result object. It seems like it'd be much more preferable to throw a warning and continue command execution.

sun commented 7 months ago

💡 If you run into this issue and need a quick workaround, try this:

$ wp db query "SELECT option_name FROM wp_options WHERE option_value LIKE '%mysqli_result%'"
_wpallimport_session_98_

$ wp option delete _wpallimport_session_98_
Success: Deleted '_wpallimport_session_98_' option.

$ wp search-replace old new --all-tables --report-changed-only --skip-columns=guid,user_email,comment_author_email,display_meta,domain,referrer
Success: Made 10797 replacements.

I was wondering why we had mysqli_result objects in the database in the first place. An old version of the WP All Import plugin stored them as part of its logs in the database. So finding and deleting them not only resolved the issue but additionally cleaned up the database. 👍

Of course, we should still avoid the error with the proposed PR. 🙂

MarkBerube commented 7 months ago

you could also be super specific about the empty mysqli_result objects if you want to be safe since PHP serializes objects the same way every time:

where option_value REGEXP 'O:13:"mysqli_result":5:{s:13:"current_field";N;s:11:"field_count";N;s:7:"lengths";N;s:8:"num_rows";N;s:4:"type";N;}';
danielbachhuber commented 5 months ago

🔨

vimes1984 commented 4 months ago

Saved my day :)