65671 - JS (SpiderMonkey) Date() timezone problems on UNIX platforms

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[rob]

SM gets timezone wrong if local time offset at UNIX epoch
was not the same as it is for the date being manipulated

This manifests itself in Europe/London which changed its timezone
to GMT+0100 as an experiment with year round daylight savings over 
the winter of 69/70. Modern unices with zic(8) know about this and
report localtime as UTC+0100 with no daylight savings for date around
1/1/1970. Everything becomes sane again in 1971 when the timezone
becomes GMT+0000 with +0100 of daylight savings time in summer.


Reproducible: Always
Steps to Reproduce:
1. Take a Linux/FreeBSD/OpenBSD system
2. Set timezone to Europe/London
3. create a new Date, and call toString() on it
4. observe date 

Actual Results:  var d = new Date(2001,01,17,00,13,09);
document.write(d.toString());
generates: "Thu Jan 18 00:13:09 GMT+2400 (BST) 2001"


Problem is in PRMJ_LocalGMTDifference() in prmjtime.c which gets
TZ offset by comparing localtime at 2/1/70 00:00:00 with 24*3600

This should get the offset by comparing localtime with UTC time for
target date instead as timezone isn't necessarily constant over time.

Comment 1

[rob]

Attachment: Possible fixes to jsdate.c & prmjtime.[ch]

Comment 2

[Keyser Sose]

Adding Patch keyword.

Comment 3

[Phil Schwartau]

Assigning to mccabe for date expertise - is this related to bug 63720 ?


cc'ing Patrick -

Comment 4

[rob]

I pasted the wrong example code into the original description,
the actual code which shows the problem its results are:

var d = new Date(Date.UTC(2001,0,17,0,13,9))
d.toString()
-
Thu Jan 18 00:13:09 GMT+2400 (BST) 2001

Comment 5

[Phil Schwartau]

Reassigning Mike's JS Engine bugs to Patrick, as per recent meeting - 

Comment 6

[Patrick C. Beard]

Marking future.

Comment 7

[Phil Schwartau]

*** Bug 71605 has been marked as a duplicate of this bug. ***

Comment 8

[Phil Schwartau]

Adding reporter and cc list from duplicate bug 71605 - 

Comment 9

[Gilles Durys]

*** Bug 112130 has been marked as a duplicate of this bug. ***

Comment 10

[Phil Schwartau]

*** Bug 114754 has been marked as a duplicate of this bug. ***

Comment 11

[Phil Schwartau]

*** Bug 117257 has been marked as a duplicate of this bug. ***

Comment 12

[Phil Schwartau]

*** Bug 118690 has been marked as a duplicate of this bug. ***

Comment 13

[Phil Schwartau]

*** Bug 123153 has been marked as a duplicate of this bug. ***

Comment 14

[Patrick C. Beard]

I can't reproduce this on Mac OS X, running the mach-o build, which also 
uses UNIX time functions. I set the Mac's time zone to GMT (London) and 
couldn't see a problem with the format of the date:

js> var d = new Date(Date.UTC(2001,0,17,0,13,9));
js> d 
Wed Jan 17 2001 00:13:09 GMT+0000

How does one change the timezone on a Linux system?

Comment 15

[Phil Schwartau]

> How does one change the timezone on a Linux system?

cc'ing rginda. The only commands I know about are |date| and |hwclock|,
but I'm not sure if these are appropriate. I did find these links
regarding Debian Linux, but I'm not sure this applies to Red Hat:

http://www.debian.org/doc/manuals/system-administrator/ch-sysadmin-time.html
http://www.fifi.org/cgi-bin/man2html/usr/share/man/man1/tzconfig.1.gz

Comment 16

[Robert Ginda]

Timezone is set in some config file who's name I have forgotten.

Comment 17

[Asko Tontti]

