Skip search and replace on objects that can't deserialize safely

[MarkBerube]

As discussed in the following issue thread: https://github.com/wp-cli/search-replace-command/issues/191

PHP 8.1 made it so class objects deserialized from strings must follow the types defined on the class if it has been loaded prior. Else you will face a PHP fatal TypeError:

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 approach is to catch those TypeErrors when they happen, warn the user about it but then continue the S&R process. The only con to this approach that I can see is that TypeError according to PHP's own docs doesn't exist in PHP 5.6 but running the following test in PHP 5.6:

<?php
$data = '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;}';
$error_reporting = error_reporting();
error_reporting( $error_reporting & ~E_NOTICE & ~E_WARNING );

try {
$unserialized = is_string( $data ) ? @unserialize( $data ) : false;
} catch(TypeError $e) {
    // we don't care about these - quit the fatal err & move on
}

error_reporting( $error_reporting );

shows no issues. Probably because PHP 5.6 isn't reaching the catch() block here.

[schlessera]

Confirmed that the unknown TypeError class is no issues under PHP 5.6: https://3v4l.org/bgh1S

[MarkBerube]

Behat scenario has been added! During behat testing it looks like I'm not entirely skipping that instance of search & replace if it ends up catching a TypeError. What ends up happening is it will treat $data as a string & only do str_replace() over it. Happy with that interaction, as it means those objects will still be replaced successfully.

[danielbachhuber]

During behat testing it looks like I'm not entirely skipping that instance of search & replace if it ends up catching a TypeError. What ends up happening is it will treat $data as a string & only do str_replace() over it. Happy with that interaction, as it means those objects will still be replaced successfully.

Hm, I'm not sure this is quite correct. I think we should skip search/replace entirely if we can't deserialize the object. If we search/replace against a serialized string, we can break the serialization.

[MarkBerube]

I've made it so that an exception is thrown if you hit a TypeError in serialization that should reach the empty catch() block at the end of the recursive run, so serialization is skipped on these unsafe objects. Just waiting for thoughts on this

[danielbachhuber]

Thanks again for your work on this, @MarkBerube !

This PR is directionally correct. @schlessera is going to do a deep dive in the coming days to validate the approach, evaluate the nature of the PHP changes, etc. Once he approves, this change will be a candidate for a point release.

[MarkBerube]

No problem @danielbachhuber! Some of the behat tests provide a different warning in the older PHP versions. I couldn't get behat to conditionally behave with those warnings. Should I keep trying to fix them? cc @schlessera

[schlessera]

Should I keep trying to fix them?

@MarkBerube No, don't spend more time on it until I've played around with this more. I'll ping here once I have an update.

[schlessera]

Thanks for all your work on this, @MarkBerube !

[MarkBerube]

Of course @schlessera , happy to help out!