Posted by Dave Reid on October 24, 2008 at 9:33pm
30 followers
| Project: | Drupal core |
| Version: | 6.22 |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | active |
Issue Summary
I got this error while deleting/editing polls/nodes:Strict warning: strtotime(): It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'America/Chicago' for 'CDT/-5.0/DST' instead in node_validate() (line 846 of /home/davereid/Projects/drupal-head/modules/node/node.module).
Offending lines of code in node.module:
<?php
function node_validate($node, $form = array()) {
...
// Validate the "authored on" field. As of PHP 5.1.0, strtotime returns FALSE instead of -1 upon failure.
if (!empty($node->date) && strtotime($node->date) <= 0) {
form_set_error('date', t('You have to specify a valid date.'));
}
...
}
?>
Comments
#1
Here's at least a start to changing the return value check to
strtotime($node->date) === FALSEsince we need PHP 5.2 now. Figuring out how to fix the warning. I almost wish that we did something with using Drupal's timezone with date_default_timezone_set() in _drupal_bootstrap_full() so all these strict warnings can be avoided...#2
Setting to code needs work...
#3
There seems to be no timezone-agnostic replacement for strtotime(). So the solution is to set a default timezone as early as possible.
#4
Maybe this should do just date_default_timezone_set(date_default_timezone_get())?
This needs re-roll since #279516: Remove workarounds for PHP versions less than 5.2.x was committed before this.
#5
Also changing title since this has narrowed.
#6
With the new timezone settings, how is the best way to call date_default_timezone_set()?
#7
something like
<?phpdate_default_timezone_set(variable_get('date_default_timezone', date_default_timezone_get()));
?>
could now be used
#8
This is definately needed since the system update for updating user's timezones as a part of #11077: Introduce Daylight Saving Time for Drupal cause a bunch of PHP strict notices:
Strict warning: date_create(): It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'America/Chicago' for 'CST/-6.0/no DST' instead in format_date() (line 1385 of /home/davereid/Projects/drupal-head/includes/common.inc).
#9
I'm pretty sure that drupal_initialize_variables() is called too early for the variable_get() in
date_default_timezone_set(variable_get('date_default_timezone', date_default_timezone_get()));to actually get the variable. Can date_default_timezone_set() be called later in bootstrap?#10
Oops, I thought you had tested your suggestion in #4. date_default_timezone_get() also throws strict notices, so it should just be 'UTC' as a default.
#11
The patch from #3 no longer applies cleanly to HEAD, namely the portion of the patch for node.
The patch for bootstrap.inc to use 'UTC' does apply with offset, and does eliminate the strict warnings regarding strtotime() in my testing.
#12
Revised patch to use date_default_timezone_set('America/Chicago') instead of 'UTC'. By the way, I have attached a screenshot of the user edit page when strict errors are enabled... it's a big big big explosion.
#13
#14
Can't we have a one-line patch, just calling
date_default_timezone_set(variable_get('date_default_timezone', 'UTC'));at some point in bootstrap? Also, UTC is the default in previous versions of Drupal and also in PHP itself, so not sure why you're using America/Chicago.#15
Ah, I didn't see that 'UTC' was a valid choice in the timezone select option. I had to add an error-ignore to the calls to date_default_timezone_get since they can and will generate PHP strict notices if the date.timezone PHP setting is not set.
#16
I'm having a hard time testing this because I'm also seeing other strict warnings like
Strict Standards: Declaration of DatabaseConnection_sqlite::prepare() should be compatible with that of PDO::prepare() in /home/d7strict/includes/database/sqlite/database.inc on line 165Fatal error: Declaration of DatabaseStatementPrefetch::execute() must be compatible with that of DatabaseStatementInterface::execute() in /home/d7strict/includes/database/prefetch.inc on line 23Presumably I'm not the only person seeing this? maybe we could fix all the other strict warnings in this issue? I think most of them have to do with not providing default values when declaring methods, i.e.
public function execute($args, $options) {should bepublic function execute($args = array(), $options = array()) {#17
That looks like a strict problem the the new sqlite driver. If we were to fix all strict issues in one patch, it would be a kitten-killer. Separate issues should be filed so they can be easily reviewed.
#18
I was already seeing other PDO-related strict issues, sqlite is just the latest. personally I don't think it would be too difficult to fix all the strict issues in one patch, but up to you. i'm attaching what I found so far in case you want to incorporate into this or another issue.
#19
The last submitted patch failed testing.
#20
OK this fixes all the strict issues I found so far. Also, I did not need to add an error-ignore to the calls to date_default_timezone_get() because drupal should always be fully bootstrapped before this function is called.
#21
I thought some more about this. With this patch it's rather pointless to call date_default_timezone_get() in various places as a default time zone since this will now always be 'UTC' if it hasn't been otherwise configured in drupal. So if we care about getting the server environment's time zone we should call
date_default_timezone_set(variable_get('date_default_timezone', @date_default_timezone_get()))in bootstrap.#22
The last submitted patch failed testing.
#23
Type-hinting/strict issue has been moved #318016: Type hint array parameters. Revised patch.
#24
I think my last patch was the right way to do it:
date_default_timezone_set(variable_get('date_default_timezone', @date_default_timezone_get()));This allows the server environment's time zone to be applied as the default time zone rather than ignoring it and using UTC.Also, there should be no reason for the "@" elsewhere on date_default_timezone_get() because it will always have been set during bootstrap.
#25
Here's my argument for #23...the variable 'date_default_timezone' going to be set during the install, which is not at a full bootstrap. We do not want to show the PHP strict error there. I agree we should use date_default_timezone_get() for the DRUPAL_BOOTSTRAP_FULL instead of 'UTC', since it is consistent with the default value for the 'date_default_timezone' variable elsewhere. We still need the '@' since the code will still be called on each full bootstrap, since it is provided as the default value for variable_get(). We should be hiding that PHP strict error.
#26
Install does bootstrap during install. So (last I looked at it) default time zone was getting set before hitting the install settings form.
#27
mfb, you are right! The only change needed is the one in _drupal_bootstrap_full()! I checked the install and confirmed no more strict timezone errors.
#28
#29
Revised comment that actually ends in a period now.
#30
One little problem is update.php going from d6 to d7. In d6, data_default_timezone is just a number not a valid timezone so this causes a PHP notice like "Timezone ID '10800' is invalid". Easy work-around for this would be putting the "@" in front of date_default_timezone_set().
#31
The last submitted patch failed testing.
#32
Testing failure on slave #8.
#33
See #30 re: update.php PHP notice (technically worse than the PHP strict this is fixing)
#34
#35
Shouldn't the default timezone be set to the current user's timezone (if one exists) instead? strtotime() is used e.g. in node_validate() to parse a date entered by the current user. Since all dates are displayed in the user's local timezone, it seems reasonable for him to assume that he can enter dates in his own timezone as well.
#36
This patch sets PHP's default timezone based on the user's timezone (if one exists).
#37
I don't think thats the correct approach. We want all base time operations to be done in the site's timezone format, but any 'display' time operations (like format_time) to be done in the user's timezone. I'd need to look into it though.
#38
The last submitted patch failed testing.
#39
Bitten by the 'file naming' testing bot failure.
#40
Sometimes it makes sense to use the user's time zone, e.g. the strtotime() calls in node and comment modules that c960657 mentioned, but in other cases (mainly in contrib modules) the system time zone is desired. For example, when generating URL path components with the date() function, I usually want the year/month/day to be in the system time zone.
Maybe we could try to figure out which scenario is most common, or if any contrib modules would be really broken by a fluctuating time zone depending on the user. Whichever we make the default time zone, it's not hard to parse or generate dates in any time zone, for example we could make a little utility function to replace strtotime() that uses date_create(), timezone_open(), etc. to work in the user's time zone if user-configured time zones are enabled.
#41
Wait did we agree on making Drupal compliant with E_STRICT?
#42
@chx:
I'm not sure we have reached a conclusion about maing Drupal E_STRICT compliant, but it has been requested in #350543: Use E_STRICT error reporting. This issue is not only about getting rid of the E_STRICT warning, though, but also about making Drupal behave in a consistent and well-defined way independently of the timezone setting in php.ini.
@mfb:
I have no impression of what the common scenario is. format_date() currently uses the user's timezone unless another timezone is explicitly specified. It would improve consistency if PHP's native date functions did this as well. Adding a utility function for parsing dates is probably a good idea (but outside the scope of this issue).
#43
@c960657: Here's the resulting changes in contrib modules that I could think of, that might need some documentation:
Token and Pathauto use date() rather than format_date() to create paths so the path is consistent for all users. A work-around for
date($format, $timestamp)would beformat_date($timestamp, 'custom', $format, variable_get('date_default_timezone', date_default_timezone_get()), 'en').The Views date argument and filter will now be returning different nodes for different users using the same date value. This could be considered a good thing although it could lead to some confusion.
#44
What do you think about this simplified patch. I added a small utility function to avoid code duplication between bootstrap.inc and session.inc.
#45
I think it looks good.
If the D6->D7 upgrade is the only case where date_default_timezone_set() is expected to trigger an error, I wonder whether it is better to drop the @ and instead call @date_default_timezone_set(date_default_timezone_get()) at the top of upgrade.php? This will allow the site admin to discover if the timezone variable has somehow become invalid (e.g. an empty string), or if there is a problem with the timezone database (new list of zones is not completely static).
#46
@c960657: It looks like when you moved the date_default_timezone_set() from common.inc (in a previous version of this patch) into bootstrap.inc, you resolved the issue I was having with update.php. So nevermind the @.
#47
Small nit:
+ * Sets default time zone for date/time functions to the configured time zone.Apart from that this looks RTBC to me.
#48
The last submitted patch failed testing.
#49
Ok, now "Set". Note, bootstrap.inc function documentation could use some clean up in this area.
#50
I'd say this is ready to go in.
#51
Can someone explain to me (and in the code) why we need error-suppression here?
+ $timezone = variable_get('date_default_timezone', @date_default_timezone_get());#52
I added a short code comment.
Long answer is that PHP can fallback to getting the default time zone from several different sources, some of which ("Reading the TZ environment variable (if non empty)" and "Querying the host operating system (if supported and allowed by the OS)") trigger PHP strict notices. We want to ignore those notices because we really do want to get it from those sources if needed, before setting it properly.
We could add extra verbage to the code comment if it's useful...
#53
2 issues:
+ // Ignore PHP strict notice if time zone has not yet been set.+ $timezone = variable_get('date_default_timezone', @date_default_timezone_get());
+ }
+ date_default_timezone_set($timezone);
What's the value of $timezone if no time zone has been set yet?
+ variable_set('date_default_timezone', date_default_timezone_get());Why don't we apply the same error suppression here? (Not that I like error suppression [it should be avoided at all costs], but this is inconsistent.)
#54
#55
Patch works for me, although the line numbers were way off.
#56
#57
Was there any difference besides the line numbers?
#58
Oops, I swear I typed in a comment! too many tabs.. Yes that was just a re-roll to fix the line numbers.
#59
Then I will not reapply it.
#60
Oh, what an horrible function name:
drupal_date_default_timezone_set(). I know that it is named after the corresponding PHP function (date_default_timezone_set()), and this is how we do that elsewhere. But in that case it doesn't seem wise: those two functions have different purpose and different signatures.I suggest we implement drupal_get_user_timezone() as:
<?php/**
* Return the timezone of the current user.
*/
function drupal_get_user_timezone() {
global $user;
if (variable_get('configurable_timezones', 1) && $user->uid && $user->timezone) {
$timezone = $user->timezone;
}
else {
// Ignore PHP strict notice if time zone has not yet been set *in the php.ini configuration*.
$timezone = variable_get('date_default_timezone', @date_default_timezone_get());
}
return $timezone;
}
?>
(notice the clarification on the comment)
and that we do:
<?phpdate_default_timezone_set(drupal_get_user_timezone());
?>
in _drupal_bootstrap().
#61
OK done.
#62
The last submitted patch failed testing.
#63
Reroll.
#64
The last submitted patch failed testing.
#65
reroll.
#66
The last submitted patch failed testing.
#67
Cannot reproduce this failure.
#68
The last submitted patch failed testing.
#69
Chasing HEAD.
#70
The last submitted patch failed testing.
#71
Fixed a potential test failure.
#72
The last submitted patch failed testing.
#73
Chasing HEAD.
#74
The last submitted patch failed testing.
#75
subscribe.
I suddenly get these timezone warnings now that I have PHP 5.3
#76
Chasing HEAD
#77
The last submitted patch failed testing.
#78
Fixed install ($user does not exist during install)
#79
The last submitted patch failed testing.
#80
Argh, appears that bootstrap order changed.
#81
Just noticed I included some debug code in #80
#82
This needs more reviews but I merged it in to #348448: Always report E_STRICT errors so let's review it over there.
#83
Seems like this issue should remain open - it's its own separate bug and there's plenty of discussion here about how to fix it.
Also, I'm tentatively upping this to "normal" because as far as I understand from #668496: Timezone error on D7 install (which I am marking duplicate of this), in the case of PHP 5.3 these warnings show up even when not in strict mode?
#84
webchick suggested that this patch be merged into #348448: Always report E_STRICT errors, see http://drupal.org/node/348448#comment-2139926 so that is where it has been maintained. The last patch here no longer applies due to some bootstrap refactoring.
#87
I face the same issue at Drupal 7.0a1 installation, "Configure site" step (with IIS 5.1 and PHP 5.3.1)
This issue is discussed also at #615438: Date warnings, though the suggested temporary solution as
<?php$php_version = phpversion();
if (!@date_default_timezone_get() and $php_version >= '5.3.0') {
date_default_timezone_set('UTC');
}
?>
is not limited to 5.3.0 (my case 5.3.1).
#88
This patch was merged into #348448: Always report E_STRICT errors so setting to "duplicate" until someone convinces me to split it back out or does it themselves :)
#89
#81: tz.patch queued for re-testing.
#90
As I said in #84 this patch no longer applies, but fine, don't believe me :p
#91
The last submitted patch, tz.patch, failed testing.
#92
Fixed the problem (in my case) in line 1814 that generated warnings.
#93
@Deslack this doesn't work, because PHP thinks that a "@" timestamp already has a time zone of UTC and ignores any time zone you pass in to date_create(). Example code:
<?phpprint date_format(date_create('@' . time(), timezone_open('Indian/Antananarivo')), 'r') . "\n";
print date_format(date_create('@' . time(), timezone_open('America/Los_Angeles')), 'r') . "\n";
?>
See #84 for link to working version of this patch in #348448: Always report E_STRICT errors
#94
Hi, i am very new to drupal and have just installed 6.1.16, and notice that this issue is also occurring every time i edit a page.
it was first raised in 2008, and yet is still happening, and all the patches seem to be marked as failed.
do you know when this will be fixed? I seem to have seen someone showing me a drupal installation which got me interested in the first place that didn't have this issue.
Thanks
#95
@wcndave: There should be some easy work-around in .htaccess or settings.php: setting a default time zone, changing the error reporting level, etc. Try adding this to your .htaccess (or Apache configuration) file:
php_value date.timezone Europe/London#96
adding to htaccess work perfectly, thank you!
#97
FYI you can also set it to
UTC(for GMT time) or any other valid time zone.Europe/Londonwill use daylight saving/summer time.#98
I think that the best is just write in the php.ini the date.timezone. I wouldn't write anything in the bootstrap if it isn't strictly necessary.
Example:
date.timezone = Europe/Andorra
It usually is commented with a ';' so you should just remove the semicolon.
#99
#80: tz.patch queued for re-testing.
#100
The last submitted patch, tz.patch, failed testing.
#101
#348448: Always report E_STRICT errors
#102
Not sure if this is still relevant but there's a global fix for this here: http://groups.drupal.org/node/17970#comment-179313
From: uberhacker
#105
@studiotwelve: Thanks! That worked for me putting it into sites/default/settings.php, and in one line cleared not only several pages of these errors, but also a mysterious MySQL error while setting up Date and Calendar modules.
#106
#102 Worked perfectly!! Thank you studiowelve and uberhacker!
#107
Ditto, @studiotwelve @uberhacker. Thanks =)
#108
#102 Worked !!. I'm using Drupal 6.22 and this solution worked for me. Thanks!!!
#109
#102 worked for me under PHP 5.3.8 and Drupal 6.22
#110
This was marked as a duplicate of #348448: Always report E_STRICT errors and as that issue is for drupal 8 it was patched in drupal 8 and backported to drupal 7.
The error is still present in drupal 6 as no solution to this seems to have been backported to drupal 6 though there have been some new releases of drupal 6 since this issue started. Isn't there a way to solve this in core on drupal 6 or the best solution is to just use the ini_set config in settings.php (#102) for people that wants to run drupal 6 in PHP 5.2+?
The problem is there in drupal 6.22 node module. Unless solved in dev version the problem persists.
#111
#112
Just wanted to report that, when the central timezone hit daylight savings time, the use of the tip in #102 started producing tons of errors in my logs (as the reported timezone was no longer 'America/Chicago', but 'CST/-6.0/no DST'. Changing the value to 'America/Chicago' explicitly fixed the errors for me. Annoying, but I'm happy to not have a huge amount of logs showing that particular error :)
More on this: http://www.midwesternmac.com/blogs/jeff-geerling/drupal-6x-and-php-53x-date
#113
#102 Worked perfectly!! Thank you studiowelve and uberhacker!
#114
#102 Worked perfectly for me too))) Thank you studiowelve and uberhacker!
#115
warning: strtotime(): It is not safe to rely on the system's timezone settings. You are*required* to use the date.timezone setting or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We
selected 'America/New_York' for 'EST/-5.0/no DST'
SImple way to remove this type error just go with it http://drupalindia.net/warning-strtotime-it-not-safe-rely-systems-timezo...
More on this: http://drupalindia.net/warning-strtotime-it-not-safe-rely-systems-timezo...