Skip to content

Commit

Permalink
Forbid trappy methods from java.time (#28476)
Browse files Browse the repository at this point in the history
ava.time has the functionality needed to deal with timezones with varying 
offsets correctly, but it also has a bunch of methods that silently let you
forget about the hard cases, which raises the risk that we'll quietly do the
wrong thing at some point in the future.

This change adds the trappy methods to the list of forbidden methods to try and
help stop this from happening.

It also fixes the only use of these methods in the codebase so far:
IngestDocument#deepCopy() used ZonedDateTime.of() which may alter the offset of
the given time in cases where the offset is ambiguous.
  • Loading branch information
DaveCTurner committed Feb 2, 2018
1 parent 599c71b commit bb32516
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 4 deletions.
53 changes: 53 additions & 0 deletions buildSrc/src/main/resources/forbidden/es-server-signatures.txt
Expand Up @@ -82,3 +82,56 @@ org.joda.time.DateTime#<init>(int, int, int, int, int, int)
org.joda.time.DateTime#<init>(int, int, int, int, int, int, int)
org.joda.time.DateTime#now()
org.joda.time.DateTimeZone#getDefault()

@defaultMessage Local times may be ambiguous or nonexistent in a specific time zones. Use ZoneRules#getValidOffsets() instead.
java.time.LocalDateTime#atZone(java.time.ZoneId)
java.time.ZonedDateTime#of(int, int, int, int, int, int, int, java.time.ZoneId)
java.time.ZonedDateTime#of(java.time.LocalDate, java.time.LocalTime, java.time.ZoneId)
java.time.ZonedDateTime#of(java.time.LocalDateTime, java.time.ZoneId)
java.time.ZonedDateTime#truncatedTo(java.time.temporal.TemporalUnit)
java.time.ZonedDateTime#of(int, int, int, int, int, int, int, java.time.ZoneId)
java.time.ZonedDateTime#of(java.time.LocalDate, java.time.LocalTime, java.time.ZoneId)
java.time.ZonedDateTime#of(java.time.LocalDateTime, java.time.ZoneId)
java.time.ZonedDateTime#ofLocal(java.time.LocalDateTime, java.time.ZoneId, java.time.ZoneOffset)
java.time.OffsetDateTime#atZoneSimilarLocal(java.time.ZoneId)
java.time.zone.ZoneRules#getOffset(java.time.LocalDateTime)

@defaultMessage Manipulation of an OffsetDateTime may yield a time that is not valid in the desired time zone. Use ZonedDateTime instead.
java.time.OffsetDateTime#minus(long, java.time.temporal.TemporalUnit)
java.time.OffsetDateTime#minus(long, java.time.temporal.TemporalUnit)
java.time.OffsetDateTime#minus(java.time.temporal.TemporalAmount)
java.time.OffsetDateTime#minusDays(long)
java.time.OffsetDateTime#minusHours(long)
java.time.OffsetDateTime#minusMinutes(long)
java.time.OffsetDateTime#minusMonths(long)
java.time.OffsetDateTime#minusNanos(long)
java.time.OffsetDateTime#minusSeconds(long)
java.time.OffsetDateTime#minusWeeks(long)
java.time.OffsetDateTime#minusYears(long)
java.time.OffsetDateTime#plus(long, java.time.temporal.TemporalUnit)
java.time.OffsetDateTime#plus(java.time.temporal.TemporalAmount)
java.time.OffsetDateTime#plusDays(long)
java.time.OffsetDateTime#plusHours(long)
java.time.OffsetDateTime#plusMinutes(long)
java.time.OffsetDateTime#plusMonths(long)
java.time.OffsetDateTime#plusNanos(long)
java.time.OffsetDateTime#plusSeconds(long)
java.time.OffsetDateTime#plusWeeks(long)
java.time.OffsetDateTime#plusYears(long)
java.time.OffsetDateTime#with(java.time.temporal.TemporalAdjuster)
java.time.OffsetDateTime#with(java.time.temporal.TemporalField, long)
java.time.OffsetDateTime#withDayOfMonth(int)
java.time.OffsetDateTime#withDayOfYear(int)
java.time.OffsetDateTime#withHour(int)
java.time.OffsetDateTime#withMinute(int)
java.time.OffsetDateTime#withMonth(int)
java.time.OffsetDateTime#withNano(int)
java.time.OffsetDateTime#withOffsetSameInstant(java.time.ZoneOffset)
java.time.OffsetDateTime#withOffsetSameLocal(java.time.ZoneOffset)
java.time.OffsetDateTime#withSecond(int)
java.time.OffsetDateTime#withYear(int)

@defaultMessage Daylight saving is not the only reason for a change in timezone offset.
java.time.zone.ZoneRules#getStandardOffset(java.time.Instant)
java.time.zone.ZoneRules#getDaylightSavings(java.time.Instant)
java.time.zone.ZoneRules#isDaylightSavings(java.time.Instant)
Expand Up @@ -609,13 +609,11 @@ private static Object deepCopy(Object value) {
return Arrays.copyOf(bytes, bytes.length);
} else if (value == null || value instanceof String || value instanceof Integer ||
value instanceof Long || value instanceof Float ||
value instanceof Double || value instanceof Boolean) {
value instanceof Double || value instanceof Boolean ||
value instanceof ZonedDateTime) {
return value;
} else if (value instanceof Date) {
return ((Date) value).clone();
} else if (value instanceof ZonedDateTime) {
ZonedDateTime zonedDateTime = (ZonedDateTime) value;
return ZonedDateTime.of(zonedDateTime.toLocalDate(), zonedDateTime.toLocalTime(), zonedDateTime.getZone());
} else {
throw new IllegalArgumentException("unexpected value type [" + value.getClass() + "]");
}
Expand Down
Expand Up @@ -22,6 +22,8 @@
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
Expand Down Expand Up @@ -986,6 +988,22 @@ public void testCopyConstructor() {
assertIngestDocument(ingestDocument, copy);
}

public void testCopyConstructorWithZonedDateTime() {
ZoneId timezone = ZoneId.of("Europe/London");

Map<String, Object> sourceAndMetadata = new HashMap<>();
sourceAndMetadata.put("beforeClockChange", ZonedDateTime.ofInstant(Instant.ofEpochSecond(1509237000), timezone));
sourceAndMetadata.put("afterClockChange", ZonedDateTime.ofInstant(Instant.ofEpochSecond(1509240600), timezone));

IngestDocument original = new IngestDocument(sourceAndMetadata, new HashMap<>());
IngestDocument copy = new IngestDocument(original);

assertThat(copy.getSourceAndMetadata().get("beforeClockChange"),
equalTo(original.getSourceAndMetadata().get("beforeClockChange")));
assertThat(copy.getSourceAndMetadata().get("afterClockChange"),
equalTo(original.getSourceAndMetadata().get("afterClockChange")));
}

public void testSetInvalidSourceField() throws Exception {
Map<String, Object> document = new HashMap<>();
Object randomObject = randomFrom(new ArrayList<>(), new HashMap<>(), 12, 12.34);
Expand Down

0 comments on commit bb32516

Please sign in to comment.