![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 2 years ago
Last modified 16 hours ago
#61762 new defect (bug)
Options can be incorrectly cached following database errors.
| Reported by: | peterwilsoncc | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Options, Meta APIs | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
Various wpdb methods can return falsey values to indicate one of two things:
0: zero effected rowsfalse: a database error
In get_option(), get_network_option(), delete_option() and delete_network_option() the results of the database query are imprecisely checked and can result in populated the notoptions cache incorrectly. Either by adding to the cache following a database error, or not adding to the cache when the zero rows are effected while deleting an option.
In add_network_option(), update_network_option() the notoptions cache is cleared prior to the database query without accounting for database errors. (This simply causes additional queries rather than incorrect data being returned.)
Related #61484, props @bjorsch for picking up.
Change History (2)
This ticket was mentioned in PR #12595 on WordPress/wordpress-develop by @gunjanjaswal.
16 hours ago
#1
- Keywords has-patch has-unit-tests added
#2
@
16 hours ago
Opened a PR for this: https://github.com/WordPress/wordpress-develop/pull/12595
It gates each notoptions write on the query being known to have succeeded: $wpdb->last_error on the read paths in get_option() and get_network_option(), and false !== $result on the delete paths. A zero-row delete still primes the cache, a failed query no longer does.
delete_network_option() was checking a truthy result, so a successful delete affecting zero rows never primed at all. That's fixed too.
I left the notoptions clearing in add_network_option() and update_network_option() alone for now, since the description notes it only costs extra queries rather than returning incorrect data. Happy to fold it into the same patch if you would rather have it all together.
Unit tests cover the read and delete paths, including the zero-row case.
$wpdb->get_row()returns null both when an option row is missing and when the query itself failed.get_option()andget_network_option()treated the second case as the first and primed thenotoptionscache, so a transient database error could hide an option that is still in the database. With a persistent object cache that state sticks around, and the option keeps returning its default long after the database has recovered.delete_option()had the same problem from the other direction: it primednotoptionsbefore checking the result of$wpdb->delete(), so a failed delete marked a still-present option as nonexistent.Each
notoptionswrite is now gated on the query being known to have succeeded, using$wpdb->last_erroron the read paths andfalse !== $resulton the delete paths.delete_network_option()was guarded on a truthy result, which meant a successful delete affecting zero rows never primed the cache. Zero rows is a successful query, so it primes there now too.Adds regression tests covering the read and delete paths, including the zero-row case.
Clearing
notoptionsinadd_network_option()andupdate_network_option()is left for a follow-up, per the ticket.