"Option value is not valid JSON in <prefix>_options" reported during/after update to 9.0.12

[waded]

A customer got an automatic WordPress exception report emailed to them implicating this plugin, and I thought I'd pass the exception along. When I investigated, pages were loading fine, no error was occurring as far as I could tell. I did not turn on logging. The automated report indicated URL was /wp-login.php?wpe-login=true.

WordPress version was 6.0.3, hosted on GoDaddy's Managed WordPress hosting.

The customer has version 9.0.12 of the plugin, the contents of the optionwpgmza_global_settings in the options table does appear to be valid JSON, and option wpgmza_db_version is now 9.0.12, seeming to imply the upgrade eventually did succeed, that it is no longer going through this code path?

Perhaps a race condition that can occur in upgrades or something like that? Is "+OK" an expected value to Upgrader->upgrade call?

Error Details
=============
An error of type E_ERROR was caused in line 29 of the file /var/www/wp-content/plugins/wp-google-maps/lib/codecabin/class.settings.php. Error message: Uncaught Exception: Option value is not valid JSON in kobkc_options - Syntax error in /var/www/wp-content/plugins/wp-google-maps/lib/codecabin/class.settings.php:29
Stack trace:
#0 /var/www/wp-content/plugins/wp-google-maps/includes/class.settings.php(20): codecabin\Settings->__construct('wpgmza_global_s...')
#1 /var/www/wp-content/plugins/wp-google-maps/includes/class.global-settings.php(27): WPGMZA\Settings->__construct('wpgmza_global_s...')
#2 /var/www/wp-content/plugins/wp-google-maps/includes/class.upgrader.php(10): WPGMZA\GlobalSettings->__construct()
#3 /var/www/wp-content/plugins/wp-google-maps/includes/class.database.php(22): WPGMZA\Upgrader->upgrade('+OK')
#4 /var/www/wp-content/plugins/wp-google-maps/includes/class.plugin.php(100): WPGMZA\Database->__construct()
#5 [internal function]: WPGMZA\Plugin->__construct()
#6 /var/www/wp-content/plugins/wp-google-maps/includes/class.factory.php(57): ReflectionClass->newInstanceArgs(Array)
#7 /var/www/wp-content/plugins/wp-google-maps/includes/class.plugin.p

[DylanCodeCabin]

Hi @waded - Thank you for taking the time to report this to us, we do appreciate it.

We've added this to our internal tasks and we will be looking into this in the coming week or two. I believe you are correct in saying this was an edge case, but ideally we should be able to get this resolved to avoid these kinds of reports from being sent out by mistake.

To confirm WPGMZA\Upgrader->upgrade('+OK') Is not an expected input, instead this "should" have been the version number before upgrade, this allows the code to auto-trigger some migration logic for older versions.

With all that said, we're on it and we'll get to the bottom of it as soon as possible.

[waded]

Thanks. I updated issue to include the WordPress version (6.0.3) and hosting detail (GoDaddy Managed WordPress) if that's helpful.

My best guess is something in their stack sneaks the "+OK" into the plugin's PHP file temporarily during automatic plugin update - it'd not have been them, they're quite hands-off the source code on this site. (Does that unexpected input cause the error seen? I don't know.)

If you need me to dig through wp-content to see what plugin/mu-plugin code might be responsible for the +OK behavior let me know. (Does seem risky for wp-google-maps to parse its own plugin header to get target version.)

[DylanCodeCabin]

Thanks for the additional insights, those are a great help!

I see what you are saying about reading the variable from the plugin file directly. We have been using this implementation for years, without any issues and it's quite similar to the way WordPress core checks plugin versions as well. We use to store this as a variable, but for various reasons, we decided a more dynamic approach would be better for update delivery.

With that said, we are very eager to improve any shortcomings in the flow and make it more stable overall. This is the first report we've had of the "+OK" (or any other non-version) value being passed to the method, which is why we did not anticipate it as a potential edge case, but we should have!

One reason why this is quite interesting is that the expected value is a version number stored within the database (Not pulled from the plugin header). Essentially the flow of an update looks like this:

1. Check version mismatch, if true, download and replace files 1.1 Do not update any version numbers in the database
2. On the first run (of the new version) compare the stored version (from the database) (This is the Upgrader class) 2.1 This is compared to the version in the file header 2.2 If migration is required perform it
3. Update the database version number

Based on your report, the stored database value would have been "+OK", or it was filtered, or mutated along the way. Which reaffirms your point that there may be some mutation of some part of the update flow on the server. From what we're seeing in the error logs, this isn't related to the files, but rather the database (or the connections to the database).

---

With all that said, as I've mentioned, we want to make sure it doesn't come up again on your side, or for any other customers. We're working on a solution which will assert the input parameter more strictly, ensuring that if/when anything else is passed to the system, no errors are caused by the method.

I should also mention that at this stage, much as you've mentioned, this is all a series of "best guesses" for the moment, but we're confident we have a way of preventing it from causing issues. We should also be able to simulate at least the main part of this edge case, to ensure the patch resolves the issue.

I do appreciate your offer to dig a bit further, but I'd hate to take any more of your time with this. I'm pretty sure my team and I have a handle on this, and we will get it patched and shipped with our next update.

Thank you again for your continued time, I do appreciate it tremendously! 😊

P.S. I do apologize for the lengthy reply, but I wanted to be as transparent and diligent as possible

[DylanCodeCabin]

We fixed this in the latest build - We handle those edge cases more gracefully