The file is /etc/localtime and it is symbolic link or copy of the timezone file
from /usr/share/zoneinfo. Different Linux distributions and Unix flavors offer
their own configuration tools to change it. On RedHat 7.2 you can start the
configuration tool from GNOME menu: Programs - System - Date/Time Properties
or running "dateconfig" from command line.

On Linux (and generally on Unix) you can temporarily and locally change
timezone with commands:

  % date
  Sat Feb  9 11:10:12 EET 2002
  % TZ=Europe/London; export TZ
  % date
  Sat Feb  9 09:10:31 GMT 2002
  % mozilla

I am still seeing this bug with RedHat Linux 7.2 and Mozilla 2002020809.
Tested with page http://www.livejournal.com/update.bml (from bug 118690).

Comment 18

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 127246 has been marked as a duplicate of this bug. ***

Comment 19

[Andreas Höfler]

I'm having problems with the time on Win2000 and not with the timezone-field but
with the time itself, so I don't know if Bug 127246 is really a duplicate of
this bug...

Comment 20

[Richard Lloyd]

Just confirming that this problem is present on HP-UX 11.00 (Mozilla 0.9.8) -
it's even present on Netscape 4.79 (Linux and HP-UX 11.00) too !

One high profile site using it is the official soccer site for Liverpool
Football Club at http://www.liverpoolfc.tv/ (skip over any splash page and
the date [wrong if you're in the UK timezone] is present in the Announcements 
box).

What's disappointing about this bug is:
a) it hits UK UNIX users hard (OK, that's probably just me and Alan Cox :-) ).
b) other browsers (Opera, IE, Konqueror) don't have this bug.
c) other Mozilla-based browsers (Galeon, Netscape 6.2.1) have this bug.
d) this bug has no Priority setting at all.
e) its Severity is normal, whereas I'd argue it's a pretty major one for the
   UNIX platform.
f) There is no target milestone - I'll be quite disappointed if Mozilla 1.0
   ships with a completely broken Date() JavaScript implementation for UK
   UNIX users.
g) A patch was submitted over a year ago and hasn't even had a review, let
   alone a super-review.

Comment 21

[Jim Dunn]

Not sure if the attachment (id=22707)
is good.  You have a parameter discrepency that
will cause compilers to complain

Line 505 you call 
"base = PRMJ_ToExtendedTime(tsecs);" 
where tsecs is of type  JSInt64 whereas it is defined on line 229 as 
"static JSInt64 PRMJ_ToExtendedTime(JSInt32 base_time)"

So you are trying to pass a JSInt64 to a call that only accepts a JSInt32
PLEASE do not check this patch in as is, you will break (at least) the
HP-UX compilers.  Thanks!  Jim

Comment 22

[John Collins]

I'd really like to add my name to comment 20 as a UK person who keeps getting
hit by this bug. I reported an error to a website owner before I realised it was
here.

Surely it is more important to get today's date and time right than worry about
experiments with daylight time 30-odd years ago?

Please can the severity of this bug be raised - thanks.

Comment 23

[Phil Schwartau]

Raising severity to major; priority to P1. We should fix this bug ASAP;
it is hitting many users, as evidenced by the number of duplicate bugs. 

> f) There is no target milestone - I'll be quite disappointed if Mozilla 1.0
>    ships with a completely broken Date() JavaScript implementation for UK
>    UNIX users.

I agree entirely, this should be fixed for Mozilla 1.0 !

Comment 24

[rob]

Attachment: Revised patch

As noted by [e-mail], the patch that I submitted with the original
bug report had some problems. 
I've been back to the code base we are currently using and re-generated the
patch, and I think this one is more correct. It is a patch against RC3 which
was months ago though.

Code with this patch applied has been running for about 10 months within an
embedded application of SpiderMonkey (Whitebeam) on Linux/FreeBSD/OpenBSD BUT I
have never built a full Mozilla with it, nor have I built it on any other
platforms so it is a good idea to treat the patch with a degree of caution.

Comment 25

[Phil Schwartau]

cc'ing some more reviewers for this patch -

Comment 26

[rogerl (gone)]

