Problem

The problem with $date->getTimestamp() is that if the date is before 1901-12-13, it will return null on 32bit system. $datetimeplus will then be erroneously set to 1970-01-01. This is a major issue whenever an application is handling dates far in the past or future.

This is evident during the unit test of Drupal\Tests\Component\Datetime\DateTimePlusTest::testValidateFormat() when it attempted to put in date as "11-03-31 17:44:00" and expects the return value from DateTimePlus to be "0011-03-31 17:44:00". On a 32-bit, it will return "1970-01-01 00:00:00" instead.

======Output of Unit Test======

Drupal\Tests\Component\Datetime\DateTimePlusTest::testValidateFormat
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'0011-03-31 17:44:00'
+'1970-01-01 00:00:00'

========================

The system I'm testing on:
PHP 7.1.4 32bit
Apache 2.4.25
Windows 7
Drupal 8.4.x

Proposed resolution

The propose change will detect if the timestamp is beyond the scope of a 32bit integer and create the datetimeplus differently. Otherwise, it will create the datetimeplus like previously coded.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ProFire created an issue. See original summary.

mpdonadio’s picture

Version: 8.4.x-dev » 8.5.x-dev
Issue tags: -Datetimeplus +Needs tests, +Needs manual testing
Related issues: +#2795489: 2038 bug with PHP timestamps on 32-bit systems - warn users?

This was discussed at length in #2795489: 2038 bug with PHP timestamps on 32-bit systems - warn users?. If there is a workaround, then we can explore it.

mpdonadio’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -233,8 +233,13 @@ public static function createFromFormat($format, $time, $timezone = NULL, $setti
+      if ((float)$date->format("U") < -2147483648 || (float)$date->format("U") > 2147483647) {

Needs a comment on what is going on here and what those literals are.

Then, the `if (PHP_INT_SIZE)` checks in DateTimePlusTest should be removed and see if we have passing tests on 32-bit systems. We don't have any bots that support this, so it has to be verified manually.

ProFire’s picture

Status: Needs work » Needs review
FileSize
1.79 KB

Thank you for attending to this issue. What I hope to attempt to do is not just warn users, but actually solve the issue so that Drupal will work fine on 32bit too.

The literals are the actual limitations of 32bit integers, according to http://php.net/manual/en/language.types.integer.php
I have changed the maximum check to use PHP_INT_MAX instead of 2147483647. However, it is not feasible to use PHP_INT_MIN as it is only supported from PHP7 onwards. Drupal8 requires support from PHP5.5.9, thus I left -2147483648 as the literal.
I have added the comments on this in the code.

I've also added other relevant comments.

I look forward to another volunteer to manually test this too. I'm on Windows 7, running XAMPP, PHP7.1 32bit.

ProFire’s picture

I just understood what you meant by "if (PHP_INT_SIZE)" and I have updated the unit tests to reflect dates in the distant past and future. On my 32bit build, it has passed.

gambry’s picture

I'll have a look at this as soon as possible, I have a couple of environments running in 32bits.

If anyone has time in the meanwhile docker32/php is a docker image running in 32bits. You can also specifying the PHP versions as different versions may or may not been affected in the same way (see this and following comments)

neclimdul’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -233,8 +233,21 @@ public static function createFromFormat($format, $time, $timezone = NULL, $setti
+      if ((float)$date->format("U") < -2147483648 || (float)$date->format("U") > PHP_INT_MAX) {
+        //We deduct the amount of time since epoch so that the calculations for next line of code works
+        $bigDate = $date->format("U") - date("U");

Looks like we're adding 2-3 formats every creation? Can we just run it once?

Additionally hard coding "-2147483648" means it fails to act as expected on 64bit systems which is almost certainly the more common arch and the one this should be optimized for. Maybe we coud polyfill INT_MIN (as best we can) using INT_SIZE or something.

gambry’s picture

I think this issue can't be closed "won't fix" for the following reasons:
1) This is not a Drupal issue, not even PHP. It's deep in the 32bit architecture. Allowing Drupal to not fail won't fix the issue. It can actually even make it worse because we may let core correctly handle dates < 1901, but what about contrib modules? What about custom modules and custom code? or external libraries?
2) The tests is green because patch fixes the DateTimePlus implementation. But what about all the other strtotime() or getTimeStamp() calls? Are we going to fix those too? This problem has been raised in #2795489: 2038 bug with PHP timestamps on 32-bit systems - warn users? #17.
3) This is a int length problem in 32bits platforms. Those platforms have this limitation, which affects dates as well as math operations. That's a fact. If you are planning to have content handling dates < 1901 you are using the wrong platform as well as if you want to run a financial service. Fixing the Drupal side won't change the fact you do have a bigger issue.

We tell all of these with the alert message (we mention only the dates, but integers in general are affected).

I'll leave the decision to @mpdonadio and @jhedstrom as maintainers of the datetime modules and subsystems, and/or any framework manager.

gambry’s picture

I must mention I do understand Windows people may not have (yet) an alternative. But the effort on having all Drupal core and most of the modules - and some libraries - converted to allow dates < 1901 may be bigger then finding a solution to have 64bit PHP binary on Windows. :)

