Problem/Motivation

The maximum allowed timestamp is limited on 32-bit systems (includes the 32-bit Acquia Dev Desktop v2 app, running on 64-bit machines) - e.g. it can't handle dates beyond 19 January 2038 (the 2038 bug) or of December 13 1901 and earlier.

There are multiple uses of getTimestamp() and strtotime() in core alone, directly and via Drupal's DateTimePlus class which PHP DateTime.

These return FALSE for dates out of range on systems affected by the 2038 bug (see #9 for a simple test), which means the date is invariably incorrectly shown as 1970-01-01.

e.g. strtotime():

strtotime('Jan 19 2038');
2147472000
strtotime('Jan 20 2038');
false

Field types/widgets affected: (at least) the Date only and Date and Time, including "Select list" widget. You can choose years outside the 1900 to 2038 range, but because PHP can't handle them they will be stored/displayed as 1970-01-01 without warning.

Proposed resolution

Warn 32-bit users, on installation and on /admin/reports/status, plus additional test coverage #13

Remaining tasks

  • Agree what to do.
  • Write patch that display warnings
  • Figure out if/how we can run a 32-bit test?
  • Change record

User interface changes

Warnings on install, on system status (translatable text string)

Change record

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#110 interdiff-107-110.txt1.46 KBmpdonadio
#110 2795489-110.patch3.46 KBmpdonadio
#108 interdiff-80-107.txt1.6 KBmpdonadio
#108 2795489-107.patch3.4 KBmpdonadio
#81 Limited-date-range-warning.png56.55 KBgambry
#80 2038_bug_with_php-2795489-80.patch3.19 KBgambry
#80 interdiff.txt2.39 KBgambry
#74 2038_bug_with_php-2795489-74.patch3.21 KBgambry
#74 interdiff.txt1.14 KBgambry
#72 2038_bug_with_php-2795489-72.patch3.2 KBgambry
#72 interdiff.txt1.07 KBgambry
#69 2038_bug_with_php-2795489-69.patch3.13 KBgambry
#69 interdiff-48-69.txt2.38 KBgambry
#59 interdiff-2795489-48-59.txt3.81 KByogeshmpawar
#59 2038_bug_with_php-2795489-59.patch4.16 KByogeshmpawar
#48 2795489-47.patch3.05 KBMunavijayalakshmi
#42 interdiff-32-42.txt844 bytesmpdonadio
#42 2795489-42.patch3.12 KBmpdonadio
#33 interdiff-29-32.txt658 bytesmpdonadio
#32 2795489-32.patch3.12 KBmpdonadio
#32 2795489-32.patch3.12 KBmpdonadio
#29 interdiff-13-29.txt1.74 KBmpdonadio
#29 2795489-29.patch3.12 KBmpdonadio
#15 interdiff-04-15.txt3.09 KBmpdonadio
#15 2795489-15.patch3.38 KBmpdonadio
#13 interdiff-04-13.txt3.6 KBmpdonadio
#13 2795489-13.patch3.12 KBmpdonadio
#4 datetimeplus_class-2795489-4-TEST.patch1.24 KBcilefen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pjohn created an issue. See original summary.

pjohn’s picture

Title: Class limits dates to a smaller range than DateTime » DateTimePlus class limits dates to a smaller range than DateTime
mpdonadio’s picture

Issue tags: +Needs tests

Anybody want to add a cases to DateTimePlusTest::providerTestDates() and DateTimePlusTest::providerTestDateArrays() to demonstrate this and post a test-only patch?

cilefen’s picture

Abraham Lincoln's birthday does not fail.

mpdonadio’s picture

Status: Needs review » Postponed (maintainer needs more info)

OK, we have a passing, valid test using the two different create static methods that would support dates < 1970 (which are mostly thin wrappers around the \DateTime methods w/ timezone and language support), and they render out properly.

Also manually tested this date through the datetime field via the UI, and everything worked as expected.

@pjhon, we need a little more info here about what your fail case here is. Code + w/ input and expected output would be best.

Going to postpone this. Eventually, I think we want to commit this as a test improvement even if we don't uncover a bug here.

pjohn’s picture

I've narrowed it down to the createFromFormat() method. Specifically:

$d = DrupalDateTime::createFromFormat('Y-m-d', '1776-07-04');
echo($d->format('Y-m-d')); // Prints '1969-12-31'
mpdonadio’s picture

Still can't reproduce this.

use Drupal\Component\Datetime\DateTimePlus;
use Drupal\Core\Datetime\DrupalDateTime;

$date1 = \DateTime::createFromFormat('Y-m-d', '1776-07-04');
$date2 = DateTimePlus::createFromFormat('Y-m-d', '1776-07-04');
$date3 = DrupalDateTime::createFromFormat('Y-m-d', '1776-07-04');

print $date1->format('Y-m-d') . PHP_EOL;
print $date2->format('Y-m-d') . PHP_EOL;
print $date3->format('Y-m-d') . PHP_EOL;

yields

1776-07-04
1776-07-04
1776-07-04

for me. Editing the test @cilefen posted w/ that date also passes.

gambry’s picture

On some instances I get the same issue.
Script at #7 returns:

1776-07-04
1970-01-01
1970-01-01

I'm trying to figure it out why as it's definitely something related with PHP settings, because its the Datetime $date->getTimestamp() returning FALSE instead of the negative unixtimestamp.

But what really worries me is the fact FALSE is the expected behaviour according with a comment on the PHP manual for getTimestamp(), and no mentions about the negative results.

The solution should be to use $date->format('U'), which correctly returns the timestamp.
I'll keep investigating.

gambry’s picture

Status: Postponed (maintainer needs more info) » Needs work

DateTime::getTimestamp() returns FALSE on systems affected by the Year 2038 bug.
To test if your PHP instance is affected, you can quickly execute this code:

$date = '2040-02-01';
$format = 'l d F Y H:i';

$mydate1 = strtotime($date);
echo '<p>', date($format, $mydate1), '</p>';

If you get a 1970 date, then you are affected.

I suggest to replace the getTimestamp() with format('U') or to use a better way to set the $datetimeplus value.

UPDATE: although DateTime::format('U') returns the right negative value, $datetimeplus->setTimestamp($date->format('U')) is still incorrect.

mpdonadio’s picture

If this really is a 32-bit vs 64-bit time_t problem on the host system, then this is probably out of our control?

If we can't fix this in Drupal then we should add a check to `system_requirements()` and display a warning message.

tacituseu’s picture

From: https://3v4l.org/d05V7 (remember to check eol versions)

Output for 4.3.0 - 5.0.5, 5.2.6 - 5.6.28, hhvm-3.10.0 - 3.13.2, 7.0.0 - 7.1.0RC6
Wednesday 01 February 2040 00:00
Output for 5.1.0 - 5.2.5
Thursday 01 January 1970 01:00

looking at changelog of 5.2.6 it could be this bug: https://bugs.php.net/bug.php?id=44209

gambry’s picture

@tacituseu the bug is on the architecture.
PHP running on 32bit always returns the wrong values when using UNIXTIME out of the range 1970 - 2038.
I tested PHP 5.6.19 and 7.0.4 on Acquia Dev Desktop (running on 32bit on a 64bit-capable Mac) and bug is there too.

I'm afraid the only solution is the one proposed on #11.

mpdonadio’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
3.12 KB
3.6 KB

Here is some additional testing, and a status message.

Status: Needs review » Needs work

The last submitted patch, 13: 2795489-13.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing
FileSize
3.38 KB
3.09 KB

OK, here is a rework of DateTimePlus::createFromFormat() to not rely on timestamps. All of my local testing is green.

People who are experiencing this problem (or any Windows/XAMPP users), can you manually test out this patch to see if works for you?

gambry’s picture

#15 happy to test this, however if the rework is due this issue then we rely on timestamps in a lot of other places in the core (the Timestamp field and the Date view plugins, just to give some examples) and replacing timestamp usage in all of them will be really difficult.

So - basically - if people are affected from this bug we should alert them to fix it from the origin, and not patching our code which already works.

PS: I like the way you improved DateTimePlus::createFromFormat(). It was really messy, so more than happy to proceed with the clean up in here or a separated issue.

gambry’s picture

Status: Needs review » Needs work
Issue tags: -Needs manual testing

As I was presuming the patch lets createFromFormat() create and store the date properly (2040-07-06 on my test), but then I get the error:
Uncaught PHP Exception Exception: "The timestamp must be numeric." at core/lib/Drupal/Component/Datetime/DateTimePlus.php line 172

Due the DateTimeDefaultFormatter::formatDate() trying to format the date from a FALSE value returned by $date->getTimestamp().

Also #15 doesn't seem to have the system_requirements() changes from #13.

wturrell’s picture

@mpdonadio I get the same error as @gambry when testing the patch with dates > 2038 on Acquia Dev Desktop (PHP 7.0.4, Mac)

mpdonadio’s picture

Tagging this for Framework Manager review for how to proceed here. Probably best to start reading from #8 onward.

The basic problem is PHP compiled for 32-bit machines having a limited timestamp range, compared to 64-bit machines (strictly speaking, those with 64-bit time_t).

There are a few options here.

#13 adds some additional test coverage, and an addition to system_requirements to show a notice.

#15 is an attempt to fix DateTimePlus. Unit tests pass, but manual testing fails on 32-bit architecture because of downstream problems (this could potentially really snowball).

My recommendation is to start with #13, and update the TimestampDatetimeWidget to show a message about this to 32-bit users, and potentially validate the input for these uses to prevent errors and unexpected input. Before going down that road, wanted to punt up the food chain to get thoughts on the best way to proceed.

cilefen’s picture

Version: 8.2.x-dev » 8.4.x-dev
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs title update, +DrupalCampNJ2017, +Triaged for D8 major current state

Triaging:

  • This needs a title and summary update to indicate this affects 32-bit systems only.
  • Comment #19 lays out the possible ways to move forward. We will need a summary update to reflect the way it will go.
  • I agree with this remaining major.

cilefen credited ednawig.

cilefen’s picture

Crediting edna and myself on the triage.

wturrell’s picture

Title: DateTimePlus class limits dates to a smaller range than DateTime » DateTimePlus - 2038 bug with timestamps on 32-bit PHP
Issue summary: View changes
Status: Needs work » Needs review

Issue summary rewritten (could somebody else please proof read it...)

wturrell’s picture

mpdonadio’s picture

Status: Needs review » Needs work

Can someone who is running into this problem let us know which widgets this is impacting? I went to update the widgets, and wasn't sure exactly where we want to do this.

The node created/changed fields? Datetime widget? Datelist widget? I tried to simulate this, and noticed that the node created/changed fields already have a limit on them to 1902-2037, but this may have just been Chrome enforcing the min/max.

wturrell’s picture

Issue summary: View changes

For me, Date only and Date and Time (I'd suggest creating a site in Acquia Dev Desktop is the easiest way to simulate it.)
If I switch to the Select List widget, it displays years outside 1900 and 2038, but resets to 1970 on saving.

As you say, "Authored on" has a date range specified using HTML5 attributes, so that's enforced in modern browsers even with JS off.

gambry’s picture

The issue is not on Drupal nor - from my understanding - on PHP. The issue is how 32bit systems handle timestamps over the range 1902-2038 where the 32bit memory address space is not enough.

From my tests strtotime() and DateTime::getTimestamp() are affected, but there could be more.
Fixing this will be a crazy job (48 occurrences of strtotime() and 43 of getTimestamp() only in core) and additionally the improvements will only give benefits to websites (!) running on 32bit (!!) making use of dates out of the range 1902 - 2038. A really edge and limited scenario IMHO.

Testing can be done using Acquia DevDesktop (while they won't upgrade it) or docker:
- This returns boolean FALSE instead of the timestamp integer:
docker run -i docker32/php:5.6 php -r 'echo gettype(strtotime("2040-02-01"));'

- This generally shows the issue, including the false positive output of $date->format("U") returing a timestamp integer different from the input value:
docker run -i docker32/php:5.6 php -d date.timezone='UTC' -r '$date = new DateTime("2040-02-01"); echo $date->format("U");echo gettype($date->getTimestamp());echo date("Y-m-d H:i:s", $date->format("U"));'

As said on #16 I'm more than happy to proceed with the work, but I still think the best option is a message during installation and on the status report page for instances running on 32bit systems.

wturrell’s picture

Title: DateTimePlus - 2038 bug with timestamps on 32-bit PHP » 2038 bug with PHP timestamps on 32-bit systems - warn users?
Issue summary: View changes

Made another pass at the issue summary (and tested strtotime() myself).

mpdonadio’s picture

I think we have gone far enough down the road of trying workaround to realize that it is infeasible. Warning users is really all we can do. I think having it in system_requirements() is fine, and we have some additional tests that will only be run on non-32bit systems. This is just some minor adjustments to #13.

gambry’s picture

Assigned: Unassigned » gambry
gambry’s picture

Assigned: gambry » Unassigned
Status: Needs review » Needs work
  1. +++ b/core/modules/system/system.install
    @@ -904,6 +904,16 @@ function system_requirements($phase) {
    +      'severity' => REQUIREMENT_INFO,
    

    This should be REQUIREMENT_WARNING. If there are no warnings/errors the 'Requirements Review' screen is skipped at all, leaving the user unaware of the issue.
    As REQUIREMENT_WARNING the user will be notified and able to proceed by clicking 'continue anyway'.

Manually tested (through the docker image `docker32/php:5.6`) and apart from feedback above all looks good.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
3.12 KB
3.12 KB

OK, #31 is addressed.

mpdonadio’s picture

FileSize
658 bytes

Blerg.

gambry’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

  • catch committed bb25a5b on 8.4.x
    Issue #2795489 by mpdonadio, cilefen, ednawig: 2038 bug with PHP...

  • catch committed 61624a3 on 8.3.x
    Issue #2795489 by mpdonadio, cilefen, ednawig: 2038 bug with PHP...
catch’s picture

Status: Reviewed & tested by the community » Needs review
  1. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
    @@ -308,6 +308,16 @@ public function providerTestDates() {
    +    // On 32-bit systems, timestamps are limited to 1901-2038.
    

    Here it's 1901.

  2. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
    @@ -331,6 +341,16 @@ public function providerTestDateArrays() {
    +    // On 32-bit systems, timestamps are limited to 1970-2038.
    

    Here it's 1970.

    This is going to permanently warn on Windows isn't it? A quick check says it's limited to PHP_INT_4 even on 64 bit. Should we change the warning if the system is Windows?

mpdonadio’s picture

Are we filing a f/up issue to address #37 or are we going to revert and fix this patch?

#37-2 is an oopsie, it should be 1901. Strictly, it's a specific date in 1901 and a specific date in 2038 (I forget the exact ones). TimestampDatetimeWidget explicitly clamps to 1902/01/01--2037/12/31 because of this.

The problem is PHP complied for 32-bit even when run on an underlying 64-bit architecture (compiling and linking w/ the -m32 flag). 2038 is the 0x7FFFFFFF (PHP_INT_MAX on 32-bit), 1901 is 0x80000000 (ie, PHP_INT_MIN on 32-bit). Apparently, this is most Windows installs and Acquia Dev Desktop, which is why I used `PHP_INT_SIZE === 4` as the check and not for Windows in the test and in system_requirements().

  • catch committed 2f18ae0 on 8.4.x
    Revert "Issue #2795489 by mpdonadio, cilefen, ednawig: 2038 bug with PHP...

  • catch committed 83cfa86 on 8.3.x
    Revert "Issue #2795489 by mpdonadio, cilefen, ednawig: 2038 bug with PHP...
catch’s picture

I've reverted, fingers ahead of my brain this morning.

@mpdonadio so that makes sense but I wonder what the message means for people running on Windows. But then taking action for people is relatively hard if they run into this anyway.

mpdonadio’s picture

FileSize
3.12 KB
844 bytes

I don't know what we can really do here to solve the problem. We can't make the problem go away for Windows users, since there is really no solution other than to use a VM for local dev if they deploy to Linux. But, that doesn't help people who deploy to Windows and encounter this. There is also the weird case of developing on OSX, etc, and deploying to Windows environments (I can say this isn't a hypothetical).

With this patch, users will see the warning on install. They will also see it in status reports.

I see two potential additional options (not necessarily mutually exclusive).

1. We add a page in the Documentation describing this problem. In the system_requirements() warning, we can add a link to this new page. Though not really a change, we can issue a CR to announce that we now check for this and warn people (maybe do this instead of the doc page?)

2. We can add a warning to the form element #description everywhere you can do date entry. This would be the timestamp(*) widget, the datetime and daterange widgets, and a bunch of views widgets. Not sure how to handle the cases where people use the datetime form element directly for things. I started to do this and then decided it was overkill (esp, when I started thinking about that last case).

(*) The timestamp field item (and changed, created) and widget may require some more digging. The annotation limits to 4 bytes, and so does the min/max attributes on the widget, but there isn't explicit form validation for when these get bypassed by browsers that don't support HTML5 date/time entry. Storage specifies 'int', and this is 4 bytes on MySQL. IMHO, people should never be using these via Field UI, just as base fields, but that ship has sailed.

jhedstrom’s picture

Though not really a change, we can issue a CR to announce that we now check for this and warn people (maybe do this instead of the doc page?)

I like this approach.

We can add a warning to the form element #description everywhere you can do date entry

This would certainly avoid any confusion as to what is happening :)

gambry’s picture

from http://windows.php.net:

The x64 builds of PHP 5 for Windows are experimental, and do not provide 64-bit integer or large file support.

PHP 7 provides full 64-bit support. The x64 builds of PHP 7 support native 64-bit integers, LFS, 64-bit memory_limit and much more.

So the message will show only to Windows users running PHP 5.x/7.x 32 bit or PHP 5.x 64bit while still experimental.
But as #41 says: "taking action for people is relatively hard if they run into this anyway."

I think 1) is a good solution and it requires just a small change on the patch + a CR.
2) is probably shooting to a fly with a tank. It will require additional work and manual test, giving no additional value to users (as we already show the message on installation, on every update and on the status report page).

alexpott’s picture

Issue tags: +Triaged core major

Discussed with @catch, @cilefen, @cottser, @laurii, and @xjm. We agreed that this is a major issue - there is no workaround and users that have the problem are unlikely to know unless we tell them.

mpdonadio’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

This needs a re-roll b/c the array mega-issue.

Munavijayalakshmi’s picture

Assigned: Unassigned » Munavijayalakshmi
Munavijayalakshmi’s picture

Assigned: Munavijayalakshmi » Unassigned
Status: Needs work » Needs review
FileSize
3.05 KB

Rerolled the patch.

jofitz’s picture

Issue tags: -Needs reroll

Removed the Needs Reroll tag.

gambry’s picture

I think this can be RTBC, but we still need to answer #42 question. As I've already said I'm more keen to option 1) as it will require only a CR.

@mpdonadio @jhedstrom thoughts ?

wturrell’s picture

Just created a new community docs page (not yet approved) for this:

https://www.drupal.org/docs/8/system-requirements/limitations-of-32-bit-php

jhedstrom’s picture

@gambry I think I prefer #42.1 as well. The approach of adding warnings to all date entry widgets seems overkill.

I think this is RTBC once there is a CR drafted.

gambry’s picture

Removed the 'Needs Framework Manager review' tag @catch answered on #41.
Does this patch requires backport to D7?

@wturrell do you fancy writing the CR? The documentation you wrote already has all the info required for it.

mpdonadio’s picture

Did a copy/paste from the doc page to a CR.

gambry’s picture

@mpdonadio I don't see any CR attached to this issue :(

wturrell’s picture

wturrell’s picture

Issue summary: View changes

(Added to Issue Summary)

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

The CR looks good, as does the patch. Thanks all!

yogeshmpawar’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
4.16 KB
3.81 KB

So here's some clean up for patch so lets not go few coding standard issues in 8.4.x branch & also added a interdiff. so marking this issue as "Needs Review".

mpdonadio’s picture

#59, thanks for fixing those, but we can't mix coding standard fixes and bugfixes in the same patch; they are considered out-of-scope.

yogeshmpawar’s picture

coding standard fixes are related to current patch only they are not related with whole file, so i think it will be considered in current patch & if not then we can RTBC #48.

gambry’s picture

Yeah, patch on #48 has some coding standard issues needed to be addressed.
RE the arrays being broken due going over the 80chars limit, I'll leave the decision to the subsystem maintainers but if they accept breaking those into own lines (and so accepting patch in #61) then there is another issue to be fixed:

+++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
@@ -308,6 +308,20 @@ public function providerTestDates() {
+        'America/Chicago',
+        '1809-02-12T10:30:00-06:00'
+      ];

A comma should follow the last multiline array item. This issue appears on DateTimePlusTest several times on the patch.

mpdonadio’s picture

-      [['year' => 2010, 'month' => 2, 'day' => 28], 'America/Chicago', '2010-02-28T00:00:00-06:00'],
+      [[
+        'year' => 2010,
+        'month' => 2,
+        'day' => 28
+        ],
+        'America/Chicago',
+        '2010-02-28T00:00:00-06:00'
+      ],
       // Create date object from date array with hour.
-      [['year' => 2010, 'month' => 2, 'day' => 28, 'hour' => 10], 'America/Chicago', '2010-02-28T10:00:00-06:00'],
+      [[
+        'year' => 2010,
+        'month' => 2,
+        'day' => 28,
+        'hour' => 10
+        ],
+        'America/Chicago',
+        '2010-02-28T10:00:00-06:00'
+      ],
       // Create date object from date array, date only.
-      [['year' => 2010, 'month' => 2, 'day' => 28], 'Europe/Berlin', '2010-02-28T00:00:00+01:00'],
+      [[
+        'year' => 2010,
+        'month' => 2,
+        'day' => 28
+        ],
+        'Europe/Berlin',
+        '2010-02-28T00:00:00+01:00'
+      ],
       // Create date object from date array with hour.
-      [['year' => 2010, 'month' => 2, 'day' => 28, 'hour' => 10], 'Europe/Berlin', '2010-02-28T10:00:00+01:00'],
+      [[
+        'year' => 2010,
+        'month' => 2,
+        'day' => 28,
+        'hour' => 10
+        ],
+        'Europe/Berlin',
+        '2010-02-28T10:00:00+01:00'
+      ],

This section wasn't touched by the patch to fix the bug, so it is out of scope. There are also some additional coding standard problems that would need to be fixed to fully comply (def trailing commas on the last entry, would have to run phpcs for a fill list). In general, when we have a non-compliant file, we do our best to match the usage in the file, especially w/in a single method. Not making out-of-scope changes is extremely important when we need to go through history to trace out regressions.

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #48 is still RTBC.

wturrell’s picture

FWIW, the array comma thing will eventually be fixed everywhere by this: #2572613: Fix 'Drupal.Array.Array.CommaLastItem' coding standard

cilefen’s picture

Status: Reviewed & tested by the community » Needs work

Good job everyone but I'm not yet ok with the approach.

  1. +++ b/core/modules/system/system.install
    @@ -929,6 +929,15 @@ function system_requirements($phase) {
    +      'description' => t('You are running on a system where PHP is compiled or limited to using 32-bit integers. This will limit the range of dates and timestamps to the years 1901-2038.'),
    

    According to the issue summary, it's actually worse than the requirements description indicates—displaying wrong dates without warning. The description makes it seem as though the widgets would be limited.

    Field types/widgets affected: (at least) the Date only and Date and Time, including "Select list" widget. You can choose years outside the 1900 to 2038 range, but because PHP can't handle them they will be stored/displayed as 1970-01-01 without warning.

    To me, that's an untenable situation with no workaround on affected platforms. I am posing the question: Should this be REQUIREMENT_ERROR?

  2. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
    @@ -308,6 +308,16 @@ public function providerTestDates() {
    +
    +    // On 32-bit systems, timestamps are limited to 1901-2038.
    +    if (PHP_INT_SIZE !== 4) {
    +      // Create a date object in the distant past.
    +      $dates[] = ['1809-02-12 10:30', 'America/Chicago', '1809-02-12T10:30:00-06:00'];
    +      // Create a date object in the far future.
    +      $dates[] = ['2345-01-02 02:04', 'UTC', '2345-01-02T02:04:00+00:00'];
    +    }
    

    Is it just me or should we not make this conditional on PHP_INT_SIZE and instead fail on 32-bit systems?

gambry’s picture

Status: Needs work » Needs review

1. To me, that's an untenable situation with no workaround on affected platforms. I am posing the question: Should this be REQUIREMENT_ERROR?

The bug is on the architecture (nor PHP nor Drupal) and has been there since the beginning of Unix timestamp usage. While we had be using 32bit systems (5-10 years ago? So from D1 to D7), we were all affected.

If you use an affected environment but you don't need to use date out of the 1900-2038 range, you are safe and raising the error is pointless.

Is it just me or should we not make this conditional on PHP_INT_SIZE and instead fail on 32-bit systems?

For the reason above I don't think tests should fail. Your environment is not sick or not capable, it's just deprecated.
Drupal had ran and can still run on it, the problem is the architecture and not using drupal is the minor of problems.
And again if you don't use dates out of range, you can keep running on affected environments drupal even after 2038 (if system boots :p).

gambry’s picture

Status: Needs review » Needs work

@cilefen and I discussed and agreed the message may need to - I quote - "scare the user up" enough, changing the system_requirements() message to include the link Read more about 2038 bug.

gambry’s picture

Status: Needs work » Needs review
Issue tags: +DevDaysSeville
FileSize
2.38 KB
3.13 KB

Adding the link to the 2038 bug wikipedia page + fixing coding standard errors from #48 tests results.

cilefen’s picture

+++ b/core/modules/system/system.install
@@ -929,6 +929,15 @@ function system_requirements($phase) {
+      'description' => t('You are running on a system where PHP is compiled or limited to using 32-bit integers. This will limit the range of dates and timestamps to the years 1901-2038. <a href="https://en.wikipedia.org/wiki/Year_2038_problem">Read More about 2038 bug</a>.'),

How about "Read more about the 2038 bug"? Also, the URL needs a token replacement for translation. I still think this message does not explain enough about the problems that will be faced.

wturrell’s picture

Could we not link to the docs page we've specially created? (if so, use node ID in case URL changes):

https://www.drupal.org/node/2860265

gambry’s picture

#70 URL is now a token, shame on me.
#71 you are right. The documentation page already has the link to wikipedia. I've also updated the content adding the code to test if your php is affected.

Then I'm not sure what we can do more than this.
Using REQUIREMENT_ERROR is a drastic and disruptive option as it will block installations on affected platforms. That means after patch is committed people running PHP on Windows 7 or Acquia Dev Desktop (etc.) won't be able to install drupal anymore. Or if already installed with "fail [these tests] on 32-bit systems" won't be able to run tests anymore.

I don't think the situation is that critical. People will be still able to use Drupal normally as long as they don't use datetime* fields over that range.
Besides as mention in #42.2 we are ALL affected too as the timestamp field (and created and changed) use MySQL int type, which is 4 bytes and so affected by the same problem.

Thoughts && suggestions?

cilefen’s picture

+++ b/core/modules/system/system.install
@@ -929,6 +929,15 @@ function system_requirements($phase) {
+      'description' => t('You are running on a system where PHP is compiled or limited to using 32-bit integers. This will limit the range of dates and timestamps to the years 1901-2038. <a href="@drupal-doc">Read the documentation page</a>.', ['@drupal-doc' => 'https://www.drupal.org/docs/8/system-requirements/limitations-of-32-bit-php']),

Yeah I like this better. Can we make the anchor text "Read about the limitations of 32-bit PHP."?

gambry’s picture

Done.

gambry’s picture

Issue tags: +Needs backport to D7

Wondering if this one needs backport to D7? Maybe just the system_requirements()?

In the same note, due #42:

The timestamp field item (and changed, created) and widget may require some more digging. [...] Storage specifies 'int', and this is 4 bytes on MySQL. IMHO, people should never be using these via Field UI, just as base fields, but that ship has sailed.

Do we need a follow-up targeting D9 to review/discuss/address this issue?

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

I think #74 has addressed the remaining feedback.

Do we need a follow-up targeting D9 to review/discuss/address this issue?

That seems like a good idea to eliminate in Drupal 9.

cilefen’s picture

Status: Reviewed & tested by the community » Needs work

I overlooked #71 when I wrote #73. I would like to have the requirements document reviewed and approved before this change is included in a release, epecially if the URL is apt to change. I am torn between postponing and needs work, but I'm going with needs work.

Speaking for myself, I have no qualms about the warning on 32-bit, and I respect the argument made in #67 about making some test data conditional.

xjm’s picture

  1. +++ b/core/modules/system/system.install
    @@ -929,6 +929,15 @@ function system_requirements($phase) {
    +  if (PHP_INT_SIZE === 4) {
    

    Theoretically, maybe we should do less than 5? There could be problems I haven't considered with this suggestion.

  2. +++ b/core/modules/system/system.install
    @@ -929,6 +929,15 @@ function system_requirements($phase) {
    +      'title' => t('Limited Date Range'),
    

    This should be sentence-case, not title-case. ("Date" and "Range" should be lowercase instead.)

  3. +++ b/core/modules/system/system.install
    @@ -929,6 +929,15 @@ function system_requirements($phase) {
    +      'description' => t('You are running on a system where PHP is compiled or limited to using 32-bit integers. This will limit the range of dates and timestamps to the years 1901-2038. Read about the <a href="@drupal-doc">limitations of 32-bit PHP</a>.', ['@drupal-doc' => 'https://www.drupal.org/docs/8/system-requirements/limitations-of-32-bit-php']),
    

    We also should be using :placeholder here instead of @placeholder, and drupal-doc is not the best name. Do hyphens even work? Did someone manually test this warning (maybe by hacking to if (TRUE) or such)?

I don't know how to go about getting the doc page approved. New handbook policy makes me sad.

wturrell’s picture

@xjm: The docs page isn't actually in the official handbook, just community docs (partly for reason you hint at), so there's no formal "approval" to best of my knowledge, just the section maintainer keeping an eye on things, and in this case adding new pages to the parent, which others can't.

I've just messaged @opdavies who's the section maintainer for System Requirements.

@cilefen: Only reason I mentioned URL was because it's supposedly best practice (not always observed) to use node IDs when linking from one documentation page to another in case someone comes up with a better name, content changes significantly or it's moved to a different section.

gambry’s picture

#78.1 : I don't think PHP_INT_SIZE < 5is semantically correct as that will read if the sizeof(long) in your system is less than 40bits).
Although I don't think there are systems having PHP_INT_SIZE < 4 (constant have been introduced with PHP 5.0.5), never say never && better safe than sorry! So I changed PHP_INT_SIZE === 4 instances with PHP_INT_SIZE <= 4 and PHP_INT_SIZE !== 4 with PHP_INT_SIZE >4.

#78.2 done!

#78.3 Yeah, "drupal-doc" is meaningless. FormattableMarkup::placeholderFormat() uses :url as placeholder for href so let's do the same.

gambry’s picture

As a separate note:

drupal-doc is not the best name. Do hyphens even work? Did someone manually test this warning (maybe by hacking to if (TRUE) or such)?

Apparently hyphens on placeholders work! Below a screenshot of Status Report page with #74 applied.
/Limited-date-range-warning

xjm’s picture

Thanks @gambry!

yogeshmpawar’s picture

Any Update on this issue ?

cilefen’s picture

It needs a review done.

gambry’s picture

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

I think the remaining issues have been addressed. Thanks everybody!

cilefen’s picture

Has the documentation page been finalized an formally published?

mpdonadio’s picture

#87, there is a notice on https://www.drupal.org/docs/8/system-requirements/limitations-of-32-bit-php that it needs review to be

This page has not yet been reviewed by System requirements maintainer(s) and added to the menu.

I don't know who can do this (I can't), and I don't see a docs maintainer anymore in MAINTAINERS.txt

Wim Leers’s picture

This patch looks great!

But I'm wondering if the right place for docs is actually https://www.drupal.org/docs/user_guide/en/install-requirements.html instead? Or perhaps also?

(It's very confusing that we have a wiki-style https://www.drupal.org/docs/8/system-requirements, and then a https://www.drupal.org/docs/user_guide/en which requires a patch + review process. It's not clear what the scope is of the user guide, and thus whether this belongs there too.)

gambry’s picture

I still think https://www.drupal.org/docs/8/system-requirements is the right place. We could create - now or in a later step - a parent PHP section to be consistent with https://www.drupal.org/docs/7/system-requirements and then make our page a child.
Besides this issues needs to backported, and for D7 that looks the best place to me.

Additionally we can link the doc page from https://www.drupal.org/docs/user_guide/en/install-requirements.html PHP section as well,
i.e. (also read about the limitations of 32-bit PHP)

gambry’s picture

May or may not related with what @Wim Leers flagged, but @opdavies raised a good point in here: this is not D8 specific so we need to find a general place otherwise we are going to have multiple version of the same documentation.

In the doc page discussion I suggested https://www.drupal.org/docs/develop "Documentation for developers about tools, processes, and standards that is not specific to a major version of Drupal."

catch’s picture

Status: Reviewed & tested by the community » Fixed

https://www.drupal.org/docs/8/system-requirements is the right place for this in general. However we should try to resolve what the relationship of that to the user guide is. My feeling is the user guide benefits from only covering the basics, and if we add as much information as the old docs page it will be hard to manage.

Committed fdc9ff9 and pushed to 8.4.x. Thanks!

  • catch committed fdc9ff9 on 8.4.x
    Issue #2795489 by mpdonadio, gambry, Yogesh Pawar, cilefen,...
gambry’s picture

Version: 8.4.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: -Needs backport to D7 +Novice
Related issues: +#2885413: Timestamp field items are affected by 2038 bug

I've created the D9 followup to address #42 timestamp storage concerns: #2885413: Timestamp field items are affected by 2038 bug.

Also re-opening for D7 backport. Adding the Novice tag as backport should be really straight forward.

xjm’s picture

Version: 7.x-dev » 8.4.x-dev
Status: Patch (to be ported) » Needs work

@gambry, we no longer backport to D7 in the same issue. A new issue can be created.

However, in the case of this issue, I'm reverting it, because the added test coverage is failing on all three PHP 5.5 environments (unfortunately). I'll queue tests for PHP 5.5 upon revert.

xjm’s picture

Issue tags: -Novice

Also the current scope is not novice as I have no idea why this would pass on PHP 5.6 but not 5.5 unless the 5.5 environments are configured as 32-bit. :)

gambry’s picture

Thanks @xjm, didn't know we now do it in a separated issue.

I'm reverting it, because the added test coverage is failing on all three PHP 5.5 environments (unfortunately)

That looks scary to me. :(

docker run -i php:5.5 php -r '$date = new DateTime("1809-02-12 10:30", new DateTimeZone("America/Chicago")); echo $date->format("c");'
1809-02-12T10:30:00-05:50

docker run -i php:5.6 php -r '$date = new DateTime("1809-02-12 10:30", new DateTimeZone("America/Chicago")); echo $date->format("c");'
1809-02-12T10:30:00-06:00

gambry’s picture

The issue is in the way PHP 5.5 calculates the offset for dates < 13th Dec 1901 (2038 bug minimum limit), but it works correctly for > 2038:

docker run -i php:5.6 php -r '$date = new DateTime("1809-02-12 10:30", new DateTimeZone("America/Chicago")); echo $date->getOffset()'
-21600

docker run -i php:5.5 php -r '$date = new DateTime("1901-12-14 10:30", new DateTimeZone("America/Chicago")); echo $date->getOffset()'
-21600

docker run -i php:5.5 php -r '$date = new DateTime("2345-02-12 10:30", new DateTimeZone("America/Chicago")); echo $date->getOffset()'
-21600

docker run -i php:5.5 php -r '$date = new DateTime("1809-02-12 10:30", new DateTimeZone("America/Chicago")); echo $date->getOffset()'
-21036

Not sure how to proceed. The sake of this test is to cover the instance against 2038 bug, and the test results show this properly.
So we could use a different format to compare the dates i.e. not printing the offset.

  • xjm committed 7d3bdf0 on 8.4.x
    Revert "Issue #2795489 by mpdonadio, gambry, Yogesh Pawar, cilefen,...
xjm’s picture

Sorry, failed pushing the revert last night; did so now.

Nice research @gambry.

xjm’s picture

What about both using a different format for 5.5, but sticking a version check in the test and still using the existing tests for 5.6+? Is that a decent idea?

mpdonadio’s picture

This is potentially another zonedb problem, with the different PHP versions having different TZ databases with different historical / pre-1970 data.

I think the format 'Y-m-d H:i:s e' may be best here. Not sure that will work...

gambry’s picture

@mpdonadio I agree it looks like a TZ db issue.

I know I first suggested it, but honestly changing the format due an edge (using PHP 5.5, already deprecated) of an edge (only dates <= 1901) scenario soft fails looks too much to me. It means changing all expected values of those providers.

I suggest we keep format and values as they are and we conditionally check the running PHP version before adding '1809-02-12 10:30' to the provider, in this way:

  • We don't change the validity of the tests. If the running system is affected by the 2038 bug then the '2345-01-02 02:04' will fail.
  • We just have a stronger check of most recent - and hopefully mainly used - PHP versions (>= 5.6) by adding '1809-02-12 10:30'.
mpdonadio’s picture

I think I am OK with the approach in #103, but I would like to poke through the zonedb repo to double check the assumption so we can add a comment around the version check.

I also think we should (maybe in followup)

- Add the output of http://php.net/manual/en/function.timezone-version-get.php to system_requirements as an INFO.
- Update the docs to also note how different zonedb can influence how timestamps/dates get rendered out.

catch’s picture

#103 and #104 both sound good to me.

mpdonadio’s picture

Assigned: Unassigned » mpdonadio

Assigning to myself. I started thinking that we want to version check on zonedb and not necessarily PHP version, but I need to dig into this a bit.

My MAMP is

PHP 5.5.38 => 2015e
PHP 5.6.25 => 2016f
PHP 7.0.10 => 2016f

mpdonadio’s picture

And this is truly odd, https://3v4l.org/lo0Ya

mpdonadio’s picture

OK, I have been staring at changelogs for both PHP and tz, and can't find a cause, which really bothers me if we want to add a version check. If you look at https://3v4l.org/lo0Ya, you will see three potential databases that are bad, or a stretch of PHP versions, in the *middle* of the 5.5 and 5.6 releases.

I think this is the best we can do. The other option is

diff --git a/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
index 8ddd60b..1f0fc8e 100644
--- a/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
+++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
@@ -312,7 +312,10 @@ public function providerTestDates() {
     // On 32-bit systems, timestamps are limited to 1901-2038.
     if (PHP_INT_SIZE > 4) {
       // Create a date object in the distant past.
-      $dates[] = ['1809-02-12 10:30', 'America/Chicago', '1809-02-12T10:30:00-06:00'];
+      // @see https://3v4l.org/vve0b
+      if (!in_array(timezone_version_get(), ['2015.4', '2015.5', '2015.6'])) {
+        $dates[] = ['1809-02-12 10:30', 'America/Chicago', '1809-02-12T10:30:00-06:00'];
+      }
       // Create a date object in the far future.
       $dates[] = ['2345-01-02 02:04', 'UTC', '2345-01-02T02:04:00+00:00'];
     }
@@ -345,7 +348,10 @@ public function providerTestDateArrays() {
     // On 32-bit systems, timestamps are limited to 1901-2038.
     if (PHP_INT_SIZE > 4) {
       // Create a date object in the distant past.
-      $dates[] = [['year' => 1809, 'month' => 2, 'day' => 12], 'America/Chicago', '1809-02-12T00:00:00-06:00'];
+      // @see https://3v4l.org/vve0b
+      if (!in_array(timezone_version_get(), ['2015.4', '2015.5', '2015.6'])) {
+        $dates[] = [['year' => 1809, 'month' => 2, 'day' => 12], 'America/Chicago', '1809-02-12T00:00:00-06:00'];
+      }
       // Create a date object in the far future.
       $dates[] = [['year' => 2345, 'month' => 1, 'day' => 2], 'UTC', '2345-01-02T00:00:00+00:00'];
     }

?

gambry’s picture

Status: Needs review » Needs work

I like the version_compare(PHP_VERSION, '5.6.20', '>=') check more than !in_array(timezone_version_get(), ['2015.4', '2015.5', '2015.6']). It's more readable and developers are more confident with PHP versions and less with TZ dbs.

  1. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
    @@ -312,7 +312,10 @@ public function providerTestDates() {
    +      // @see https://3v4l.org/vve0b
    

    Why not linking to #108, which has the 3v4l.org link together with additional details?

  2. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
    @@ -312,7 +312,10 @@ public function providerTestDates() {
    +      if (version_compare(PHP_VERSION, '5.6.20', '>=')) {
    

    Is there any reason why this is 5.6.20? From version 5.6.15 bug is not present anymore.
    Below the summary of Correct/Wrong output for each version:
    Correct:
    5.5.0 - 5.5.24
    5.6.1 - 5.6.8
    5.6.15 - 5.6.30 (latest)

    Wrong:
    5.5.25 - 5.5.38 (latest)
    5.6.9 - 5.6.14

I must say nice work everyone, this was really good catch. Shame PHP 5.5 is now unsupported so instances running on top of it will never see a fix.

mpdonadio’s picture

gambry’s picture

Status: Needs review » Reviewed & tested by the community

Thanks a lot!

  • cilefen committed 9539eaf on 8.4.x
    Issue #2795489 by mpdonadio, gambry, Yogesh Pawar, cilefen,...
cilefen’s picture

Status: Reviewed & tested by the community » Fixed

Committed 9539eaf and pushed to 8.4.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

selinav’s picture

I can't upgrade my site on OVH share hosted.
What should I do to overstep this control ?

cilefen’s picture

@selinav Kindly open a new issue with the question.

selinav’s picture

I've open a new post about the impossibility to update the site because the message is blocking on OVH share hosted.
https://www.drupal.org/forum/support/upgrading-drupal/2017-12-08/php-203...

cilefen’s picture

As per #31 we do not intend to block update so there is a bug.

@selinav: You opened a forum post but I think we have a bug. Open an issue, not a forum post, and link it from here.

Is the PHP on this platform 32-bit, which likely is Windows? If so, do you have access to 64-bit PHP?

Edited: install/update

selinav’s picture

I have reported the bug
https://www.drupal.org/project/drupal/issues/2929999#comment-12379427

@cilefen : I wait an answer of OVH for tomorrow. For the moment I can't access to 64 bits PHP version.
Another people have reported the same problem on http://community.ovh.com/t/limitation-du-au-php-compile-en-32bits/5171/9

Dave52521’s picture

I was getting this error when installing onto wamp server wampserver3.1.0_x64.exe

I just changed the PHP Version to 7.1.9

Works ok now

hope this helps

Grayle’s picture

Isn't the problem also in MySQL? Signed integers (which is what timestamps use iirc) only go up to 2147483647. I had MySQL errors when entering dates for the year 2050. I think we'll also have to change the data type to bigint if we want to fix it properly.

gravisrs’s picture

Issue tags: -

Regarding #121 - this would be an easy fix. Most schema uses 'timestamp' storage item so all what we need is update

/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/TimestampItem.php class

and add implementation of defaultStorageSettings() that returns ['size'=>'big']