I applied the patch (on Win2k) and crash for:

js>new Date(-1000)

In PRMJ_LocalGMTDifference (prmjtime.c; line 235), the -1000 is getting passed
to gmtime() which returns NULL.

Comment 27

[serge (gone)]

gmtime() can return NULL according to msdn:
"The gmtime, mktime, and localtime functions use the same single, statically 
allocated structure to hold their results. Each call to one of these functions 
destroys the result of any previous call. If timer represents a date before 
midnight, January 1, 1970, gmtime returns NULL. There is no error return."

Comment 28

[serge (gone)]

simple bulletproofing fixes crash on window (there is no crash on linux)
localtime = gmtime(&local);
if (localtime)
      return mktime(localtime) - local;
else
      return 0;
---
but on my w2k without patch I'm getting:
js> Today = new Date();
Thu Mar 14 2002 13:48:49 GMT-0800 (Pacific Standard Time)
which is correct
with patch
js> Today = new Date();
Thu Mar 14 2002 21:50:55 GMT-0800 (Pacific Standard Time)
which is NOT correct

Comment 29

[Phil Schwartau]

Sounds like bug 127246,  "Date() gives wrong timezone on Win2K"

Comment 30

[Phil Schwartau]

Oops - ignore my last comment; I misread your data. Looks like
you're getting the right timezone, but the GMT diff is not being
applied to the time as it should be...

Comment 31

[Phil Schwartau]

AFAICT, the latest patch needs to be corrected to avoid crashing on 
bad input (Comment #28); and needs to handle Win2K correctly.

Is that where we stand now?

Comment 32

[Phil Schwartau]

cc'ing Mike -

Comment 33

[Kenton Hanson (gone)]

Attachment: Proposed patch

Proposed patch for Unix date problem.  Please test!

=Kenton=

Comment 34

[Kenton Hanson (gone)]

The problem appears to be associated with the GMT time offset + the daylight
savings time being greater or equal to 24 hours, i.e., LocalTZA +
DaylightSavingTA(t).  Rather than muck with prmjtime.c which is apparently well
behaved, I simply adjusted jsdate.c to prevent the GMT offsets from equaling or
exceeding 24 hours.  I don?t believe the LocalTZA need be dependent on date.

We must fix the Official Liverpool Soccer site for Unix hackers!

Comment 35

[serge (gone)]

Comment on attachment 79337 [details] [diff] [review]
Proposed patch

linux & w2k result of 
js> new Date(Date.UTC(2001,0,17,0,13,9))
Wed Jan 17 2001 00:13:09 GMT+0000 (BST)
---
looks correct, r=serge

Comment 36

[Kenton Hanson (gone)]

Mike Shaver or Brendan, can I get a sr= for this patch.  I'd like to get it into 1.0

Thanks

Kenton

Comment 37

[Mike McCabe]

r=mccabe

Any chance of s/time_Adj/LocalAdjustment/ - or something similar, for stylistic
consistency?

Can it effect Tonga (UTC+1300) or the Chatham Islands (UTC+1245)? :)

Thanks for looking at this; it's nice to see the skeletons from my closet put to
rest!

Comment 38

[Phil Schwartau]

Just need sr= to get this in.

Note: I am unable to apply the latest patch. I get this error:

