diff --git a/server/src/main/java/org/opensearch/common/time/DateFormatter.java b/server/src/main/java/org/opensearch/common/time/DateFormatter.java index c98bd853dfced..a88360a5b1cd8 100644 --- a/server/src/main/java/org/opensearch/common/time/DateFormatter.java +++ b/server/src/main/java/org/opensearch/common/time/DateFormatter.java @@ -39,7 +39,6 @@ import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; -import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAccessor; import java.util.ArrayList; import java.util.List; @@ -56,7 +55,7 @@ public interface DateFormatter { /** * Try to parse input to a java time TemporalAccessor * @param input An arbitrary string resembling the string representation of a date or time - * @throws DateTimeParseException If parsing fails, this exception will be thrown. + * @throws IllegalArgumentException If parsing fails, this exception will be thrown. * Note that it can contained suppressed exceptions when several formatters failed parse this value * @return The java time object containing the parsed input */ diff --git a/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java b/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java index f711b14aeb928..d351e351911ad 100644 --- a/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java +++ b/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java @@ -36,6 +36,7 @@ import org.opensearch.core.common.Strings; import java.text.ParsePosition; +import java.time.DateTimeException; import java.time.ZoneId; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; @@ -52,7 +53,6 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.BiConsumer; import java.util.stream.Collectors; @@ -285,27 +285,54 @@ public TemporalAccessor parse(String input) { * @throws DateTimeParseException when unable to parse with any parsers */ private TemporalAccessor doParse(String input) { - if (parsers.size() > 1) { - Object object = null; - if (canCacheLastParsedFormatter && lastParsedformatter != null) { - ParsePosition pos = new ParsePosition(0); - object = lastParsedformatter.toFormat().parseObject(input, pos); + if (parsers.size() != 1) { + // Copy to local from volatile to avoid error in multi-threads, + // other threads may update lastParsedformatter + final DateTimeFormatter cachedFormatter = lastParsedformatter; + if (canCacheLastParsedFormatter && cachedFormatter != null) { + final ParsePosition pos = new ParsePosition(0); + Object object = tryParseObject(cachedFormatter, input, pos); if (parsingSucceeded(object, input, pos)) { return (TemporalAccessor) object; } } for (DateTimeFormatter formatter : parsers) { - ParsePosition pos = new ParsePosition(0); - object = formatter.toFormat().parseObject(input, pos); - if (parsingSucceeded(object, input, pos)) { - lastParsedformatter = formatter; - return (TemporalAccessor) object; + if (formatter != cachedFormatter || !canCacheLastParsedFormatter) { + final ParsePosition pos = new ParsePosition(0); + Object object = tryParseObject(formatter, input, pos); + if (parsingSucceeded(object, input, pos)) { + if (canCacheLastParsedFormatter) { + lastParsedformatter = formatter; + } + return (TemporalAccessor) object; + } } } - throw new DateTimeParseException("Failed to parse with all enclosed parsers", input, 0); + throw new DateTimeParseException("failed to parse date field [" + input + "] with format [" + format + "]", input, 0); + } else { + return parsers.get(0).parse(input); + } + } + + /** + * Try to parse input with given formatter, return null if it fails to parse input. + * Because of JDK bug https://bugs.openjdk.org/browse/JDK-8319640, + * {@link java.text.Format#parseObject(String, ParsePosition)} may throw ${@link DateTimeException} + * rather than {@link DateTimeParseException}, causing early termination of the loop in {@link #doParse(String)}, + * this method catches DateTimeException to avoid the exception is leaked to invoker. + * + * @param formatter formatter used to parse input. + * @param input input to parse + * @param pos initial ParsePosition + * @return parse result if there is {@link DateTimeException} + */ + private Object tryParseObject(final DateTimeFormatter formatter, final String input, final ParsePosition pos) { + try { + return formatter.toFormat().parseObject(input, pos); + } catch (final DateTimeException ex) { + return null; } - return this.parsers.get(0).parse(input); } private boolean parsingSucceeded(Object object, String input, ParsePosition pos) { @@ -318,9 +345,7 @@ public DateFormatter withZone(ZoneId zoneId) { if (zoneId.equals(zone())) { return this; } - List parsers = new CopyOnWriteArrayList<>( - this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList()) - ); + List parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList()); List roundUpParsers = this.roundupParser.getParsers() .stream() .map(p -> p.withZone(zoneId)) @@ -334,9 +359,7 @@ public DateFormatter withLocale(Locale locale) { if (locale.equals(locale())) { return this; } - List parsers = new CopyOnWriteArrayList<>( - this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList()) - ); + List parsers = this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList()); List roundUpParsers = this.roundupParser.getParsers() .stream() .map(p -> p.withLocale(locale)) diff --git a/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java b/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java index 681daf1755890..54b8716fbd2df 100644 --- a/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java +++ b/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java @@ -683,4 +683,15 @@ public void testCamelCaseDeprecation() { } } } + + public void testToVerifyFixForDateTimeException() { + DateFormatter formatter = DateFormatter.forPattern("strict_date_optional_time||epoch_millis"); + assertEquals(1522431028842L, Instant.from(formatter.parse("2018-03-30T17:30:28.842Z")).toEpochMilli()); + assertEquals(1522431028842L, Instant.from(formatter.parse("1522431028842")).toEpochMilli()); + assertThrows(IllegalArgumentException.class, () -> formatter.parse("")); + // text causes DateTimeException from DateTimeFormatter#parseObject for the pattern in this test, + // and causes parse failure. + assertThrows(IllegalArgumentException.class, () -> formatter.parse("2018-03-30T17-30-28.842Z")); + } + }