ProFire’s picture

Status: Needs work » Needs review

1)
I certainly agree that this is a 32bit architecture issue. But PHP is able to handle dates before 1901. It needs to be passed the correct value to do so, as example in:
+ $datetimeplus = new static($bigDate . " seconds", $date->getTimezone(), $settings);

This isn't a hack, but rather, to use float instead of int. Float is capable of handling wider range of values, which should be used instead. Thus the use of (float)$date->format("U").

2)
I strongly believe that the ability to handle wider range of dates won't break any contrib modules. This isn't a reduction of capabilities, but an expansion. With the datetime range expanded, I'm sure many contrib modules will benefit from this increased range.

3)
I also believe that with Drupal supporting 32bit, we open up drupal to wider audience/usage.

4)
I'm not sure of any alternative to the literal -2147483648. I'm hestitant to implement further computational checks as it won't gain any significant advantage, as compared to just a 1-off check with the literal. I'm very open to further suggestions/alternatives.

mpdonadio’s picture

Assigned: Unassigned » mpdonadio
Status: Needs review » Needs work
+++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
@@ -307,18 +307,11 @@ public function providerTestDates() {
-      if (version_compare(PHP_VERSION, '5.6.15', '>=')) {
-        $dates[] = ['1809-02-12 10:30', 'America/Chicago', '1809-02-12T10:30:00-06:00'];
-      }
+++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
@@ -343,18 +336,11 @@ public function providerTestDateArrays() {
-      if (version_compare(PHP_VERSION, '5.6.15', '>=')) {
-        $dates[] = [['year' => 1809, 'month' => 2, 'day' => 12], 'America/Chicago', '1809-02-12T00:00:00-06:00'];
-      }

These are invalid test changes. It will cause fails on a range of particular PHP, regardless of 32 vs 64 bit (see around #2795489-108: 2038 bug with PHP timestamps on 32-bit systems - warn users?). Needs work because of that.

In addition, the change is to ::createFromFormat, but the extreme dates don't go through that. I am a little baffled why we didn't add the dates to ::providerTestDateFormat(), too.

Taking this issue for a bit.

mpdonadio’s picture

Assigned: mpdonadio » Unassigned
Status: Needs work » Needs review
FileSize
5.4 KB

OK, here are test changes against 8.5.x

- removed the explicit 32-bit check
- added the extreme dates to the coverage for createFromFormat()
- added a new test to check the timestamp that comes back via createFromFormat(); https://3v4l.org/oh8mK make me confident this is correct

This comes up green for me locally in PHP7. Will trigger 5.5 and 5.6.

Can someone verify the fails, both test and input, on a 32-bit system and let us know what version of PHP exactly (the timezone database varies between versions and can impact some of the tests).

Assuming the bot likes this, any changes should make this version of DateTimePlusTest green, but I think we may need more timestamp testing.

ProFire’s picture

I ran your new test against the previous datetimeplus patched codes and found 3 additional failures:

1) Drupal\Tests\Component\Datetime\DateTimePlusTest::testDateFormat with data set #4 ('1809-02-12 10:30', 'America/Chicago', 'Y-m-d H:i', 'c', '1809-02-12T10:30:00-06:00')
Test new DateTimePlus(1809-02-12 10:30, America/Chicago, Y-m-d H:i): should be 1809-02-12T10:30:00-06:00, found 1809-02-12T11:30:00-06:00.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1809-02-12T10:30:00-06:00'
+'1809-02-12T11:30:00-06:00'

C:\xampp\htdocs\drupal-8.4.x\core\tests\Drupal\Tests\Component\Datetime\DateTimePlusTest.php:225

2) Drupal\Tests\Component\Datetime\DateTimePlusTest::testTimestamps with data set #2 ('1809-02-12 10:30', 'America/Chicago', '1809-02-12T10:30:00-06:00', -5076977400.0)
Test DateTimePlus::createFromFormat('Y-m-d H:i', 1809-02-12 10:30, America/Chicago): should be 1809-02-12T10:30:00-06:00, found 1809-02-12T11:30:00-06:00.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1809-02-12T10:30:00-06:00'
+'1809-02-12T11:30:00-06:00'

C:\xampp\htdocs\drupal-8.4.x\core\tests\Drupal\Tests\Component\Datetime\DateTimePlusTest.php:854

3) Drupal\Tests\Component\Datetime\DateTimePlusTest::testTimestamps with data set #3 ('2345-01-02 02:04', 'UTC', '2345-01-02T02:04:00+00:00', 11833956240.0)
Test timestamp for 2345-01-02 02:04 is -1050945648
Failed asserting that false matches expected 11833956240.0.

C:\xampp\htdocs\drupal-8.4.x\core\tests\Drupal\Tests\Component\Datetime\DateTimePlusTest.php:855

I am submitting a new patch that handles the timezone offset, and also to override the DateTime::getTimestamp() so that it will return float instead of false when the number goes beyond 32bit.

As usual, I'm on PHP7.1 32bit. The test with my patch passes.

gambry’s picture

I strongly believe that the ability to handle wider range of dates won't break any contrib modules.

