InstantDeserializer fails to parse negative numeric timestamp strings for pre-1970 values

[advorako]

Encountered while debugging through a project making use of this for Avro serialization/deserialization:

For an object containing a java.time.Instant with associated Avro schema field of "type" : [ "null", { "type" : "string", "java-class" : "java.time.Instant"} ] ...

• A non-null Instant value will serialize into a numeric timestamp string, which combines its epoch seconds and nanoseconds offset member values, separated by a period '.' character.
• The epoch seconds portion may be positive or negative, depending on which side of the Unix Epoch (1970-01-01) it lands on.
• For example, January 1, 1970 at 00:00:03 becomes "3.000000000", while December 31, 1969 at 23:59:57 becomes "-3.000000000".

InstantDeserializer._fromString() is used to try and parse these numeric timestamp strings, but its _countPeriods() helper method does not account for the possible leading '-' character
https://github.com/FasterXML/jackson-modules-java8/blob/2.16/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L369-L384

This causes _fromString() to skip over the parsing method that would work in this situation ...
https://github.com/FasterXML/jackson-modules-java8/blob/2.16/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L409-L410
... and instead fall through into DateTimeFormatter, which cannot handle this type of string and throws a DateTimeParseException
https://github.com/FasterXML/jackson-modules-java8/blob/2.16/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L433

Example code:

public class Example
{
    public static class InstantWrapper
    {
        public Instant time;
        public InstantWrapper() { time = null; }
        public InstantWrapper(final Instant time) { this.time = time; }
    }

    public static void main(final String[] args) throws IOException
    {
        final String instantWrapperSchema = "{\n"
            +"\"type\": \"record\",\n"
            +"\"name\": \"InstantWrapper\",\n"
            +"\"fields\": [\n"
            +"{\"name\": \"time\", \"type\" : [ \"null\", {\"type\" : \"string\", \"java-class\" : \"java.time.Instant\"} ]}\n"
            +"]}";

        final AvroSchema avroSchema = new AvroSchema(new Schema.Parser().setValidate(true).parse(instantWrapperSchema));

        final AvroMapper avroMapper = new AvroMapper();
        avroMapper.registerModule(new JavaTimeModule());

        // post-epoch times work
        final InstantWrapper postEpochWrite = new InstantWrapper(Instant.parse("1970-01-01T12:00:00Z"));
        final byte[] postEpochBytes = avroMapper.writer(avroSchema).writeValueAsBytes(postEpochWrite);
        final InstantWrapper postEpochRead = avroMapper.readerFor(InstantWrapper.class).with(avroSchema).readValue(postEpochBytes);
        System.out.println("post-epoch result: " + postEpochRead.time);

        // pre-epoch times do not
        final InstantWrapper preEpochWrite = new InstantWrapper(Instant.parse("1969-12-31T12:00:00Z"));
        final byte[] preEpochBytes = avroMapper.writer(avroSchema).writeValueAsBytes(preEpochWrite);
        final InstantWrapper preEpochRead = avroMapper.readerFor(InstantWrapper.class).with(avroSchema).readValue(preEpochBytes); // throws exception
        System.out.println("pre-epoch result: " + preEpochRead.time);
    }
}

Output:

post-epoch result: 1970-01-01T12:00:00Z
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java.time.Instant` from String "-43200.000000000": Failed to deserialize java.time.Instant: (java.time.format.DateTimeParseException) Text '-43200.000000000' could not be parsed at index 6 (through reference chain: testprojectgroup.testprojectartifact.Example$InstantWrapper["time"])
	at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:1958)
	at com.fasterxml.jackson.databind.DeserializationContext.handleWeirdStringValue(DeserializationContext.java:1245)
	at com.fasterxml.jackson.datatype.jsr310.deser.JSR310DeserializerBase._handleDateTimeException(JSR310DeserializerBase.java:176)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer._fromString(InstantDeserializer.java:418)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer.deserialize(InstantDeserializer.java:321)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer.deserialize(InstantDeserializer.java:54)
	at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:138)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:310)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:2125)
	at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1603)
	at testprojectgroup.testprojectartifact.Example.main(Example.java:44)
Caused by: java.time.format.DateTimeParseException: Text '-43200.000000000' could not be parsed at index 6
	at java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
	at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1777)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer._fromString(InstantDeserializer.java:412)
	... 9 more

I realize this example may be somewhat of an edge case (if not an outdated, deprecated, and/or ill-advised one), given the documentation's current directions towards the use of AvroJavaTimeModule and a schema using numeric data types for java time classes (which does work), but figured this might be worth logging just in case there might be other means of encountering this problem.

For whatever it may be worth, configuring WRITE_DATES_AS_TIMESTAMPS=false appears to provide a workaround (for newly-serialized data, at least), should one's Avro schema not be easily changeable.

[cowtowncoder]

I think I'd be open to accepting a PR, if you think you could provide one? I agree that it is an edge case, but if special handling would help and would not break main use case, that sounds like a reasonable improvement to me.

[cowtowncoder]

In absence of fix, even PR for reproduction would be useful.
Marking test-needed to indicate we'd need JAva 8 module - only test, outside of Avro module (since this module cannot have dep to Avro module, or vice versa).

[cowtowncoder]

Fixed via #362 for 2.18.4