Did destroy serialized data

Edit: Fixed in 1.3.4. It’s OK now, I guess.

Original review follows.

Please proceed with caution when using this plugin. This plugin doesn’t handle serialized data graciously and can, therefore, cause irreversible data loss.

Multiple users (including me) have reported this error to the plugin authors, but the development of this plugin seems to be abandoned. The latest commit on their GitHub was over 16 months ago from writing this review (version 1.3.3). The “Last updated” time on the plugin page here does not reflect code changes but is there because of “Tested up to” version bumps.

More developers are moving towards a “Key-value store” model for storing metadata; this helps ease the load on your database and keep everything nicely packed together. When WordPress encounters this type of data, it serializes it. BSR (Better Search Replace) tries to work with this, but does so erroneously.

The issue starts with method BSR_DB::recursive_unserialize_replace(), which recursively deserializes data. The cause is an assignment error. You can find this at /includes/class-bsr-db.php on line 351~356 of the plugin.

These errors are often overlooked and can easily be mitigated by using Yoda conditions, where the code would’ve caused a fatal error instead of data loss.

if ( $data = $this->unserialize( $data ) !== false ) {} // $data = ( false | true ).

Which should be:

if ( ( $data = $this->unserialize( $data ) ) !== false ) {}

Or, in Yoda:

if ( false !== ( $data = $this->unserialize( $data ) ) ) {}

Or, even better, decouple it from the assignment and don’t overwrite the data before testing its validity:

$unserialized_data = $this->unserialize( $data );
if ( false !== $unserialized_data ) {
	$data = $unserialized_data;
	// Code here...
}

But, even with that fixed, that part of the method doesn’t consider recursively unserializing the data, while that method implies it does. So… well, this is the real solution:

elseif ( is_serialized_string( $data ) ) {
	$unserialized_data = $this->unserialize( $data );
	if ( false !== $unserialized_data ) {
		$data = $unserialized_data;
		$data = $this->recursive_unserialize_replace( $from, $to, $data, false, $case_insensitive );
		$data = serialize( $data );
	}
}

I’d wholeheartedly recommend this plugin when this issue is fixed. Until then, please make a database backup before using it. Or, better yet, don’t use it.

• This topic was modified 5 years, 5 months ago by Sybre Waaijer.