FasterXML / jackson-datatypes-collections

Jackson project that contains various collection-oriented datatype libraries: Eclipse Collections, Guava, HPPC, PCollections
Apache License 2.0
77 stars 52 forks source link

guava and/or hppc test failures if you use guava 33 #148

Closed pjfanning closed 1 month ago

pjfanning commented 5 months ago

see https://github.com/FasterXML/jackson-datatypes-collections/pull/147

com.fasterxml.jackson.databind.exc.MismatchedInputException: Guava `Collection` of type `com.google.common.collect.ImmutableSortedMultiset<java.lang.String>` does not accept `null` values
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 2]
    at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
    at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1767)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1541)
    at com.fasterxml.jackson.datatype.guava.deser.GuavaImmutableCollectionDeserializer._tryToAddNull(GuavaImmutableCollectionDeserializer.java:1[21](https://github.com/FasterXML/jackson-datatypes-collections/pull/147/checks#step:4:22))
    at com.fasterxml.jackson.datatype.guava.deser.GuavaImmutableCollectionDeserializer._deserializeContents(GuavaImmutableCollectionDeserializer.java:74)
    at com.fasterxml.jackson.datatype.guava.deser.GuavaImmutableCollectionDeserializer._deserializeContents(GuavaImmutableCollectionDeserializer.java:18)
    at com.fasterxml.jackson.datatype.guava.deser.GuavaCollectionDeserializer.deserialize(GuavaCollectionDeserializer.java:138)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4905)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3848)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3831)
    at com.fasterxml.jackson.datatype.guava.fuzz.Fuzz124_64610Test.lambda$testOSSFuzzIssue64610$0(Fuzz124_64610Test.java:27)
    at org.junit.Assert.assertThrows(Assert.java:1001)
    at org.junit.Assert.assertThrows(Assert.java:981)
    at com.fasterxml.jackson.datatype.guava.fuzz.Fuzz124_64610Test.testOSSFuzzIssue64610(Fuzz124_64610Test.java:25)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at junit.framework.TestCase.runTest(TestCase.java:177)
    at junit.framework.TestCase.runBare(TestCase.java:142)
    at junit.framework.TestResult$1.protect(TestResult.java:1[22](https://github.com/FasterXML/jackson-datatypes-collections/pull/147/checks#step:4:23))
    at junit.framework.TestResult.runProtected(TestResult.java:142)
    at junit.framework.TestResult.run(TestResult.java:125)
    at junit.framework.TestCase.run(TestCase.java:130)
    at junit.framework.TestSuite.runTest(TestSuite.java:241)
    at junit.framework.TestSuite.run(TestSuite.java:[23](https://github.com/FasterXML/jackson-datatypes-collections/pull/147/checks#step:4:24)6)
    at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
    at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
Error:    TestContainerSerializers.testObjectContainerSerializer:152 » JsonMapping Class `java.lang.String` not subtype of `com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<com.carrotsearch.hppc.cursors.ObjectCursor<java.lang.Object>>>>>>>>>>>>>>>` (through reference chain: java.lang.String[0])
cowtowncoder commented 5 months ago

Yeah I noticed there was an odd new failure -- not sure it's even related to Guava 33. Will need to figure it out once I get home (traveling for work this week).

EDIT: fails 2.18 even without Guava upgrade. No idea why, very odd.

cowtowncoder commented 5 months ago

@pjfanning I figured out the root cause for regression; databind issue https://github.com/FasterXML/jackson-databind/issues/4443. I could not quite figured out how to make that work so reverted it from 2.18 (since it is not 100% necessary change and can be done at a later point if and when finding out how to make it work).

I also filed #149 to test against multiple Guava versions, so that even when we upgrade "suggested" version -- one for which module declares dependency, module works on wider range of versions. Similar testing is already done for Joda.

With that we can increase the baseline, but I think there are some risk (... of getting lots of complaints for "breaking our Guava use" from users that rely on default Guava module brings in if there are no overrides) with going to the latest. Looking at:

https://mvnrepository.com/artifact/com.google.guava/guava

it looks like 32.0.1-jre is the oldest one with no CVEs, so maybe go with that as baseline?

Although I guess that as long as:

  1. We have CI checking for compatibility with older versions (down to whatever we decide is to be supported)
  2. Guava version we default to still works with JDK 8

I am not as much against bigger leap.

cowtowncoder commented 5 months ago

Ok, I noticed that guava/README.md (https://github.com/FasterXML/jackson-datatypes-collections/tree/2.18/guava) did not have any information for latest Jackson versions' compatibility, so went back and verified 2.16, 2.17 and 2.18.

Basically:

so those are probably the limits to verify. I don't think we really have to support versions before, say, 25.1, but until there's a reason to break compatibility might as well not do that.

cowtowncoder commented 1 month ago

I think this was resolved, WDYT @pjfanning ?

pjfanning commented 1 month ago

ok - https://github.com/FasterXML/jackson-datatypes-collections/pull/152 should cover this