Closed Bug 351066 Opened 18 years ago Closed 12 years ago

Incorrect DST for dates after 2038 when January 1st falls on a Tuesday

Categories

(Rhino Graveyard :: Core, defect)

Product:
Rhino Graveyard
Component:
Core
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: carll, Assigned: hannesw)

Details

Attachments

(2 files, 1 obsolete file)

Details clarifying issue which also demonstrates that this is broader in scope than initially reported
78.00 KB, text/plain
Details
New patch
1.92 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

On January 4, 1974, President Nixon signed into law the Emergency Daylight Saving Time Energy Conservation Act of 1973. Then, beginning on January 6, 1974, implementing the Daylight Saving Time Energy Act, clocks were set ahead for a 15-month period through April 27, 1975.

Java knows this fact.  Rhino uses 1974 as an equivalent year for the purpose of certain calculations involving Tuesdays.  In 2041 Jan 6th is a Tuesday and Rhino switches to Daylight savings even though it shouldn't.

Reproducible: Always

Steps to Reproduce:
Context cx = Context.enter();
Object o =  cx.evaluateString(cx.initStandardObjects(), "new Date( \"1/6/2041 02:00:00\" ); ", "code", 1, null);
o = cx.jsToJava( o, Date.class );
System.out.println( o );

Actual Results:  
Sun Jan 06 01:00:00 EST 2041

Expected Results:  
Sun Jan 06 02:00:00 EST 2041

I've run this on a Mac using rhino, in the firefox browser and on a redhat linux machine from commandline.

The problem lies in the NativeDate.EquivalentYear function which picks 1974.  1974 is obviously a weird year to be using in light of the legislation passed.
This bug is related to 58118 where the EquivalentYear function was written.
Not reproducible in FireFox on Windows XP.  So, not sure if this is isolated to Rhino, SpiderMonkey, or what.
Priority: -- → P5
Priority: P5 → --
We are experiencing this problem with release 1.6R2.

We use the libraries in a financial application and we are hitting this issue very frequently and it's causing us problems with out policies.  Is there anyway to get this escalated to a P1 or P2?  It's having a very significant impact.
I can't say I fully understand the problem, and I can't reproduce it with my current Rhino snapshot. But from the original report it sounds like the problem can be fixed by choosing another non-leap year where January 1 is a Tuesday (line 704 in NativeDate.java). If I'm not mistaken 1985 is such a year. 

Can somebody confirm this change fixes the problem? It seems to fix a few (though not all) date-related failures with the Mozilla test suite.
Assignee: nobody → hannesw
> Can somebody confirm this change fixes the problem? It seems to fix a few
> (though not all) date-related failures with the Mozilla test suite.

I was wrong about the newly fixed tests in Mozilla suite. So what remains to be determined is whether the patch fixes the original bug for those who can reproduce it.
Please review attachment which further clarifies details of issue reported. Please also note that this document shows how the problem is broader in scope impacting more than the single condition reported where year parameter passed in has Jan 1 falling on a Tuesday in a non-leap year where 1974 is used for the EquivalentYear.
Please note that the attachment is in MS Word format.
I am trying to ascertain the status of this bug at this point. Please advise if the document attachment from 03/29/2012 has been reviewed and if the scope of the reported problem there is understood. Also, please advise if it is expected that this will be corrected and if so what the estimated time frame would be.

Again, we ask if there is anyway to get this escalated to a P1 or P2 as it's having a very significant impact.
How do we escalate this issue.  This is essentially a show stopper for us....
Sorry for the delay. I'm setting importance to "critical" on this bug.

I'm willing to help getting this bug fixed. While I understand the issue to some extent, the thing that would help me most is some piece of code to reproduce the problem. The code snippet from the original report seems to work for me, possibly because of my CET time zone setting:

js> new Date("1/6/2041 02:00:00")
Sun Jan 06 2041 02:00:00 GMT+0100 (MEZ)

Can you confirm this code still works for you to reproduce the bug, or can you provide some other piece of code to reproduce it?
Severity: normal → critical
I was able to reproduce the bug by explicitly setting "user.timezone" to "America/New_York" (Note: using "EST" does not work!). Alternatively, changing the OS timezone setting works, too. Tested with Win7_x64 and Ubuntu 11.10 (x64).

