Skip to content

Commit 246a7df

Browse files
authored
Core: Fix epoch millis java time formatter (elastic#33302)
The existing implemention could not deal with negative numbers as well as +- 999 milliseconds around the epoch. This commit uses Instant.ofEpochMilli() and parses the input to a number instead of using a date formatter.
1 parent 978d1ed commit 246a7df

File tree

3 files changed

+121
-2
lines changed

3 files changed

+121
-2
lines changed

server/src/main/java/org/elasticsearch/common/time/DateFormatters.java

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
import java.time.DayOfWeek;
2626
import java.time.Instant;
2727
import java.time.LocalDate;
28+
import java.time.ZoneId;
2829
import java.time.ZoneOffset;
2930
import java.time.ZonedDateTime;
3031
import java.time.format.DateTimeFormatter;
3132
import java.time.format.DateTimeFormatterBuilder;
33+
import java.time.format.DateTimeParseException;
3234
import java.time.format.ResolverStyle;
3335
import java.time.format.SignStyle;
3436
import java.time.temporal.ChronoField;
@@ -879,11 +881,47 @@ public class DateFormatters {
879881

880882
/*
881883
* Returns a formatter for parsing the milliseconds since the epoch
884+
* This one needs a custom implementation, because the standard date formatter can not parse negative values
885+
* or anything +- 999 milliseconds around the epoch
886+
*
887+
* This implementation just resorts to parsing the input directly to an Instant by trying to parse a number.
882888
*/
883-
private static final CompoundDateTimeFormatter EPOCH_MILLIS = new CompoundDateTimeFormatter(new DateTimeFormatterBuilder()
889+
private static final DateTimeFormatter EPOCH_MILLIS_FORMATTER = new DateTimeFormatterBuilder()
884890
.appendValue(ChronoField.INSTANT_SECONDS, 1, 19, SignStyle.NEVER)
885891
.appendValue(ChronoField.MILLI_OF_SECOND, 3)
886-
.toFormatter(Locale.ROOT));
892+
.toFormatter(Locale.ROOT);
893+
894+
private static final class EpochDateTimeFormatter extends CompoundDateTimeFormatter {
895+
896+
private EpochDateTimeFormatter() {
897+
super(EPOCH_MILLIS_FORMATTER);
898+
}
899+
900+
private EpochDateTimeFormatter(ZoneId zoneId) {
901+
super(EPOCH_MILLIS_FORMATTER.withZone(zoneId));
902+
}
903+
904+
@Override
905+
public TemporalAccessor parse(String input) {
906+
try {
907+
return Instant.ofEpochMilli(Long.valueOf(input)).atZone(ZoneOffset.UTC);
908+
} catch (NumberFormatException e) {
909+
throw new DateTimeParseException("invalid number", input, 0, e);
910+
}
911+
}
912+
913+
@Override
914+
public CompoundDateTimeFormatter withZone(ZoneId zoneId) {
915+
return new EpochDateTimeFormatter(zoneId);
916+
}
917+
918+
@Override
919+
public String format(TemporalAccessor accessor) {
920+
return String.valueOf(Instant.from(accessor).toEpochMilli());
921+
}
922+
}
923+
924+
private static final CompoundDateTimeFormatter EPOCH_MILLIS = new EpochDateTimeFormatter();
887925

888926
/*
889927
* Returns a formatter that combines a full date and two digit hour of

server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,15 @@ public void testCustomTimeFormats() {
7171

7272
public void testDuellingFormatsValidParsing() {
7373
assertSameDate("1522332219", "epoch_second");
74+
assertSameDate("0", "epoch_second");
75+
assertSameDate("1", "epoch_second");
76+
assertSameDate("-1", "epoch_second");
77+
assertSameDate("-1522332219", "epoch_second");
7478
assertSameDate("1522332219321", "epoch_millis");
79+
assertSameDate("0", "epoch_millis");
80+
assertSameDate("1", "epoch_millis");
81+
assertSameDate("-1", "epoch_millis");
82+
assertSameDate("-1522332219321", "epoch_millis");
7583

7684
assertSameDate("20181126", "basic_date");
7785
assertSameDate("20181126T121212.123Z", "basic_date_time");
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.time;
21+
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import java.time.ZoneId;
25+
import java.time.ZonedDateTime;
26+
import java.time.format.DateTimeParseException;
27+
import java.time.temporal.TemporalAccessor;
28+
29+
import static org.hamcrest.Matchers.containsString;
30+
import static org.hamcrest.Matchers.is;
31+
32+
public class DateFormattersTests extends ESTestCase {
33+
34+
// the epoch milli parser is a bit special, as it does not use date formatter, see comments in DateFormatters
35+
public void testEpochMilliParser() {
36+
CompoundDateTimeFormatter formatter = DateFormatters.forPattern("epoch_millis");
37+
38+
DateTimeParseException e = expectThrows(DateTimeParseException.class, () -> formatter.parse("invalid"));
39+
assertThat(e.getMessage(), containsString("invalid number"));
40+
41+
// different zone, should still yield the same output, as epoch is time zoned independent
42+
ZoneId zoneId = randomZone();
43+
CompoundDateTimeFormatter zonedFormatter = formatter.withZone(zoneId);
44+
assertThat(zonedFormatter.printer.getZone(), is(zoneId));
45+
46+
// test with negative and non negative values
47+
assertThatSameDateTime(formatter, zonedFormatter, randomNonNegativeLong() * -1);
48+
assertThatSameDateTime(formatter, zonedFormatter, randomNonNegativeLong());
49+
assertThatSameDateTime(formatter, zonedFormatter, 0);
50+
assertThatSameDateTime(formatter, zonedFormatter, -1);
51+
assertThatSameDateTime(formatter, zonedFormatter, 1);
52+
53+
// format() output should be equal as well
54+
assertSameFormat(formatter, randomNonNegativeLong() * -1);
55+
assertSameFormat(formatter, randomNonNegativeLong());
56+
assertSameFormat(formatter, 0);
57+
assertSameFormat(formatter, -1);
58+
assertSameFormat(formatter, 1);
59+
}
60+
61+
private void assertThatSameDateTime(CompoundDateTimeFormatter formatter, CompoundDateTimeFormatter zonedFormatter, long millis) {
62+
String millisAsString = String.valueOf(millis);
63+
ZonedDateTime formatterZonedDateTime = DateFormatters.toZonedDateTime(formatter.parse(millisAsString));
64+
ZonedDateTime zonedFormatterZonedDateTime = DateFormatters.toZonedDateTime(zonedFormatter.parse(millisAsString));
65+
assertThat(formatterZonedDateTime.toInstant().toEpochMilli(), is(zonedFormatterZonedDateTime.toInstant().toEpochMilli()));
66+
}
67+
68+
private void assertSameFormat(CompoundDateTimeFormatter formatter, long millis) {
69+
String millisAsString = String.valueOf(millis);
70+
TemporalAccessor accessor = formatter.parse(millisAsString);
71+
assertThat(millisAsString, is(formatter.format(accessor)));
72+
}
73+
}

0 commit comments

Comments
 (0)