ome/bioformats

Argument #1 to org.joda.time.DateTime is declared as a long but an int is being passed in MetamorphReader.decodeTime(int millis).

scuniff opened this issue · 1 comments

Not entirely sure about all of this, but….

Regarding

DateTime tm = new DateTime(millis, DateTimeZone.UTC);

Line 2051
DateTime tm = new DateTime(millis, DateTimeZone.UTC);

Argument #1 to org.joda.time.DateTime is declared as a long but an int is being passed in MetamorphReader.decodeTime(int millis).

So argument variable millis should be changed from int to long.

An int is between -2,147,483,648 and 2,147,483,647.

So the time range from the DateTime constructor is only Dec 1969 - Jan 1970

Code example:

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
public class JodaDateTime2 {

	public static void main(String[] args) {
		// TODO Auto-generated method stub

		int millis = -2147483648;
		DateTime tm = new DateTime(millis, DateTimeZone.UTC);
		System.out.println("DateTime for "+ millis + " is " + tm);

		millis = 2147483647;
		tm = new DateTime(millis, DateTimeZone.UTC);
		System.out.println("DateTime for  "+ millis + " is " + tm);

	}

}

Output:

DateTime for -2147483648 is 1969-12-07T03:28:36.352Z
DateTime for 2147483647 is 1970-01-25T20:31:23.647Z

So argument variable millis should be changed from int to long.

But, if there’s any software out there (OMERO,imagej,cell profiler, custom plugins, etc) now using this method it is presently only using a 32 bit int for the argument which is not correct and producing dates constricted to the example above.

How to go about determining this?

Add code to decodeTime() that checks to see if the argument is between –2,147,483,648 and 2,147,483,647 and flag an error?

Or, is there nothing here to worry about??😎

Also, joda recommends using java.Time API after jdk 1.8 instead of their API.

https://www.joda.org/joda-time/

Note that from Java SE 8 onwards, users are asked to migrate to java.time (JSR-310) - a core part of the JDK which replaces this project.

dgault commented

Thanks @scuniff for raising the issue. I investigated with some sample files that we have and the MetaMorph files are using the DateTime in 2 separate values, 1 for the date (year, month, day) and one for the time (hours, minutes, seconds etc). For the date we are using a method that performs the calculations from the int that is stored in the metadata. For the time we are using DateTime but are only interested in the time portion, and for that purpose the int values provide the correct output. I don't believe that the decodeTime and decodeDate methods are used anywhere outside of the MetaMorphReader, so they should probably have been listed as private rather than public, though changing them at this stage would be a breaking API change and thus would require a major release.