GregorianCalendar - it's a bomb ;-) [code insight]

[Xandau]

on the beginning i have to say that i use SDK 1.3.1_11


public void m() {
Integer scanInterval = new Integer(581);
try {
Date d;
GregorianCalendar gc = new GregorianCalendar();
GregorianCalendar gc2 = new GregorianCalendar();
System.out.println("GC = "+ gc.getTime().toString());

gc.set(gc.HOUR_OF_DAY, gc.get(gc.HOUR_OF_DAY) +
scanInterval.intValue());
System.out.println("GC = "+ gc.getTime().toString());
gc2.add(GregorianCalendar.HOUR_OF_DAY, scanInterval.intValue());
System.out.println("GC2 = "+ gc2.getTime().toString());
System.out.println("XXX "+ gc.get(gc.HOUR_OF_DAY));
} catch (Exception ex) {
ex.printStackTrace();
}
}

and on standard output we can see:
GC = Tue Jun 22 15:03:42 CEST 2004
GC = Fri Jul 16 20:03:42 CEST 2004
GC2 = Fri Jul 16 20:03:42 CEST 2004
XXX 20

- so the results are excactrly the same as we have expected ...
but if we change a value of "scanInterval" to higher like 585 or more...

public void m() {
Integer scanInterval = new Integer(590);
try {
Date d;
GregorianCalendar gc = new GregorianCalendar();
GregorianCalendar gc2 = new GregorianCalendar();
System.out.println("GC = "+ gc.getTime().toString());

gc.set(gc.HOUR_OF_DAY, gc.get(gc.HOUR_OF_DAY)
+scanInterval.intValue());
System.out.println("GC = "+ gc.getTime().toString());
gc2.add(GregorianCalendar.HOUR_OF_DAY, scanInterval.intValue());
System.out.println("GC2 = "+ gc2.getTime().toString());
System.out.println("XXX "+ gc.get(gc.HOUR_OF_DAY));
} catch (Exception ex) {
ex.printStackTrace();
}
}


the results are unbelivible :
GC = Tue Jun 22 15:07:37 CEST 2004
GC = Fri May 28 12:04:50 CEST 2004
^^^^^^^^^^^^^^^^^^^^^^^^^^
GC2 = Sat Jul 17 05:07:37 CEST 2004
XXX 12

i am wondering why 580 hours is counted corrected but 584 is counted
wrong...


regards,
Adam

 

[Guest]

gc.set(gc.HOUR_OF_DAY, gc.get(gc.HOUR_OF_DAY) + scanInterval.intValue());
✄ ✄ ✄ ...
i am wondering why 580 hours is counted corrected but 584 is counted wrong...

Bug is inside computeTime method of GregorianCalendar.
Here the following value
zzz * 60 *60 * 1000
is assigned to the millisInDay int variable,
where zzz is the value used in the invocation of:
gc.set(gc.HOUR_OF_DAY, zzz)

As you can note the int expression
zzz * 60 *60 * 1000
is OK when zzz <= 596, but it overflow when zzz >= 597.

So the bug!

Probably this bug must be submitted to Sun,
or, perhaps, the bug is already known by Sun.

- Dario

 

[Thomas Weidenfeller]

i am wondering why 580 hours is counted corrected but 584 is counted
wrong...

You are working well out of the range of HOUR_OF_DAY. The set behavior
for values larger than 23 hours is not defined. You were just lucky that
it worked until 580 hours and that no little green gremlins came out of
your CD drive.

You would have to study the source code (comes with the SDK) to figure
out why it works for so long. As a guess, a variable overflows at that
point, is cut of, and you are thrown back in time. Interestingly, you
are thrown back something like 24 days. And 24 * 24 is 576, close to
your magic numbers. Yep, looks pretty much like an overflow to me.

/Thomas

 

[Michael Borgwardt]

Bug is inside computeTime method of GregorianCalendar.
Here the following value
zzz * 60 *60 * 1000
is assigned to the millisInDay int variable,
where zzz is the value used in the invocation of:
gc.set(gc.HOUR_OF_DAY, zzz)

As you can note the int expression
zzz * 60 *60 * 1000
is OK when zzz <= 596, but it overflow when zzz >= 597.

So the bug!

Probably this bug must be submitted to Sun,
or, perhaps, the bug is already known by Sun.

Actually, I wouldn't really call it a but. Note that the method is called*set*.
Setting HOUR_OF_DAY to such an extremely large value makes no sense. The *add*,
where it does make sense, works fine.

Of course, it shouldn't overflow silently. The parameter should be checked and
an exception thrown when it's too large, and the restriction documented
in the API.

 

[Thomas Weidenfeller]

Bug is inside computeTime method of GregorianCalendar. [...]
So the bug!

Probably this bug must be submitted to Sun,
or, perhaps, the bug is already known by Sun.

I don't think it is a bug. Calendar's documentation indicates that the
field is for the 24-hour clock of the day. And I think we can agree that
there is no 580 hour in a day. The set is simply operated outside of the
specification.

The only change I would consider if I would be Sun would be to throw an
exception if someone tries to set the field to something outside the
range of [0..23].

/Thomas

 

[Roedy Green]

The only change I would consider if I would be Sun would be to throw an
exception if someone tries to set the field to something outside the
range of [0..23].

the other way of thinking is, what timestamp represents 26 hours after
.... and getting what you want with illegal values.

In BigDate you have a choice.

 

[Michael Borgwardt]

The only change I would consider if I would be Sun would be to throw an
exception if someone tries to set the field to something outside the
range of [0..23].

the other way of thinking is, what timestamp represents 26 hours after

That's what the add() method is for.

 

[Xandau]

Thanks all for answers,
however i think that even if i have used a set
methond with to large number i should got an exception....

have changed a call to add... a now is working well.

Adam

 

[Michael Borgwardt]

Thanks all for answers,
however i think that even if i have used a set
methond with to large number i should got an exception....

I think everyone agrees that the silent failure via integer overflow
and especially the lack of documentation of the allowable value range
is a serious flaw in the API.

 

[Guest]

I think everyone agrees that the silent failure via integer overflow
and especially the lack of documentation of the allowable value range
is a serious flaw in the API.

Bugs 4936355 and 5067733 are fixed in Tiger.Beta.

- Dario