Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1: strftime() is deprecated #15864

Open
JoshuaLuckers opened this issue Oct 24, 2021 · 6 comments
Open

PHP 8.1: strftime() is deprecated #15864

JoshuaLuckers opened this issue Oct 24, 2021 · 6 comments
Labels
bug The issue in the code or project, which should be addressed.

Comments

@JoshuaLuckers
Copy link
Contributor

The strftime() function is called by the core in various places.

In PHP 8.1 this will trigger a deprecation warning because the strftime() and gmstrftime() functions have been deprecated in favor of date()/DateTime::format() (for locale-independent formatting) or IntlDateFormatter::format() (for locale-dependent formatting).

The strftime() and gmstrftime() functions exhibit similar issues as strptime(), in that the formats they support, as well as their behavior, is platform-dependent. Unlike strptime(), these functions are available on Windows, though with a different feature set than on Linux. Musl-based distributions like Alpine do not support timezone-related format specifiers correctly. These functions are also locale-based, and as such may exhibit thread-safety issues.

Once again date() or DateTime::format() provide portable alternatives, and IntlDateFormatter::format() provides a more sophisticated, localization-aware alternative.

References:

Environment

  • MODX 2.x, 3.x
  • PHP 8.1
@JoshuaLuckers JoshuaLuckers added the bug The issue in the code or project, which should be addressed. label Oct 24, 2021
@muzzwood
Copy link
Contributor

muzzwood commented Nov 4, 2021

I've been experimenting with this.
It looks like the PHP extension intl would be a requirement if we're going to use IntlDateFormatter.
Maybe we need a helper method/class that determines if it's installed and uses DateTime::format which isn't locale aware otherwise.

Usage of IntlDateFormatter would be something along the lines of:

$locale = $this->modx->getOption('locale') ?? '';
$timezone = $this->modx->getOption('timezone') ?? '';

$formatter = new IntlDateFormatter($locale, IntlDateFormatter::SHORT, IntlDateFormatter::SHORT, $timezone);
return $formatter->format(new DateTime());

@rthrash
Copy link
Member

rthrash commented Jan 10, 2022

This issue has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/whats-happening-with-the-strftime-and-date-output-modifiers/4826/3

@Mark-H
Copy link
Collaborator

Mark-H commented Feb 3, 2022

Two weeks ago I released a SiteDash update that tracks available PHP extensions to see if requiring intl would be feasible. On a sample size so far of about 18% of sites connected to SiteDash, roughly 81% of those have the intl extension enabled. That's more than imagick (75%) but less than fileinfo (91%).

At those numbers I'd personally say we can conditionally use it in core; fallback to non-localised date() with a warning-level error suggesting people to install intl for locale support.

@JoshuaLuckers
Copy link
Contributor Author

Sounds good to me!

@JoshuaLuckers
Copy link
Contributor Author

@Mark-H I'm proposing to make a start with this in 3.1. What do you think?

@opengeek
Copy link
Member

@Mark-H I'm proposing to make a start with this in 3.1. What do you think?

Just keep in mind this is only a deprecation warning and will continue to work until PHP 9 is released. We also need to consider places where this will cause BC breaks and keep compatibility for those on PHP <= 8.x for the MODX 3.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed.
Projects
None yet
Development

No branches or pull requests

5 participants