[wp-trac] [WordPress Trac] #22192: update_option() strict checks can cause false negatives

[WordPress Trac]

#22192: update_option() strict checks can cause false negatives
------------------------------------------+-----------------------------
 Reporter:  nacin                         |       Owner:  joemcgill
     Type:  defect (bug)                  |      Status:  assigned
 Priority:  normal                        |   Milestone:  Future Release
Component:  Options, Meta APIs            |     Version:
 Severity:  normal                        |  Resolution:
 Keywords:  needs-patch needs-unit-tests  |     Focuses:  performance
------------------------------------------+-----------------------------
Changes (by joemcgill):

 * keywords:  has-patch has-unit-tests close => needs-patch needs-unit-tests
 * milestone:  6.4 => Future Release


Comment:

 After spending time testing [https://github.com/WordPress/wordpress-
 develop/pull/5498#issuecomment-1765268048 PR #5498], which reintroduced
 the `gmt_offset` issues that were originally reported by @Mamaduka
 [comment:52 here]—and after confirming with others who worked on this
 ticket, I decided to revert all of these changes.

 The behavior we initially confirmed in [56648] is that many loosely equal
 values were ''already'' not affected by this bug. However, some loosely
 equal falsey values were. I'm beginning to think that the previous
 behavior needs to be maintained.

 What is happening with the `gmt_offset` issue is that there are many
 places in Core that expects the value of get_option( 'gmt_offset' ) to be
 an (int) when calculating timestamps. When you set the timezone to UTC+0,
 the `timezone_string` option is deleted and `update_option( 'gmt_offset',
 0 )` is passed. Previously, this worked as expected, but if loosely equal
 values are not updated then this update would get skipped, causing the
 option to remain empty in the DB. Then, code like `current_time(
 'timestamp' )` will end up trying to multiply a string `''` with
 `HOUR_IN_SECONDS`, which throws a warning. In other places in Core, this
 causes a fatal
 ([https://github.com/WordPress/gutenberg/pull/54806#issuecomment-1734813934
 see example]).

 We could work around these edge cases by adding an additional filter that
 casts this specific option to an (int), but I suspect there are other
 places where a similar problem could cause errors if we're not allowing an
 empty value to be updated in the DB—particularly when there is a pre-
 filter in play that returns a loosely equivalent value, as is the case
 with `gmt_offset`.

 Given the timeline, I'm punting this to a future release. If this is
 attempted again, we would need to either check for updates against the raw
 DB value (not just what `get_option()` returns) or we should likely close
 this as `wontfix`.

 Regardless, adding additional unit tests confirming the current behavior
 should be committed, and could be backported to the 6.4 branch during the
 RC phase if ready.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/22192#comment:111>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform