Repository navigation
Fix date-only parsing failure when local midnight is skipped by DST #3109
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
base: main
Are you sure you want to change the base?
Changes from all commits
bac6f8e
3a11c27
321a5de
f7d8104
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,50 @@ private static GregorianCalendar createUtcCalendar() { | |
| return calendar; | ||
| } | ||
|
|
||
| @Test | ||
| public void testParseDateOnlyDSTGap() throws ParseException { | ||
| TimeZone defaultTimeZone = TimeZone.getDefault(); | ||
| try { | ||
| // 1966-11-01 00:00 does not exist in America/Sao_Paulo because clocks were shifted | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This example makes it look as if this is a problem that almost nobody will have. Googling suggests that there are present-day timezones that switch to and from DST at midnight. I believe you can use America/Santiago and 2026-09-06 for a more realistic test case.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is the exact reproducer from #2539 though; and based on that issue it is parsed as date of birth, so not such an unreasonable / uncommon case? (Not saying though that a reproducer with a more recent date wouldn't be useful.) |
||
| // forward from 0:00 to 1:00 at midnight; parsing must not fail for this valid date | ||
| TimeZone.setDefault(TimeZone.getTimeZone("America/Sao_Paulo")); | ||
| Date date = ISO8601Utils.parse("1966-11-01", new ParsePosition(0)); | ||
|
|
||
| GregorianCalendar calendar = | ||
| new GregorianCalendar(TimeZone.getTimeZone("America/Sao_Paulo"), Locale.US); | ||
| // Calendar was created with current time, must clear it | ||
| calendar.clear(); | ||
| calendar.setTime(date); | ||
| assertThat(calendar.get(Calendar.YEAR)).isEqualTo(1966); | ||
| assertThat(calendar.get(Calendar.MONTH)).isEqualTo(Calendar.NOVEMBER); | ||
| assertThat(calendar.get(Calendar.DAY_OF_MONTH)).isEqualTo(1); | ||
| // The resolved hour depends on the DST rules of the JDK's bundled timezone data | ||
| // (midnight itself, or the first hour after the 0:00 -> 1:00 transition), so only | ||
| // assert that the date fields are preserved while the hour stays within the day | ||
| assertThat(calendar.get(Calendar.HOUR_OF_DAY)).isAnyOf(0, 1); | ||
| } finally { | ||
| TimeZone.setDefault(defaultTimeZone); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testParseInvalidDateOnlyStillFails() { | ||
| TimeZone defaultTimeZone = TimeZone.getDefault(); | ||
| try { | ||
| // Strict parsing introduced for date-only values must keep rejecting dates which do | ||
| // not exist, even in time zones where midnight is skipped by a DST transition | ||
| TimeZone.setDefault(TimeZone.getTimeZone("America/Sao_Paulo")); | ||
| assertThrows( | ||
| ParseException.class, () -> ISO8601Utils.parse("2021-02-30", new ParsePosition(0))); | ||
| assertThrows( | ||
| ParseException.class, () -> ISO8601Utils.parse("2021-13-01", new ParsePosition(0))); | ||
| assertThrows( | ||
| ParseException.class, () -> ISO8601Utils.parse("1966-00-01", new ParsePosition(0))); | ||
| } finally { | ||
| TimeZone.setDefault(defaultTimeZone); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testDateFormatString() { | ||
| GregorianCalendar calendar = new GregorianCalendar(utcTimeZone(), Locale.US); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me a bit uneasy. I'm sure there's more than one way we could get this exception.
Could we just construct a
GregorianCalendarinstance that specifies noon instead of midnight? Then we should be able to avoid the issue.I also think ultimately we should rewrite this method so it uses
java.timeAPIs, but that's a bigger project, and might have implications for Android (with old target SDKs and no desugaring).