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 warningsFigure out if/how we can run a 32-bit test?Change record
User interface changes
Warnings on install, on system status (translatable text string)
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#110 | interdiff-107-110.txt | 1.46 KB | mpdonadio |
#110 | 2795489-110.patch | 3.46 KB | mpdonadio |
#108 | 2795489-107.patch | 3.4 KB | mpdonadio |
Comments
Comment #2
pjohn CreditAttribution: pjohn commentedComment #3
mpdonadioAnybody want to add a cases to DateTimePlusTest::providerTestDates() and DateTimePlusTest::providerTestDateArrays() to demonstrate this and post a test-only patch?
Comment #4
cilefen CreditAttribution: cilefen commentedAbraham Lincoln's birthday does not fail.
Comment #5
mpdonadioOK, 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.
Comment #6
pjohn CreditAttribution: pjohn commentedI've narrowed it down to the
createFromFormat()
method. Specifically:Comment #7
mpdonadioStill can't reproduce this.
yields
for me. Editing the test @cilefen posted w/ that date also passes.
Comment #8
gambryOn 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.
Comment #9
gambryDateTime::getTimestamp()
returnsFALSE
on systems affected by the Year 2038 bug.To test if your PHP instance is affected, you can quickly execute this code:
If you get a 1970 date, then you are affected.
I suggest to replace theor to use a better way to set the $datetimeplus value.getTimestamp()
withformat('U')
UPDATE: although
DateTime::format('U')
returns the right negative value,$datetimeplus->setTimestamp($date->format('U'))
is still incorrect.Comment #10
mpdonadioIf 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.
Comment #11
tacituseu CreditAttribution: tacituseu commentedFrom: https://3v4l.org/d05V7 (remember to check eol versions)
looking at changelog of 5.2.6 it could be this bug: https://bugs.php.net/bug.php?id=44209
Comment #12
gambry@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.
Comment #13
mpdonadioHere is some additional testing, and a status message.
Comment #15
mpdonadioOK, 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?
Comment #16
gambry#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.Comment #17
gambryAs 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.Comment #18
wturrell CreditAttribution: wturrell commented@mpdonadio I get the same error as @gambry when testing the patch with dates > 2038 on Acquia Dev Desktop (PHP 7.0.4, Mac)
Comment #19
mpdonadioTagging 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.
Comment #20
cilefen CreditAttribution: cilefen commentedTriaging:
Comment #22
cilefen CreditAttribution: cilefen commentedCrediting edna and myself on the triage.
Comment #23
wturrell CreditAttribution: wturrell commentedIssue summary rewritten (could somebody else please proof read it...)
Comment #24
wturrell CreditAttribution: wturrell commentedComment #25
mpdonadioCan 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.
Comment #26
wturrell CreditAttribution: wturrell commentedFor 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.
Comment #27
gambryThe 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()
andDateTime::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.
Comment #28
wturrell CreditAttribution: wturrell commentedMade another pass at the issue summary (and tested strtotime() myself).
Comment #29
mpdonadioI 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.
Comment #30
gambryComment #31
gambryThis 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.
Comment #32
mpdonadioOK, #31 is addressed.
Comment #33
mpdonadioBlerg.
Comment #34
gambryThanks!
Comment #37
catchHere it's 1901.
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?
Comment #38
mpdonadioAre 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().
Comment #41
catchI'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.
Comment #42
mpdonadioI 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.
Comment #43
jhedstromI like this approach.
This would certainly avoid any confusion as to what is happening :)
Comment #44
gambryfrom http://windows.php.net:
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).
Comment #45
alexpottDiscussed 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.
Comment #46
mpdonadioThis needs a re-roll b/c the array mega-issue.
Comment #47
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedComment #48
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedRerolled the patch.
Comment #49
jofitz CreditAttribution: jofitz at ComputerMinds commentedRemoved the Needs Reroll tag.
Comment #50
gambryI 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 ?
Comment #51
wturrell CreditAttribution: wturrell commentedJust created a new community docs page (not yet approved) for this:
https://www.drupal.org/docs/8/system-requirements/limitations-of-32-bit-php
Comment #52
jhedstrom@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.
Comment #53
gambryRemoved 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.
Comment #54
mpdonadioDid a copy/paste from the doc page to a CR.
Comment #55
gambry@mpdonadio I don't see any CR attached to this issue :(
Comment #56
wturrell CreditAttribution: wturrell commentedhttps://www.drupal.org/node/2860500 (thanks btw)
Comment #57
wturrell CreditAttribution: wturrell commented(Added to Issue Summary)
Comment #58
jhedstromThe CR looks good, as does the patch. Thanks all!
Comment #59
yogeshmpawarSo 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".
Comment #60
mpdonadio#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.
Comment #61
yogeshmpawarcoding 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.
Comment #62
gambryYeah, 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:
A comma should follow the last multiline array item. This issue appears on
DateTimePlusTest
several times on the patch.Comment #63
mpdonadioThis 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.
Comment #64
jhedstromPatch in #48 is still RTBC.
Comment #65
wturrell CreditAttribution: wturrell commentedFWIW, the array comma thing will eventually be fixed everywhere by this: #2572613: Fix 'Drupal.Array.Array.CommaLastItem' coding standard
Comment #66
cilefen CreditAttribution: cilefen commentedGood job everyone but I'm not yet ok with the approach.
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.
To me, that's an untenable situation with no workaround on affected platforms. I am posing the question: Should this be REQUIREMENT_ERROR?
Is it just me or should we not make this conditional on PHP_INT_SIZE and instead fail on 32-bit systems?
Comment #67
gambryThe 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.
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).
Comment #68
gambry@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.
Comment #69
gambryAdding the link to the 2038 bug wikipedia page + fixing coding standard errors from #48 tests results.
Comment #70
cilefen CreditAttribution: cilefen commentedHow 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.
Comment #71
wturrell CreditAttribution: wturrell commentedCould 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
Comment #72
gambry#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?
Comment #73
cilefen CreditAttribution: cilefen commentedYeah I like this better. Can we make the anchor text "Read about the limitations of 32-bit PHP."?
Comment #74
gambryDone.
Comment #75
gambryWondering if this one needs backport to D7? Maybe just the system_requirements()?
In the same note, due #42:
Do we need a follow-up targeting D9 to review/discuss/address this issue?
Comment #76
jhedstromI think #74 has addressed the remaining feedback.
That seems like a good idea to eliminate in Drupal 9.
Comment #77
cilefen CreditAttribution: cilefen commentedI 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.
Comment #78
xjmTheoretically, maybe we should do less than 5? There could be problems I haven't considered with this suggestion.
This should be sentence-case, not title-case. ("Date" and "Range" should be lowercase instead.)
We also should be using
:placeholder
here instead of@placeholder
, anddrupal-doc
is not the best name. Do hyphens even work? Did someone manually test this warning (maybe by hacking toif (TRUE)
or such)?I don't know how to go about getting the doc page approved. New handbook policy makes me sad.
Comment #79
wturrell CreditAttribution: wturrell commented@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.
Comment #80
gambry#78.1 : I don't think
PHP_INT_SIZE < 5
is semantically correct as that will readif 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 changedPHP_INT_SIZE === 4
instances withPHP_INT_SIZE <= 4
andPHP_INT_SIZE !== 4
withPHP_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.Comment #81
gambryAs a separate note:
Apparently hyphens on placeholders work! Below a screenshot of Status Report page with #74 applied.
Comment #82
xjmThanks @gambry!
Comment #83
yogeshmpawarAny Update on this issue ?
Comment #84
cilefen CreditAttribution: cilefen commentedIt needs a review done.
Comment #85
gambryAnd also finalising and publishing the documentation page .
Comment #86
jhedstromI think the remaining issues have been addressed. Thanks everybody!
Comment #87
cilefen CreditAttribution: cilefen commentedHas the documentation page been finalized an formally published?
Comment #88
mpdonadio#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
I don't know who can do this (I can't), and I don't see a docs maintainer anymore in MAINTAINERS.txt
Comment #89
Wim LeersThis 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.)
Comment #90
gambryI 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)
Comment #91
gambryMay 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."
Comment #92
catchhttps://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!
Comment #94
gambryI'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.
Comment #95
xjm@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.
Comment #96
xjmAlso 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. :)
Comment #97
gambryThanks @xjm, didn't know we now do it in a separated issue.
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
Comment #98
gambryThe 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.
Comment #100
xjmSorry, failed pushing the revert last night; did so now.
Nice research @gambry.
Comment #101
xjmWhat 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?
Comment #102
mpdonadioThis 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...Comment #103
gambry@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:
Comment #104
mpdonadioI 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.
Comment #105
catch#103 and #104 both sound good to me.
Comment #106
mpdonadioAssigning 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
Comment #107
mpdonadioAnd this is truly odd, https://3v4l.org/lo0Ya
Comment #108
mpdonadioOK, 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
?
Comment #109
gambryI 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.Why not linking to #108, which has the 3v4l.org link together with additional details?
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.
Comment #110
mpdonadioComment #111
gambryThanks a lot!
Comment #113
cilefen CreditAttribution: cilefen commentedCommitted 9539eaf and pushed to 8.4.x. Thanks!
Comment #115
selinav CreditAttribution: selinav commentedI can't upgrade my site on OVH share hosted.
What should I do to overstep this control ?
Comment #116
cilefen CreditAttribution: cilefen commented@selinav Kindly open a new issue with the question.
Comment #117
selinav CreditAttribution: selinav commentedI'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...
Comment #118
cilefen CreditAttribution: cilefen commentedAs 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
Comment #119
selinav CreditAttribution: selinav commentedI 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
Comment #120
Dave52521 CreditAttribution: Dave52521 commentedI 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
Comment #121
Grayle CreditAttribution: Grayle at Dropsolid commentedIsn'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.
Comment #122
gravisrs CreditAttribution: gravisrs commentedRegarding #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']