current_time function: GMT offset is not always an integer

[reputeinfosystems]

Hello,

We found a bug in the WordPress 6.2 RC with the function current_time located in wp-includes\functions.php. Before 6.2 RC, the line under this function was like as below.

<?php
    return $gmt ? time() : time() + (int)( get_option( 'gmt_offset' ) * HOUR_IN_SECONDS );
?>

While in WordPress 6.2 RC, it gets changes and integer typecasting moved inside the brackets.

<?php
    return $gmt ? time() : time() + ( (int) get_option( 'gmt_offset' ) * HOUR_IN_SECONDS );
?>

This creates time difference when the site timezone is like '+11:30', '+05:30', etc. When we call the function with timestamp parameter

<?php current_time('timestamp'); ?>

in WordPress 6.1.1 or lower, it returns me the gmt_offset 11.5 ( for +11:30 ), 5.5 ( for +05:30 ) and calculate properly without making any difference. 

While from WordPress 6.2, first it'll convert the gmt_offset to integer and that considers 30 minutes less than original. Before WordPress 6.2, the gmt_offset first converted into seconds and then it typecasted to integer but with 6.2 first it typecasting gmt_offset to integer and then converting to seconds so it makes the 30 minutes difference on above examples.

We would suggest that if it's needed to type cast only gmt_offset (inside the bracket as per WordPress 6.2) then either use float or double so that the above timezone makes no difference and calculate proper timestamp.

Suggested change:

<?php
    return $gmt ? time() : time() + ( (float) get_option( 'gmt_offset' ) * HOUR_IN_SECONDS );
?>

Thanks

[mukesh27]

Hi there! thanks for ticket.

The above change committed in https://core.trac.wordpress.org/changeset/55054.

Ping @SergeyBiryukov @audrasjb for thoughts.

[reputeinfosystems]

@SergeyBiryukov @audrasjb I think my solution will work for both situations. Pleas do the needful.

[Rarst]

Duplicate of #57035.

Cheers, this does look wrong. I'll reopen issue that did a faulty fix to keep discussion in one place.

[audrasjb]

As discussed with @hellofromTonya, reopening to handle the specific timezone issue, as #57035 was about a PHP8 compat problem :)

Moving for 6.2.1 consideration, as it appears it is currently not critical enough to delay WordPress 6.2.

[hellofromTonya]

The questions being considered are:

• How severe is this regression?
• What are its impact on users?

The answers to these questions will help to determine if this can be fixed in 6.2.1 or if it must be fixed now before the 6.2 final release (causing the release to be delayed until tomorrow).

Some context:

• Regression introduced by [55054] / #57035 to fix a fatal error in PHP 8+.
• ​10,973+ plugins use this function including:
  • The Events Calendar
  • MailPoet
  • Jetpack
  • WooCommerce
• A quick scan of their usages shows these impacts:
  • Incorrect queries in commerce stores
  • Incorrect human readable times
  • Incorrect time for task triggers (such as MailPoet for cron workers)
• Impacts ​partial-hour timezones such as +05:30 UTC.

Are there other impacts? Of those identified, how severe are they?

In discussing with @audrasjb and @SergeyBiryukov, we're currently thinking the severity is not severe enough to warrant delaying the 6.2 final release by a day, but rather to do a fast 6.2.1 release.

What do you think?

[Rarst]

Time slippage can cause broken date permalinks if the posts are near midnight and shift into a different day.

[hellofromTonya]

Replying to Rarst:

Time slippage can cause broken date permalinks if the posts are near midnight and shift into a different day.

Thanks @Rarst.

As a Component Maintainer, what are your thoughts of when to fix it?

[hellofromTonya]

A note from @SergeyBiryukov he made in comment:14:ticket:57035:

Interesting, I made a note recently in comment:4:ticket:57030 that gmt_offset can be a float, but somehow completely forgot about that here. Good catch!

[reputeinfosystems]

@hellofromTonya I think this is serious for plugins which are appointment or events plugins like our BookingPress. As soon as people update to 6.2, all appointments and events will be booked incorrectly and I think most of plugin will have to release immediate patch.

[Rarst]

Time slippage was probably the most reported impact when we tightened the Date/Time core code and third party code messing with time zones shifted things around for people.

I am not really familiar with core release process.

The volume of issues would be less if only partial-hour time zones are affected, but in my opinion the severity is a lot here to just send out in stable release.

[hellofromTonya]

Thank you @Rarst and @reputeinfosystems for sharing these additional impacts. I agree that these are more significant than original thought as they can impact bookings, date permalinks, and potentially stores. Though the regression affects ​partial-hour timezones, these timezones include India, Australia, Sri Lanka, and more.

