Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:4082
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Mon, 18 Aug 2003 19:24:48 -0500
To: internals@lists.php.net
Message-ID: <20030819002448.GA24390@john.css.tayloru.edu>
References: <20030818215556.GB23568@john.css.tayloru.edu>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20030818215556.GB23568@john.css.tayloru.edu>
User-Agent: Mutt/1.3.99i
Subject: Re: [PHP-DEV] Resolved gmmktime() bug (patch enclosed)
From: lehresma@css.tayloru.edu (Luke Ehresman)

Thanks,  I added the comment, and the 4.3.2 patch to the bug report.
Luke


On Mon, Aug 18, 2003 at 04:55:56PM -0500, Luke Ehresman wrote:
> Hi,
> 
> A co-worker and I have been working on some intensive timestamp/timezone
> manipulation code for the past week or so, and ran across what we believed
> to be a bug in the way PHP handled gmmktime().  Browsing through the code,
> sure enough I found the source of the problem.  I fixed it, and have
> attached a patch.
> 
> As I was searching around, I discovered that this has been an open bug
> since 4.0.6 (2 years!).  I believe that this patch should fix most of the
> issues related to the following problem:
>     http://bugs.php.net/bug.php?id=14391
> 
> 
> The Problem:
> ============
> The function gmmktime() is supposed to take a date in GMT as a paramter
> and return a timestamp in GMT.  No manipulation of the time should occur.
> The only circumstance in which manipulation of the time should happen is
> if the seventh parameter (is_dst) is set to 1.  This is the expected
> behavior.  However, in reality I noticed that when I didn't specify a
> seventh parameter, it was applying a daylight savings time offset when I
> tried to get the epoch.  See the example code.
> 
> <?php
> 
>     // Get a timestamp for the epoch (jan 1, 1970)
>     echo "<pre>";
>     echo gmmktime(0,0,0,1,1,1970,1)."\n";
>     echo gmmktime(0,0,0,1,1,1970,0)."\n";
>     echo gmmktime(0,0,0,1,1,1970)."\n";
> 
>     // Now get timestamp for June 1
>     echo "\n";
>     echo gmmktime(0,0,0,6,1,1970,1)."\n";
>     echo gmmktime(0,0,0,6,1,1970,0)."\n";
>     echo gmmktime(0,0,0,6,1,1970)."\n";
> 
> ?>
> 
> I expected the first set of outputs to look like this:
> 
>     -3600
>     0
>     0
> 
> Which it does, so this is fine.  However, The second block of outputs is
> in June which if I had been using simply mktime(), I would have expected a
> DST offset (note: DST = daylight savings time) to be applied by default
> (on the 3rd command).  However, since I'm using gmmktime() by default no
> DST offset should be applied.  The odd behavior manifests itself in the
> second block of output:
> 
>     13046400
>     13050000
>     13046400
> 
> This is pretty meaningless until you realize what is actually going on.
> The last output, which is generated by a gmmktime() that has no 7th
> parameter, is applying an offset for daylight savings time.
> 
> The Solution
> ============
> I had a moment of panic and was sincerely hoping that GMT did not actually
> use daylight savings time, so I did some research and found that while the
> timezone that contains Greenwich actually does use daylight savings time,
> the GMT standard time reference does not.  So right there I knew something
> was wrong on the PHP side.  I loaded up the source of PHP (like a good
> open source enthusiast), and sure enough, I discovered the source of the
> problem (no pun intended).
> 
> PHP is using the C function php_mktime() for both mktime() and gmmktime().
> There is a flag as a parameter called "gm" which is 1 or 0 depending on
> whether it was called from mktime() or gmmktime().  The problem is that
> there is no check before applying the server's daylight savings
> information to the timestamp.
> 
> As a fix for this, I changed the behavior so that by default if there is
> no 7th parameter for gmmktime, it will not apply any DST offset.  This
> differs from mktime which by default will appy the server's DST offset.
> 
> I have attached patches for both PHP php-5.0.0b1 and php-4.3.2.  I would
> imagine the same code would work on most recent versions of PHP as it
> doesn't seem that this function has had much recent development.  The
> patch can be applied by using the following:
> 
>     $ cd /usr/local/php-4.3.2
>     $ patch -p2 < ~/gmmktime-php_4.3.2.patch
> 
> Feel free to contact me if you have any questions.
> 
> Luke
> -- 
> Luke Ehresman
> luke[at]ehresman.org
> http://www.luke.ehresman.org

> diff -u clean/php-4.3.2/ext/standard/datetime.c php-4.3.2/ext/standard/datetime.c
> --- clean/php-4.3.2/ext/standard/datetime.c	2003-05-04 07:22:00.000000000 -0400
> +++ php-4.3.2/ext/standard/datetime.c	2003-08-18 17:18:19.000000000 -0400
> @@ -116,7 +116,20 @@
>  	/* Let DST be unknown. mktime() should compute the right value
>  	** and behave correctly. Unless the user overrides this.
>  	*/
> -	ta->tm_isdst = -1;
> +	/*
> +	**  If we are using gmmktime(), do not use the local
> +	**  server's setting of DST.  GMT does not use daylight
> +	**  savings, so unless the user overrides us, this should
> +	**  be set to 0.
> +	**  - Luke Ehresman, Aug 2003, <lehresma@css.tayloru.edu>
> +	*/
> +	if (gm) {
> +		ta->tm_isdst = 0;
> +		is_dst = 0;
> +	} else {
> +		ta->tm_isdst = -1;
> +		is_dst = -1;
> +	}
>  
>  	/*
>  	** Now change date values with supplied parameters.
> Common subdirectories: clean/php-4.3.2/ext/standard/tests and php-4.3.2/ext/standard/tests

> --- php-5.0.0b1/ext/standard/bak	2003-08-18 17:30:31.000000000 -0400
> +++ php-5.0.0b1/ext/standard/datetime.c	2003-08-18 17:30:48.000000000 -0400
> @@ -112,7 +112,20 @@
>  	/* Let DST be unknown. mktime() should compute the right value
>  	** and behave correctly. Unless the user overrides this.
>  	*/
> -	ta->tm_isdst = -1;
> +	/*
> +	**  If we are using gmmktime(), do not use the local
> +	**  server's setting of DST.  GMT does not use daylight
> +	**  savings, so unless the user overrides us, this should
> +	**  be set to 0.
> +	**  - Luke Ehresman, Aug 2003, <lehresma@css.tayloru.edu>
> +	*/
> +	if (gm) {
> +		ta->tm_isdst = 0;
> +		is_dst = 0;
> +	} else {
> +		ta->tm_isdst = -1;
> +		is_dst = -1;
> +	}
>  
>  	/*
>  	** Now change date values with supplied parameters.
> 

> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

-- 
Luke Ehresman
luke[at]ehresman.org
http://www.luke.ehresman.org