---
svdi@svdi-VirtualBox:~/Arbeitsfläche$ java -classpath js.jar -ea -Duser.language=en -Duser.country=US -Duser.timezone=America/New_York org.mozilla.javascript.tools.shell.Main
Rhino 1.7 release 3 2011 05 09
js> new java.util.Date(new Date("1/6/2041 02:00:00").getTime()).toString()
Sun Jan 06 01:00:00 EST 2041
js> new Date("1/6/2041 02:00:00")
Sun Jan 06 2041 01:00:00 GMT-0500 (EST)
---
Thanks André. I've now studied the current code in relation with ECMA spec and other implementations. The relevant section of ES5 is 15.9.1.8: 

http://es5.github.com/#x15.9.1.8

This second paragraph in 15.9.1.8 says an implementation should not try to determine if a date was subject to DST but just whether it would be under current rules, and the last paragraph proposes the "equivalent year" method implemented in Rhino and Spidermonkey. 

So the equivalent year method does have its place (Java in contrast uses no DST for ancient dates which is arguably more correct). The problem is with how and when to apply it.

First, the equivalent year method is currently applied for dates before 1970 and after 2038. The reason for applying it to future dates seems to have been that some OSes at the time where not able to handle those dates correctly (this was adopted from Spidermonkey, so I don't know if this ever was a problem with Java). In any case it's not a problem with current Java (tested with JDK 7) which extrapolates DST rules for future dates. So it should be safe to just stop applying this to future dates.

Second, as noted in the original report and the attached Word document, 1974 and 1975 seem to be bad years because of DST starting in January or February in some locations. I'm not sure this is per se a big issue when applied to pre-1970 dates, but it seems we can replace these years with 1985 and 1986 which are non-leap years starting on a Tuesday and Wednesday, respectively.
Status: NEW → ASSIGNED
Summary: Dates when Jan 6th falls on a Tuesday aren't setting DST correctly. → Incorrect DST for dates after 2038 when January 1st falls on a Tuesday
Attached patch New patch — — Splinter Review 
Patch based on findings listed in comment 12.
Attachment #609673 - Attachment is obsolete: true
Pushed last patch to git master and rhino_1_8 branches

https://github.com/mozilla/rhino/commit/f99cc11d616f0cdda2c42bde72b3484df6182947
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
My reading of 15.9.1.8 suggests to remove the EquivalentYear() method:

The original intention of 15.9.1.8 seems to be to avoid the burden of including a complete dst-timezone database for ECMAScript implementors. Instead of that, implementors 'only' need to implement the "current daylight saving time algorithm" (cf. 2nd paragraph of 15.9.1.8). In addition to that, the third paragraph makes it available for implementors to query the host environment for actual daylight saving times (independent of the "current daylight saving time algorithm"). 

Rhino does not attempt to actually implement the "current daylight saving time algorithm", but instead applies the third paragraph and queries the host environment (i.e. Java). Java (Oracle JDK and OpenJDK at least) uses internally the "tz database" (http://www.iana.org/time-zones). If we now assume that the "tz database" has got daylight saving time information for any year in question, the equivalent year mapping described in 15.9.1.8 is the identify function. Therefore the EquivalentYear() method can be removed. 

That also means bug 58118 was invalid to begin with.
Also see bug 365349 comment 13 concerning the spec and the de-facto standard, citing:
"[...] if major browser implementations rely on the OS for all DST information, and the OS does retain historical knowledge, then the de-facto standard is more what you want, and not what ECMA-262 specifies."
I like the idea of removing EquivalentYear(), but I'm reading 15.9.1.8 differently. IMO it explicitly requires DST to be applied for historic dates, and offers the "equivalent year" method as one simple way to do so. I made some quick tests with V8 and Spidermonkey, and V8 does apply DST to historical dates, although AFAICT with different start and end dates. Spidermonkey used to do this as well (the equivalent year code is still there) but it seems to have changed recently. 

If anybody can dig up the exact DST rules V8 or other JS engines are following, or find the bugzilla discussion for the recent change in Spidermonkey I'd be in favor of reopening this issue in order to become more compliant and/or sane. Until then I'd rather leave this in its current state.
I tried to find any recent bugs or issues recently posted related to the SpiderMonkey DST handling and came up with the following. Please forgive me if these are not related; I am not even sure if the request to find the discussion for the recent change in SpiderMonkey was actually directed to Mozilla staff but I wanted to do whatever I could to get this finalized. 

https://bugzilla.mozilla.org/show_bug.cgi?id=560163

https://bugzilla.mozilla.org/show_bug.cgi?id=564127

http://code.google.com/p/v8/source/browse/branches/bleeding_edge/test/mozilla/mozilla.status?spec=svn11416&r=11416
	# The following two tests assume that daylight savings time starts first Sunday # in April. 
	This is not true when executing the tests outside California! 
	# In Denmark the adjustment starts one week earlier!. 
	# Tests based on shell that use dates in this gap are flaky. 
	ecma/Date/15.9.5.10-1: PASS || FAIL 
	ecma/Date/15.9.5.12-1: PASS || FAIL 
	ecma/Date/15.9.5.14: PASS || FAIL 
	ecma/Date/15.9.5.34-1: PASS || FAIL
Hannes am I correct that your comment about leaving "this in its current state" would mean as you had updated with the removal of the call to EquivalentYear function for years after 2038? I just want to clarify that as opposed to leaving as it was coded before your update which would actually be a digression for our ability to address the issue that was filed with us related to this code. Thanks for your effort and progress on this.
(In reply to Hannes Wallnoefer from comment #17)
> I like the idea of removing EquivalentYear(), but I'm reading 15.9.1.8
> differently. IMO it explicitly requires DST to be applied for historic
> dates, and offers the "equivalent year" method as one simple way to do so.

Ah ok, I was mislead by the actual implementations. The spec requires all equivalent years to produce the same result (cf. last sentence in 15.9.1.8). This requirement is not followed by browser vendors to give more sane results. More below...

> If anybody can dig up the exact DST rules V8 or other JS engines are
> following, [...]

Here are the rules implemented in Spidermonkey (SM), V8 and JavaScriptCore (JSC) in pseudo-code.

SM:
if (year < 1970 or year > 2038) then
  map year to one out of: [1971..1996]
fi

https://github.com/mozilla/mozilla-central/blob/master/js/src/jsdate.cpp#L391
https://github.com/mozilla/mozilla-central/blob/master/js/src/jsdate.cpp#L404
https://github.com/mozilla/mozilla-central/blob/master/js/src/jsdate.cpp#L430

V8:
if (year < 1970 or year > 2038) then
  map year to one out of: [2008..2035]
fi

http://code.google.com/p/v8/source/browse/trunk/src/date.cc#222
http://code.google.com/p/v8/source/browse/trunk/src/date.h#146
http://code.google.com/p/v8/source/browse/trunk/src/date.h#159


JSC:
if (year < 1900 or year > 2038) then
  map year to one out of: [2010..2037]
fi

https://gitorious.org/webkit/webkit/blobs/master/Source/JavaScriptCore/runtime/DatePrototype.cpp#line238
https://gitorious.org/webkit/webkit/blobs/master/Source/WTF/wtf/DateMath.cpp#line333


The comments in SM and V8 suggest they use 1970 and 2038 as limits to avoid problems in specific OS's. (year=1970 => time=0 => start of epoch, year=2038 => time=0x7FFFFFFF => max signed int). In addition to that, violating the spec is also required to give the users more expected results. Since the old trac bug database for ES4 is no longer online, we can't check the entry mentioned in bug 365349 comment 14. I was still able to find the following mails/wiki pages:
[1] https://mail.mozilla.org/pipermail/es5-discuss/2009-August/002964.html
[2] http://wiki.ecmascript.org/doku.php?id=proposals:date_and_time#time_zones
[3] http://wiki.ecmascript.org/doku.php?id=discussion:date_and_time#lack_of_historical_dst

Per [1], it was even considered for ES5 to update 15.9.1.8 to reflect the de-facto standard implemented in browsers, but then the proposed changes were rejected by the TC39 members due to time limits and to minimize the overall amount of changes in ES5.
Thanks for looking up all those code and links. I wasn't aware all major engines are using the equivalent year method, just with different year ranges. 

Sybil, I did not mean to go back to the old buggy behavior. 

Still a bit unsure about the current state, though. André, what's your stance? Do you still think we should remove the EquivalentYear method all together?
Yes, I think I'd lean towards removing EquivalentYear, because mapping years to arbitrary chosen others years just for the sake of applying DST feels wrong to me. What about mailing to es-discuss first, maybe we get some more input from those folks?
No need to mail to es-discuss, 15.9.1.8 was on the meeting agenda [1] and also brought to attention by Luke Hoban (Microsoft) in march [2,3]. The current ECMAScript Internationalization API spec draft [4] also proposes to remove the restrictions from 15.9.1.8.

[1] https://mail.mozilla.org/pipermail/es-discuss/2012-May/022838.html
[2] https://mail.mozilla.org/pipermail/es-discuss/2012-March/020830.html
[3] https://mail.mozilla.org/pipermail/es-discuss/2012-March/020832.html
[4] http://wiki.ecmascript.org/doku.php?id=globalization:specification_drafts
Hannes, I see that this issue has been closed with a "RESOLVED FIXED" status but am unclear on how to obtain the update at this point. Is there a new version of rhino that I need to download or a patch that I need to apply to my existing version of rhino or some other means to obtain the update that was applied? Your response as soon as possible would be greatly appreciated as I am under pressure to get this resolved on our end. Thanks so much for your work on this and I await your reply. Have a great day.
Please respond to previous request for information about how to obtain the update ... This is essentially a show stopper for us and we are under a great deal of pressure to get this resolved. Thanks again.
Sybil, you can get the fixed version with the 1.7R4 release available here:

http://www.mozilla.org/rhino/download.html

... unless you're using the version of Rhino bundled with the JDK, in which case it will be hard to get an update.
Hannes, thanks so much for the reply. I have downloaded the new release and will begin implementing and testing on our end shortly. As a side note, we are not using a version of Rhino bundled with the JDK so hopefully this will be a pretty straightforward upgrade. 

Can you advise if there are any concerns with running this new release under jdk 1.4? The implementation related which the issue was reported under to us utilizes jdk 1.4 and I just want to be aware if there are compatibility issues so I will know going in if a jdk upgrade will also be required.

Thanks again for your assistance and have a great day.
Hannes, just a follow up to see if I can get a response to the question I posted on 07/20 related to any concerns with running this new release under jdk 1.4? Can you please provide a reply at your earliest convenience even if it is a situation where you cannot deem it compatible because it was not certified in your testing. I am just curious if the compatibility status is known on your side. 

Thanks much.
Hannes, I did find the answer to the jdk 1.4 compatibility question in the release notes for 1.7 ... "In order to support people running Rhino on JDK 1.4, we use Retrotranslator to produce js-14.jar, which is compatible with JDK 1.4." ... and I do see where that is included in the new Rhino 1.7R4 download so I am good to go. Thanks so much again for your work on this.
Hannes, I implemented the js-14.jar from the rhino1_7R4.zip download as the implementation being tested uses jdk 1.4. This was documented as being the appropriate jar to use in the relase notes for 1.7R1 where it states 
"We now require at least JDK 1.5 in order to compile Rhino sources. As a result, the <tt>js.jar</tt> in the binary distribution is not runnable with JDK 1.4. In order to support people running Rhino on JDK 1.4, we use Retrotranslator to produce <tt>js-14.jar</tt>, which is compatible with JDK 1.4." 

I am having an issue based on jdk 1.4 incompatibility as shown in the stack trace below based on the method Integer.valueOf(int) not existing in jdk 1.4.x; My understanding is that this method is new to Java 1.5.

Can you please look into this as quickly as possible? This does represent a set back for us as we were expecting that we would be up and running once the update was implemented. Please provide a response as soon as possible since this has been a show stopper for us for many months.

Thanks much for your consideration.







Caused By:
        Unexpected Error; nested exception is:
        java.lang.NoSuchMethodError: java.lang.Integer.valueOf(I)Ljava/lang/Integer;
Caused By:
        java.lang.Integer.valueOf(I)Ljava/lang/Integer;
Caused by: java.lang.NoSuchMethodError: java.lang.Integer.valueOf(I)Ljava/lang/Integer;
        at org.mozilla.javascript.gen.ScriptingCode_6.<clinit>(ScriptingCode)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:274)
        at java.lang.Class.newInstance0(Class.java:308)
        at java.lang.Class.newInstance(Class.java:261)
        at org.mozilla.javascript.optimizer.Codegen.createScriptObject(Codegen.java:89)
        at org.mozilla.javascript.Context.compileImpl(Context.java:2394)
        at org.mozilla.javascript.Context.compileString(Context.java:1335)
        at org.mozilla.javascript.Context.compileString(Context.java:1324)
        at com.adminserver.utl.scripting.RhinoScopeUtl.compile(RhinoScopeUtl.java:230)
        at com.adminserver.utl.CalculateUtl.compileAll(CalculateUtl.java:358)
        at com.adminserver.utl.CalculateUtl.resolveInputs(CalculateUtl.java:2803)
        at com.adminserver.utl.CalculateUtl.resolveInputs(CalculateUtl.java:2725)
        at com.adminserver.utl.CalculateUtl.resolveInputsAndReturnValues(CalculateUtl.java:2703)
        at com.adminserver.bll.impl.InquiryScreenImpl.processMathResult(InquiryScreenImpl.java:150)
        at com.adminserver.bll.impl.InquiryScreenImpl.processInquiryScreenResults(InquiryScreenImpl.java:77)
        at com.adminserver.bll.InquiryScreenBllBean.processInquiryScreenResults(InquiryScreenBllBean.java:70)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:324)
        at org.jboss.invocation.Invocation.performCall(Invocation.java:345)
        at org.jboss.ejb.StatelessSessionContainer$ContainerInterceptor.invoke(StatelessSessionContainer.java:214)
        at org.jboss.resource.connectionmanager.CachedConnectionInterceptor.invoke(CachedConnectionInterceptor.java:185)
        at org.jboss.ejb.plugins.StatelessSessionInstanceInterceptor.invoke(StatelessSessionInstanceInterceptor.java:130)
        at org.jboss.webservice.server.ServiceEndpointInterceptor.invoke(ServiceEndpointInterceptor.java:51)
        at org.jboss.ejb.plugins.CallValidationInterceptor.invoke(CallValidationInterceptor.java:48)
        at org.jboss.ejb.plugins.AbstractTxInterceptor.invokeNext(AbstractTxInterceptor.java:105)
        at org.jboss.ejb.plugins.TxInterceptorCMT.runWithTransactions(TxInterceptorCMT.java:363)
        at org.jboss.ejb.plugins.TxInterceptorCMT.invoke(TxInterceptorCMT.java:166)
        at org.jboss.ejb.plugins.SecurityInterceptor.invoke(SecurityInterceptor.java:139)
        at org.jboss.ejb.plugins.LogInterceptor.invoke(LogInterceptor.java:192)
        ... 47 more
Hannes as a frame of reference, I performed the same test using rhino 1.6R2 and did not see the error as shown in the prior comment under that test although the date issue at the basis of this issue was seen as would be expected. This comment is being provided simply to support that the error being seen using js-14.jar seems to be related to the implementaion of the new version of the rhino library.
Sybil, this error is caused by code that is generated on the fly by Rhino's bytecode compiler (and therefore not processed by Retrotranslator). One workaround would be to disable optimization in Rhino (and thus bytecode generation) by setting the optimization level to -1 on the rhino Context object. Would that be a viable solution (or workaround) for you?
Hannes, thanks so much for the prompt reply. I did change the optimization setting to -1 and the results looked great. We are going to go ahead with use of this setting at this time and will wait to see if the client complains about increased performance time related to that change. Hopefully that will not be the case.

Thanks again for all of your help. It is greatly appreciated.
Hi,

And cant's say that if this issue is causing my problem or not but still
I have this code that get me incorrect date:
        Context c = Context.enter();
        Object o =  c.evaluateString(c.initStandardObjects(), "new Date(2011, 0, 1); ", "code", 1, null);
        o = Context.jsToJava(o, Date.class);
        System.out.println( o );
result:
        Fri Dec 31 23:00:00 MSK 2010

This issue can be reproduced on 1.7R5(prerelease)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: