Skip to content

StdDateFormat does not interpret "millis" correctly #1668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brenuart opened this issue Jun 16, 2017 · 4 comments
Closed

StdDateFormat does not interpret "millis" correctly #1668

brenuart opened this issue Jun 16, 2017 · 4 comments
Milestone

Comments

@brenuart
Copy link
Contributor

Here is an excerpt from the ISO8601 norm (https://en.wikipedia.org/wiki/ISO_8601 - sorry could not find any official source) - in the Times section it says:

The basic format is [hh][mm][ss] and the extended format is [hh]:[mm]:[ss].
...
Decimal fractions may be added to any of the three time elements. However, a fraction may only be added to the lowest order time element in the representation.

So the following representation are accepted:

  • 01.5, means 1 hour and a half, i.e. equivalent to 01:30:00
  • 01:01.5, means 1 hour, 1 minute and a half, i.e. equivalent to 01:01:30
  • 01:01:01.5, means ..., i.e. equivalent to 01:01:01.500

It is important to note the norm doesn't have the concept of millis but fraction of seconds. This is even better explained by the BNF presented in RFC3339 (https://www.ietf.org/rfc/rfc3339.txt), section 5.6 and/or appendix A.

Unfortunately, the way Jackson's StdDateFormat treats the fraction of seconds is not inline with the above.

  • 00:00:00.5 is interpreted as 00:00:00.005, i.e. 5 millis instead of 0.5 seconds.
  • 00:00:00.1234 is interpreted as 00:00:01.234, i.e. 1234 millis which is equivalent to 1 sec and 234 millis.

These cases silently produce wrong date values - at least according to ISO8601 norm.

One can argue java.util.Date as millis precision only and won't be able to cope with .1234 anyway. This is true, but not a reason to produces wrong result. It should either truncate to the first 3 digits (as does Joda) or refuse and throw an InvalidFormatException.


Issue described as of Jackson 2.8.9

@cowtowncoder
Copy link
Member

You are right, and this was my understanding as well (that one can only vary precision of second fraction, and if so, it would be interpreted the way you describe). So, flawed handling.
Either variations should be supported, with valid results, or result in exception indicating the nature of the problem (not matching template).

Question then is whether this is something StdDateFormat directly does, or underlying SimpleDateFormat.

I'll see if I can quickly reproduce this now.

@cowtowncoder
Copy link
Member

Ok, I think this piece of code in StdDateFormat is wrong:

                // And possible also millisecond part if missing
                if (timeLen < 12) { // missing, or partial
                    StringBuilder sb = new StringBuilder(dateStr);
                    switch (timeLen) {
                    case 11: sb.append('0');
                    case 10: sb.append('0');
                    case 9: sb.append('0');
                        break;
                    default:
                        sb.append(".000");
                    }
                    dateStr = sb.toString();
                }

with the obvious problem being wrong ordering (should add three zeroes if length 9, two for 10, one for 11). There are plenty more problems there, too, since code assumes that all preceding parts are included in their entirety (4-digit year)

I don't know is SimpleDateFormat can reliably deal with variable length milliseconds; I suspect not as there'd be no point in adding padding if it did (that is, padding has been added to work around problem).

@cowtowncoder cowtowncoder added this to the 2.9.0 milestone Jun 27, 2017
@cowtowncoder
Copy link
Member

Fixed via #1678. Some notes:

  1. No incomplete values accepted for year/month/day, hours/minutes.
  2. Seconds optional, but if they exist, must be 2-digit
  3. Fractional seconds allowed, 1-3 digits (could accept longer, truncate, alternatively)
  4. Values themselves passed to GregorianCalendar, with proper leniency setting (via config overrides, annotations); may allow use of "wrap-around" values.

@brenuart
Copy link
Contributor Author

brenuart commented Jun 27, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants