Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
This issue was originally created to address a change in PHP 5.4 where $_SERVER['REQUEST_TIME'] was changed from an int to a float. This change was backed out (can't find definitive reference for this), and REQUEST_TIME_FLOAT was introduced instead. This means that this patch is not needed anymore.
Proposed resolution
- Revert this patch.
- Remove explicits casts of REQUEST_TIME to int not using the define()
Remaining tasks
Revert this patch.
User interface changes
None.
API changes
None.
Data model changes
None.
Original Issue
Starting with PHP 5.4.0, $_SERVER['REQUEST_TIME'] is a float with microseconds. This change has probably not be considered carefully enough, because most of the rest of PHP gets really confused by float timestamps:
$ php5 -r 'echo $_SERVER["REQUEST_TIME"]; var_export(date_create("@" . $_SERVER["REQUEST_TIME"]));'
1309862915.4905
DateTime::__set_state(array(
'date' => '4946-07-05 10:48:35',
'timezone_type' => 1,
'timezone' => '+00:00',
))
I suggest we just cast our own REQUEST_TIME constant to (int).
| Comment | File | Size | Author |
|---|---|---|---|
| #32 | request_time_is_a_float-1209470-32.patch | 12.99 KB | Munavijayalakshmi |
| #21 | interdiff-15-21.txt | 12.71 KB | mpdonadio |
| #21 | request_time_is_a_float-1209470-21.patch | 13.42 KB | mpdonadio |
| #15 | Request-time-int-1209470-15.patch | 728 bytes | anchal29 |
| #1 | 1209470-request-time-float.patch | 862 bytes | Damien Tournoud |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #2
sunLooks good.
The cast to integer is safe here, since we only have 4 decimals:
Comment #3
Dries CreditAttribution: Dries commentedHow weird to provide that information as a float instead of, say, microseconds. What were they smoking?
Committed to 8.x. Thanks DamZ.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #5
sunComment #6
sun#1: 1209470-request-time-float.patch queued for re-testing.
Comment #7
webchickWow, what are they smoking is right. ;P
Committed and pushed to 7.x. Thanks!
Comment #9
rjbrown99 CreditAttribution: rjbrown99 commentedNot to kick a dying dog, but if the D8 release is scheduled for fall 2013 that means that Drupal 6 will be supported until that time as well. Since we are already at PHP 5.4.0 release candidate 8, I am assuming that a final 5.4 release will be out long before that time. I am therefore concluding that Drupal 6 and PHP 5.4 will coexist as supported during the same time period.
Given that thought, does this need a backport? And, if the answer is yes, do we also need to backport #305645: DX: replace $_SERVER['REQUEST_TIME'] with a define which creates the constant REQUEST_TIME and #302763: Remove time() so it's easier to fix? This could also be an issue in contrib if people are using $_SERVER["REQUEST_TIME"].
If the answer is no feel free to re-close this and tell me I'm off-base. If you would like me to attempt to roll these three issues together into a 6.x patch, I'd also be happy to do that.
Comment #10
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe PHP core team identified that this was an unwelcome change and decided to make REQUEST_TIME a int again. We can remove the cast from 8.x and 7.x.
Comment #11
sun@Damien: A huge THANKS to you for tracking changes and getting involved upstream in the PHP project!
--
For D8, I believe this also means that we can change the
defineinto aconst.Comment #11.0
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #12
jhedstromNeeds an issue summary update since the issue was fixed, but this remains open to revert the change as explained in #10.
Comment #13
mpdonadioComment #14
dawehnerGood to know, this basically allows us to get rid of a couple of annoying
(int)bitsComment #15
anchal29 CreditAttribution: anchal29 as a volunteer commentedReverted the patch.
Comment #16
jibranComment #18
mpdonadioThat's weird, https://www.drupal.org/pift-ci-job/147014 shows nothing but passes. Triggering manual retest before I reset RTBC.
Comment #19
mpdonadioRetest of #15 is green. Back to RTBC.
Comment #20
mpdonadioActually, I am setting this back to NW to address some other casts that crept in.
Comment #21
mpdonadioDid some greps, and think this catches all uses of the int cast where the REQUEST_TIME is grabbed from $_SERVER directly, or via the request.
Comment #22
gnugetThis looks good to me, I think we can move this to RTBC
Thanks.
Comment #30
gnugetI wonder if we should now use the Time service added on Drupal 8.3.x instead to cast to INT the REQUEST_TIME constant.
Comment #31
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedComment #32
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedRerolled the patch.
Comment #33
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedComment #34
agomezmoron CreditAttribution: agomezmoron at La Drupalera by Emergya commentedComment #35
agomezmoron CreditAttribution: agomezmoron at La Drupalera by Emergya commentedIt worked for me but I queued an extra test for PHP 7 - there are some many users with PHP updated and we need to ensure it will not break anything
Comment #36
agomezmoron CreditAttribution: agomezmoron at La Drupalera by Emergya commentedAfter seeing with PHP7 works and it is not broken, so it is fine from my side.
Comment #37
mpdonadioThis needs to be against 8.4.x, and it doesn't apply. I also don't think this qualifies as a bug anymore.
Comment #38
mpdonadioClosing this to avoid the multiple-patch-on-same-issue-thing. New work should be on #2862477: Remove casts from REQUEST_TIME. I will get issue credits transferred over.