[//d/JS_EXP/mozilla/js/src] patch < 65671.patch
patch unexpectedly ends in middle of line
patch: **** Only garbage was found in the patch input.

Comment 39

[serge (gone)]

you have to convert '\r' -> '\r\n' on win32

Comment 40

[Phil Schwartau]

Nope, the patch fails to apply on Linux, either.
The problem seems to be ^M Mac line endings;
same issue as at 

   http://bugzilla.mozilla.org/show_bug.cgi?id=89443#c44
   http://bugzilla.mozilla.org/show_bug.cgi?id=89443#c45

Comment 41

[Kenton Hanson (gone)]

Comment on attachment 79337 [details] [diff] [review]
Proposed patch

changed line endings

Comment 42

[Phil Schwartau]

Kenton, you've marked your patch as obsolete: but can you attach
a new one I could test? Thanks - 

Comment 43

[Kenton Hanson (gone)]

Comment on attachment 79337 [details] [diff] [review]
Proposed patch

Phil,  I don't know how it got marked obsolete.

Comment 44

[Kenton Hanson (gone)]

Comment on attachment 79337 [details] [diff] [review]
Proposed patch

tabs removed

Comment 45

[Kenton Hanson (gone)]

Attachment: patch 79337 correctly formatted

patch 79337 formatted correctly.

Comment 46

[Kenton Hanson (gone)]

Attachment: patch 79337 with Unix line endings

In the previous version I tried to detab the patch.  However, CVS failed
becuase the original source code has tabs embedded.

Comment 47

[Phil Schwartau]

I have been able to apply the latest patch successfully on both 
WinNT and Linux, and ran the whole JS testsuite against it.
I got no regressions, only known failures from other bugs - 

Just need sr= to get this one checked in. I think this is an
important one to have; it has been plaguing GMT+0 users ...

Note mccabe's request from Comment #37:

> Any chance of s/time_Adj/LocalAdjustment/ - or something similar,
> for stylistic consistency?

Comment 48

[Kenton Hanson (gone)]

Regarding comment #37 I tried "LocalAdjustment".  It just doesn't look right. 
It doesen't refer to time.

Comment 49

[Mike McCabe]

TimeAdjustment?

Comment 50

[John Bandhauer]

Comment on attachment 82204 [details] [diff] [review]
patch 79337 with Unix  line endings

sr=jband
(moving r=mccabe forward)

Comment 51

[Brendan Eich [:brendan]]

Can we get this on the trunk, please?  Who should own this bug?  I'm taking it
for fast 1.0 rc3 consideration.

/be

Comment 52

[Brendan Eich [:brendan]]

Attachment: mccabe-ly name: AdjustTime instead of time_Adj

I'll transfer review stamps and check into the trunk now.

/be

Comment 53

[Brendan Eich [:brendan]]

Comment on attachment 83768 [details] [diff] [review]
mccabe-ly name: AdjustTime instead of time_Adj

Does this fix suggest that there is a bug in ECMA-262?

/be

Comment 54

[Brendan Eich [:brendan]]

Fix is in the trunk, going for drivers approval for rc3.  Anyone on the cc list,
please point out other bugs with fixes lying around that should be considered
for mozilla1.0/js1.5.

/be

Comment 55

[Phil Schwartau]

Attachment: Relevant ECMA-262 passages on this issue

Comment 56

[Steven Cole]

Re: Brendan's comment 53 about potential errors in ECMA-262.

I was worried about this as well, and even went so far as to grab a co-worker's
linux box to help me understand the issue.

What I saw was that the daylight savings adjustment for London (from the
function PRMJ_DSTOffset) was set to 23 hours (presumably from some Linux kernal
call; I didn't go deep into that to see where 23 hours was coming from).

Clearly, DST in London doesn't mean that we switch days (which is what 23 hours
*really* means), the result should have really been -1 hour.

So khanson's fix is basically bad-OS protection.  Legitimate daylight
savings adjustments should probably be in the range -12 to 12 hours, and the
patch does some simple math to pretend that's the truth.

Perhaps the true fix should be placed in PRMJ_DSTOffset, but *I'm* not familiar
with other calls to that function in the rest of mozilla and I'd be worried
about breaking something.

[I just checked: the lines in PRMJ_DSTOffset that read
    if(diff < 0){
	diff += PRMJ_DAY_SECONDS;
    }
are *very* suspicious.]

--scole

Comment 57

[Kenton Hanson (gone)]

I also was worried about changing too much in prmjtime.c (see comment#34).  I
was also suspicious of PRMJ_DSTOffset not being in the range -12 to +12 hours. 
That is clearly not a problem, since GMT times exhibit both + and - offsets
correctly.
So this defensive patch protects against offsets of 24 hours or greater.

Comment 58

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 83768 [details] [diff] [review]
mccabe-ly name: AdjustTime instead of time_Adj

a=[e-mail] as per drivers list.  Make sure it's in trunk as well if not
already

Comment 59

[Brendan Eich [:brendan]]

Checked into branch as well as trunk.

/be

Comment 60

[Phil Schwartau]

Verified that the patch is in both trunk and branch;
adding the fixed1.0.0 and verified1.0.0 keywords.

Could one of our U.K. users confirm that this has fixed the 
problem? If not, please reopen this bug; thanks -

Comment 61

[olecom]

I'm using GNU+Linux.
About bug:
That's so old bug ...
But it seemes to me that coders don't care about it ;-)
I get this bug on my mozilla's clock every day.
I dug source for solution, even wrote small program to
investigate solution, but i'm not an experience programmer.
Problem with this very small, bug still is, just like this:
if
var tDate = new Date();
then
String(tDate) == "Fri Jun 04 2004 01:55:51 GMT+0300 (EET)"
but
gluon:/mnt/work/BSU/DIPLOMA/hardware/spectro/pc_sp_drv$ LANG=C date
Fri Jun  4 02:02:14 EEST 2004
gluon:/mnt/work/BSU/DIPLOMA/hardware/spectro/pc_sp_drv$
thus
"EET" != "EEST"

bug is in jsdate.c. (even commented ;-):
>    /* not sure how this affects things, but it doesn't seem
>       to matter. */
>    split->tm_isdst = (DaylightSavingTA(timeval) != 0);
there's matter as long as "Opened: 2001-01-16 16:58 PDT"
localtime_r() returns good tm.tm_isdst tm.tm_gmtoff why
they're changed again ?

IMHO code for GNU OS in jsdate.c prmjtime.c is terrible, but
MAC OS is licked and separated like something from gods ;-)
Comments below are conclusive:
prmjtime.c:
>     * N.B. This hasn't been tested with anything that actually _uses_
>     * tm_gmtoff; zero might be the wrong thing to set it to if you really need
>     * to format a time.  This fix is for jsdate.c, which only uses
>     * JS_FormatTime to get a string representing the time zone.  */
jsdate.c: * - look at tweaking function return types to return double instead
> * of int; this seems to make things run slightly faster sometimes.
> * (though it could be architecture-dependent.)  It'd be good to see
> * how this does on win32.  (Tried it on irix.)  Types could use a
> * general going-over.
And at first look many functions may be cleaned and optimized.

object Date has to be rewritten again.
Thanks.

Comment 62

[olecom]

If this BUG "Leave as VERIFIED FIXED", does in mean that no one even try to
answer comment above ?

Thanks

Comment 63

[Karn Kallio]

It seems that this problem is still around in the spidermonkey that Firefox 2.0.0.12 is built against.  Venezuela recently changed its timezone from GMT-4:00 to GMT-4:30, and since then, Date has been giving dates one day in the future, although the UTC dates are correct.


Here is an illustration of the problem:

The following is from firefox 2.0.0.12.
d = new Date();
Sat Feb 16 2008 19:22:08 GMT+1930 (VET)

d.toUTCString();
Fri, 15 Feb 2008 23:52:08 GMT

d.getTimezoneOffset();
-1170


Now the same but from Konqueror:

d = new Date();
Fri Feb 15 2008 19:27:50 GMT-0430

d.toUTCString();
Fri, 15 Feb 2008 23:57:50 GMT

d.getTimezoneOffset();
270

The UTC string is correct for both, but the date string is correct in Konqueror and wrong in Firefox.  Also the offset from UTC is correct in Konqueror and wrong in Firefox.

Karn

Comment 64

[Nickolay_Ponomarev]

This is an old fixed bug. You should be filing new bugs for new issues, even they are similar to an old fixed issue. bug 318454 can be now used to track this.