Drupal 8 is not compatible with PHP 5.5.9

@Mile23 what is the exact fail? Do we do the timezone setting from another date in test code or in real runtime code?

You can see warnings in

https://travis-ci.org/paul-m/drupal_dependency_bounds_test/jobs/29397795...

Hard to tell exactly what is happening without some additional context/debug, but I think this is happening directly in DateTimePlus::createFromFormat():

    $datetimeplus = new static('', $timezone, $settings);

    $date = \DateTime::createFromFormat($format, $time, $datetimeplus->getTimezone()); // <--- maybe here, too
    if (!$date instanceof \DateTime) {
      throw new \InvalidArgumentException('The date cannot be created from a format.');
    }
    else {
      // Functions that parse date is forgiving, it might create a date that
      // is not exactly a match for the provided value, so test for that by
      // re-creating the date/time formatted string and comparing it to the input. For
      // instance, an input value of '11' using a format of Y (4 digits) gets
      // created as '0011' instead of '2011'.
      if ($date instanceof DateTimePlus) {
        $test_time = $date->format($format, $settings);
      }
      elseif ($date instanceof \DateTime) {
        $test_time = $date->format($format);
      }
      $datetimeplus->setTimestamp($date->getTimestamp());
      $datetimeplus->setTimezone($date->getTimezone()); // <--- think warning is happening from this

      if ($settings['validate_format'] && $test_time != $time) {
        throw new \UnexpectedValueException('The created date does not match the input value.');
      }

This section of code is fugly and should be refactored, but I am not really sure we can eliminate the possibility of the numeric offsets being passed in instead of the proper IANA names. I think fields via the widgets are OK, but migrate would be a problem and other non-widget forms.

However, since 'validate_format' defaults to TRUE, if this were widespread we should be seeing bug reports.

My reading of https://bugs.php.net/bug.php?id=45543 and the pull request with the fix is that we may want to consider changing the min to 5.5.10.

@mpdonadio nice sleuthing. I think you're probably right - that does look like the probable cause. So yeah I think we have to bump the minimum version to 5.5.10. IMO there is a case to say that we only actually support the latest patch release on each PHP minor version - i.e what we test on. That said bumping to 5.5.10 as a bug fix makes sense because I agree with

but I am not really sure we can eliminate the possibility of the numeric offsets being passed in instead of the proper IANA names

This is what the patch would look like. I hand edited the lockfile since I wasn't sure it there was a better way to regenerate it and keep the dependency choices the same. ComposerIntegrationTest passes locally, which means the hash isn't dependent on the php requirements (which surprises me, and I double checked that part with a test).

I think the other references to 5.5.9 need to stay. In addition to updating https://www.drupal.org/docs/8/system-requirements and some child pages, would probably want a CR, too?

Also assuming product and framework manager signoff is needed?

And a thought, the attached updates everywhere. We are kinda lying, though. Seems like core/lib/Drupal/Component/Datetime/composer.json , core/composer.json, composer.lock may be the only places to touch?

Out of due diligence, this is what a potential mitigation would look like, but don't have 5.5.9 locally to test. Not 100% sure this would fully prevent the problem, especially because of the DateTime::setTimezone() call in DateTimePlus::format().

Even if we did this, I would worry that we may be masking a problem. I think there are just too many ways for a TZ offset to get used instead of a TZ name.

I may want to do this, though, in another issue as it does simplify that method.

So yeah I think we have to bump the minimum version to 5.5.10. IMO there is a case to say that we only actually support the latest patch release on each PHP minor version - i.e what we test on.

A counterpoint to this is that AFAIK, neither 5.5.10 nor any higher 5.5 version is secure. The only secure build of PHP in the 5.5 range is the 5.5.9 one that Ubuntu backports security fixes to.

Can we add an inline comment indicating what we don't do because it's incompatible with 5.5.9, so it doesn't get helpfully refactored away in the future? Could even add an @todo to a postponed followup to use the HEAD code once we require 5.6+. (I checked to confirm that PHP 5.5.10 was released before 5.6.0.)

Promoting to major since this is an incompatibility with a PHP version we currently support.

Which approach do we want to go with, #5 or #7?

I am not 100% convinced that #7 is a full fix. Tests may pass, but there could still be hidden bugs or cases where the problem may creep up from calls to \DateTime::setTimezone(), especially contrib. If we go this route, I think we should also add a hook_requirements warning (probably add a bit to system_requirements() where we check the version in system.module:180).

Do we know if PHP's fix of this in https://bugs.php.net/bug.php?id=45543 was considered a security fix? I only ask because the link at the bottom says Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=22dba2f5f3211efe6c3b9bb24734c811ca64c68c, but I don't know if that means it's considered a security fix or not.

If it was a security fix, do we know if Ubuntu backported it to its PHP 5.5.9 build on Ubuntu 14.04?

If the bug fix was not backported to Ubuntu's 5.5.9 build, then I think we need to do something more along the lines of #7 than of #5.

I am not 100% convinced that #7 is a full fix.

Right, at a minimum, I think we should make sure that core and contrib callers of setTimezone() on a DrupalDateTime object work correctly. Is this possible to accomplish by implementing a setTimezone() method on either DrupalDateTime or DateTimePlus?

there could still be hidden bugs or cases where the problem may creep up from calls to \DateTime::setTimezone(), especially contrib

We should make sure there aren't any cases of core doing this. E.g., by having core go through the DrupalDateTime or DateTimePlus objects. However, if a contrib module is invoking \DateTime::setTimezone() directly in a way that's buggy in PHP 5.5.9, then I think it's that module's responsibility to fix that: either by changing the arguments it uses to call it, or using DrupalDateTime/DateTimePlus, or bumping up the PHP requirement in its own .info.yml file.

DrupalDateTime::setTimezone() and DateTimePlus::setTimezone() both exist, because they have a magic __call() method to a protected \DateTime object, so all three have this problem now (it's more obvious when you read the 8.5.x version of DateTimePlus with the @method annotation, see #2902707: Document magic methods in DateTimePlus and DrupalDateTime using phpDoc @method.

So, we would have to implement our own DateTimePlus::setTimezone(), and convert the numeric ones like '+02:00' into named ones and the update the proxied \DateTime object. The only names we can use are the backzone (ie, deprecated) ones like 'Etc/GMT+2' but even then a few locations use offset at the 30min mark which don't have a corresponding name. It would be messy.

The only names we can use are the backzone (ie, deprecated) ones like 'Etc/GMT+2'

If we only do this for when the PHP version is <5.5.10, then what's the downside to using the deprecated timezone identifiers? If there is a downside, would another option be to hard-code a map from the 24 or so (maybe more if we want to support the half-hour ones) numeric ones to a representative continent/city?

• Drupal 8 support for PHP 5.5 and 5.6 will end on March 6, 2019
• PHP 5.5 only supported for existing Drupal 8 sites, new Drupal 8 sites require PHP 7.0.8