WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 13 days ago

#32848 reopened defect (bug)

Null values for Update Options does not reset in $all_options

Reported by: MikeNGarrett Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.2.2
Component: Cache API Keywords: has-patch needs-unit-tests bulk-reopened
Focuses: performance Cc:

Description

This is a fun, extremely niche case.

We were using the Settings API to save out options for a plugin when we came across a really frustrating scenario. Half the time our setting was returning values that were not accurate to the setting that was stored in the database and in cache.

Come to find, the value being returned from wp_load_alloptions was incorrect while every other instance was correct. In our case, these values were in the database, memcache, and the single value in wp-object-cache.

On line 319 of option.php, we're using isset to check the value in the array. This returns false if the value is NULL. This leads to all kinds of sadness.

I would propose we instead use array_key_exists here to ensure we're getting true if the key is present in the array at all.

Attachments (2)

32848.diff (510 bytes) - added by MikeNGarrett 4 years ago.
Patch #1
32848.2.diff (5.1 KB) - added by MikeNGarrett 4 years ago.
Patch 2 - replace all isset in option.php

Download all attachments as: .zip

Change History (8)

@MikeNGarrett
4 years ago

Patch #1

This ticket was mentioned in Slack in #core by mikengarrett. View the logs.


4 years ago

#2 @MikeNGarrett
4 years ago

Co-contributor: @tndavide, @jipmoors

Last edited 4 years ago by MikeNGarrett (previous) (diff)

#3 @MikeNGarrett
4 years ago

  • Keywords has-patch needs-unit-tests added

I ran back through and realized there were quite a few places here that are using isset for assessing array keys existence. Here's a patch to switch all isset to array_key_exists. Next step, let's create some unit tests.

@MikeNGarrett
4 years ago

Patch 2 - replace all isset in option.php

#4 @iseulde
5 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This ticket has not seen any activity in over *two* years, so I'm closing it as "wontfix".

The ticket may lack decisiveness, may have become irrelevant, or may not have gathered enough interest.

If you think this ticket does deserve some attention again, feel free to reopen.

For bugs, it would be great if you could provide updated steps to reproduce against the latest version of WordPress (5.0.2 at the time of writing). Remember images or a video can be superior to explain a problem. At the very least, quickly test again to make sure the bug still exists.

If it’s an enhancement or feature, some extra motivation may help.

Thank you for your contributions to WordPress! <3

#5 @JeffPaul
3 months ago

  • Keywords bulk-reopened added
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

A decision was made to reopen tickets that were closed in the bulk edit that this ticket was affected by. This ticket is being placed back into the Awaiting Review milestone so it can be individually evaluated and verified to determine if it is still relevant/valid or reproducible.

#6 @MikeNGarrett
13 days ago

  • Keywords changed from has-patch, needs-unit-tests, bulk-reopened to has-patch needs-unit-tests bulk-reopened

This is still present, but it's a very narrow and uncommon case. I would be ok closing this.

https://core.trac.wordpress.org/browser/tags/5.1.1/src/wp-includes/option.php#L391

Note: See TracTickets for help on using tickets.