I said the opposite: contrib modules may break websites. There are 43 calls to strtotime() in core only. I imagine the number of these calls in contrib and libraries is much more. By fixing DateTimePlus only we let the user believes he/she can use a wider range of dates, when this isn't totally true.

This isn't a hack

I have conflicting opinions about it. It is true Drupal and PHP on 32bit systems can both handle wider ranges of dates, as long as we avoid getting/setting timestamps. With the patch applied:

// Correct
$date = new DrupalDateTime('1890-05-05');
$date->format('Y-m-d H:i:s'); // 1890-05-05 00:00:00
$date->getTimestamp(); // -2513847600

// Fails
$date2 = strtotime('1890-05-05'); // FALSE

// Fails
$date3 = new DrupalDateTime();
$date3->setTimestamp($date->getTimestamp());
$date3->format('Y-m-d H:i:s'); // 2026-06-11 07:28:16
$date3->getTimestamp(); // 1781119696

// Fails
$date4 = DrupalDateTime::createFromTimestamp($date->getTimestamp());
$date4->format('Y-m-d H:i:s'); // 2026-06-11 07:28:16
$date4->getTimestamp(); // 1781119696

So if we continue with the work in here we have to fix the failing scenarios above, together with find a way to avoid developers to use strtotime() or any other code which can potentially break with dates out of 32bit timestamp allowance.

ProFire’s picture

I'm not entirely sure of the direction of Drupal. I trust the direction of Drupal Community.

We ain't responsible to fix strtotime() as this is not Drupal's function. But since DateTimePlus is Drupal's code, I would assume that we as drupal community have a responsibility to ensure the function works as expected, particularly for those contrib modules that rely on it.

Likewise, for the contrib modules, it is their responsibility to handle their own codes in 32bit systems. Should the developers behind the contrib module rely on Drupal's DateTimePlus, they can count on the community to give them the best support. And by relying on Drupal's DateTimePlus, they can focus on their functionality instead of taking care of breaking changes due to 32/64 bit differences when it comes to datetime.

Do advise further. I'm keen to see how we can improve Drupal as we progress as a whole.

mpdonadio’s picture

Status: Needs review » Needs work

Not mentioning code standard, comment, polishing tweaks yet.

+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -233,8 +233,23 @@ public static function createFromFormat($format, $time, $timezone = NULL, $setti
+      if ((float)$date->format("U") < -2147483648 || (float)$date->format("U") > PHP_INT_MAX) {

Still need to add the 32-bit check to this.

+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -657,5 +672,9 @@ public function format($format, $settings = []) {
+  public function getTimestamp() {
+	return $this->format("U");
+  }

This needs a 32-bit check, too, to only use this method on 32-bit systems. It is also a BC break because \DateTime::getTimestamp() returns an int, while this will return a string, https://3v4l.org/T76Xv. This will cause problems where strict type checking is used.

Also need to merge in the test additions from #12 so we have a single patch. It is also helpful to post an interdiff with each new patch.

This is Needs Work for the reasons above. Leaving the Needs tests because we may need some more assertions.

Still pondering the comments from #13 on.

ProFire’s picture

Status: Needs work » Needs review

I agree on the BC break. We should give this portion a consideration. I could typecast it to int when it's within the range limit, and then float when it is outside of range limit. But I need further advise. If it's a serious BC break, then perhaps this could be due next major release?

mpdonadio’s picture

Status: Needs review » Needs work

I appreciate your work and desire to figure out a solution here, but this issue is still Needs Work for the reasons in #16 (see issue status meanings). It will also require some nitpicks over coding standards at some point and probably a Change Record, but we are not at that point yet.

There can be no change to the way the class works on a 64-bit system including returned types; any potential changes need 32-bit checks in the code.

I am very worried about the impact of this change, especially considering our existing test coverage in this class, DrupalDateTime, the widgets, the formatters, and views, and the fact that we don't have 32-bit testbots to do full runs. The issue of BC is also a very serious consideration, as it is supposed to be maintained through minors, so Drupal 9 would be first place we can have one (except in extreme situations).

ProFire’s picture

Thank you Mpdonadio for recognising my work. Please give my work one more consideration. I'm confident with this patch, there won't be any breaking change on 64bit.

This patch will attempt to return the timestamp in integer whenever possible. On 64bit, it will return integer as expected. On 32bit, it will return integer when it is within range of the integer limit, but it will return float when it is outside of the range.

This patch also contains the testsuite changes Mpdonadio did.

Being a user of D8 on production, BC is important to me. I wouldn't want my website to break.

I look forward to the community having a 32bit testbot.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

pameeela’s picture

Status: Needs review » Closed (duplicate)
Issue tags: +Bug Smash Initiative
Related issues: +#2885413: Timestamp field items are affected by 2038 bug

@ProFire thanks so much for your work on this issue!

It seems this is a duplicate of #2885413: Timestamp field items are affected by 2038 bug so I am going to close this to avoid having two streams of work dedicated to fix this. I would encourage you to check out the progress of that issue as it has had updates much more recently than this one.

I will also add you as a contributor to the issue so that you'll get credit if/when it gets fixed.

Thanks!