get_date_from_gmt assumes current gmt_offset is appropriate

[scottconnerly]

For locations that have daylight savings time, the gmt_offset changes over time. Yet get_date_from_gmt assumes the offset that is in effect now is always the right offset. But I might be trying to display date/times that are in a different timezone transition than the current one.

Proposed fix is to use timezone_transitions_get() as wp-admin/options-general.php does in order to translate those date/times accurately.

This already happens deeper in the code.

If the option timezone_string is set, then we use it to set the GMT offset, which means it is DST-aware.

Take a look at wp_timezone_override_offset() which overrides get_option('gmt_offset') by hooking into pre_option_gmt_offset.

Related: #20398

I'm not sure the the bug is addressed with this hook. This still uses an offset which is computed based on the current date, and that offset is not valid for all dates in that timezone.

Example: Timezone is Europe/London. Date is 2012-04-09, so we're in DST. gmt_offset should therefore be 1. Now I want to find the local time of the UTC 2012-01-01 12:00:00 using my timezone. The low-level code and the hook both add 1 hour to that time, which is incorrect.

#20398 has a patch: ticket:20398:20398.diff​.

Attached a patch which unifies the behaviour of get_gmt_from_date and get_date_from_gmt.

fwiw I can confirm that the patch for get_date_from_gmt works - as I am having this same issue with a plugin and will be implementing this manually for now.

Still not sure there is a bug here. Needs a test case.

Updated diff based on upstream changes

New test case for respecting DST

Test case added.

The original phpdoc for the function hints at the bug: "Simply adds the value of gmt_offset." This is not sufficient to be DST aware. The point of DST is that gmt_offset changes throughout the year, so simply adding the offset to any date will be incorrect for ~50% of dates.

The dual function get_gmt_from_date uses the timezone_string option and the DateTime class to properly respect the timezone, as alluded to in #20398. This is why the second test test_dst_respected_getting_gmt_date passes. The logic for in this function is rather nasty and roundabout, however, so the attached patch simplifies it.

The patch to get_date_from_gmt adds the same functionality, so that the timezone_string is used to convert the GMT date into a date local to that timezone, which respects DST where relevant.

Note that both functions fall back to the naïve approach of adding or subtracting gmt_offset where the timezone_string option is not available.

In 1233/tests:

Add tests for get_date_from_gmt() and get_gmt_from_date().

These tests demonstrate that get_date_from_gmt() is ignorant of DST settings.

props scholesmafia.
see #20328.

Thank you! I have committed those tests.

So, clearly any usage of get_option( 'gmt_offset' ) is improper except when dealing with the current time. I can't say we currently use it often in core, however, so that's good.

In 23618:

Properly handle timezones in get_date_from_gmt() rather than relying on the implicit gmt_offset. This offset is only good for the current time, rather than the passed time, which causes problems when converting a DST date when DST is not in effect, or vice versa.

Update get_gmt_from_date() to make these functions match in formatting, as they are complementary and just reverse a few operations.

props scholesmafia
Tests: [1233/tests]

fixes #20328.

iso8601_to_datetime() and wp-mail.php both appear to be affected. We are already parsing out the date/time pieces using regex in both of those cases, so we should be able to just re-construct a date and pass it directly to get_date_from_gmt/get_gmt_from_date.

_walk_bookmarks() is also affected, but that is not a big deal.

Moving back to 3.6 as this had a commit that nearly fixed it entirely.