​Currently discussing whether to revert for 6.2.0 or fix in 6.2.1 in Make/Core slack.

Also here's the regression in action along with a possible fix ​https://3v4l.org/tgP8m.

Fixes partial-hour timezone calculation in current_time() when 'timestamp' is passed. The gmt_offset can be a float/double for partial-hour timezones such as +5:30 UTC for India and Sri Lanka.

How?

• Changing the int type cast to float for option value.
• Then restoring the int type cast of that calculation, i.e. restoring it before changeset 55054.

Trac ticket: https://core.trac.wordpress.org/ticket/57998

[hellofromTonya]

After discussing in Make/Core with the Release Squad and Committers, there is consensus to revert [55054] now to ship in 6.2.0.

Why revert?

1. The additional impacts shared by @Rarst and @reputeinfosystems highlighted that this regression can impact bookings, date permalinks, and potentially stores for users in these ​partial-hour timezones.
2. The impacted user base includes India, Australia, and others, i.e. large population areas.
3. Even if a fast 6.2.1 were released, the impacts could already have been felt by business owners, customers, extenders, and users.
4. And then there are the unknowns or as @desrosj noted:

   I also think that impossible to anticipate the full impact of this change.
   

Why not fix in 6.2.0?
There's not enough time in the cycle to validate the fix truly fixes the issue without causing an additional issue or side effect. Thus, a fix is too risky this late in the cycle. More time is needed to discuss, add more test coverage, and consider other edge cases and scenarios.

The revert will impact the original ticket that introduced the changeset, i.e. #57035.

Once reverted, then the regression reported in this ticket is fixed as the code is restored to pre-6.2 state. Thus, this ticket can be closed work continuing in #57035 with this new knowledge.

Huge props to @reputeinfosystems for reporting this regression and @Rarst and @reputeinfosystems for sharing the additional impacts!

[hellofromTonya]

In 55598:

Date/Time: Revert [55054].

This changeset introduced a regression for partial-hour timezones such as +05:30 UTC which is India and Sri Lanka. How? These timezones are in float. The change made in [55054] type casted them to integer which dropped the decimal for the partial-hour, making the time inaccurate. For example, +05:30 UTC (India and Sri Lanka)'s 'gmt_offset' is 5.5, but with the changeset, it was changed to 5.

Reverting the changeset restores the original state of current_time() and thus resolves the regression.

Props reputeinfosystems, Rarst, hellofromTonya, desrosj, audrasjb, sergeybiryukov, costdev, priethor, francina, nekojonez, codingchicken, cbringmann.
See #57035.
Fixes #57998.

[hellofromTonya]

In 55599:

Date/Time: Revert [55054].

This changeset introduced a regression for partial-hour timezones such as +05:30 UTC which is India and Sri Lanka. How? These timezones are in float. The change made in [55054] type casted them to integer which dropped the decimal for the partial-hour, making the time inaccurate. For example, +05:30 UTC (India and Sri Lanka)'s 'gmt_offset' is 5.5, but with the changeset, it was changed to 5.

Reverting the changeset restores the original state of current_time() and thus resolves the regression.

Props reputeinfosystems, Rarst, hellofromTonya, desrosj, audrasjb, sergeybiryukov, costdev, priethor, francina, nekojonez, codingchicken, cbringmann.
Reviewed by desrosj.
Merges [55598] to the 6.2 branch.
See #57035.
Fixes #57998.

[spacedmonkey]

Wonder if could cast to float?

return $gmt ? time() : time() +  ( (float) get_option( 'gmt_offset' ) * HOUR_IN_SECONDS );

[hellofromTonya]

In 55600:

Help/About: Add Field Guide link.

In the About page, replaces the # placeholder with the link to the Field Guide.

Follow-up to [55573], [55449], [55499], [55545].

Props vladytimy, ocean90.
Fixes #57998.

[hellofromTonya]

Sorry the above changeset was for a different ticket.

@hellofromtonya I took the liberty of merging in trunk to resolve the merge conflict following the earlier revert.

@Rarst Are you able to link your GitHub and WordPress profiles so we can be sure you're given props correctly? ​https://make.wordpress.org/core/2020/03/19/associating-github-accounts-with-wordpress-org-profiles/

Closing this in favor of ​https://github.com/hellofromtonya/wordpress-develop/pull/5

Whoopsie, I misunderstood 🤦 @peterwilsoncc's PR are suggestions to apply to this PR.

Merged r58923 / ​https://github.com/WordPress/wordpress-develop/commit/6e7b1243f30af7b85d7dff44df4c86d852dec7df