diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index f0506727b..e5f19416a 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -17,6 +17,9 @@ Contributors: # 2.19.0 (not yet released) +WrongWrong (@k163377) +* #937: Added type match check to read functions + Tatu Saloranta (@cowtowncoder) * #889: Upgrade kotlin dep to 1.9.25 (from 1.9.24) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index bdafec63d..7cd03965c 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -16,6 +16,14 @@ Co-maintainers: === Releases === ------------------------------------------------------------------------ +2.19.0 (not yet released) + +#937: For `readValue` and other shorthands for `ObjectMapper` deserialization methods, + type consistency checks have been added. + A `RuntimeJsonMappingException` will be thrown in case of inconsistency. + This fixes a problem that broke `Kotlin` null safety by reading null as a value even if the type parameter was specified as non-null. + It also checks for custom errors in ObjectMapper that cause a different value to be read than the specified type parameter. + 2.19.0-rc2 (07-Apr-2025) #929: Added consideration of `JsonProperty.isRequired` added in `2.19` in `hasRequiredMarker` processing. diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt index 2a8c3b1a2..522694331 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt @@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.JsonSerializer import com.fasterxml.jackson.databind.MappingIterator import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.ObjectReader +import com.fasterxml.jackson.databind.RuntimeJsonMappingException import com.fasterxml.jackson.databind.json.JsonMapper import com.fasterxml.jackson.databind.module.SimpleModule import com.fasterxml.jackson.databind.node.ArrayNode @@ -50,21 +51,132 @@ fun ObjectMapper.registerKotlinModule(initializer: KotlinModule.Builder.() -> Un inline fun jacksonTypeRef(): TypeReference = object: TypeReference() {} +/** + * It is public due to Kotlin restrictions, but should not be used externally. + */ +inline fun Any?.checkTypeMismatch(): T { + // Basically, this check assumes that T is non-null and the value is null. + // Since this can be caused by both input or ObjectMapper implementation errors, + // a more abstract RuntimeJsonMappingException is thrown. + if (this !is T) { + val nullability = if (null is T) "?" else "(non-null)" + + // Since the databind implementation of MappingIterator throws RuntimeJsonMappingException, + // JsonMappingException was not used to unify the behavior. + throw RuntimeJsonMappingException( + "Deserialized value did not match the specified type; " + + "specified ${T::class.qualifiedName}${nullability} but was ${this?.let { it::class.qualifiedName }}" + ) + } + return this +} + +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.readValue(jp: JsonParser): T = readValue(jp, jacksonTypeRef()) -inline fun ObjectMapper.readValues(jp: JsonParser): MappingIterator = readValues(jp, jacksonTypeRef()) + .checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValues]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ +inline fun ObjectMapper.readValues(jp: JsonParser): MappingIterator { + val values = readValues(jp, jacksonTypeRef()) + + return object : MappingIterator(values) { + override fun nextValue(): T = super.nextValue().checkTypeMismatch() + } +} -inline fun ObjectMapper.readValue(src: File): T = readValue(src, jacksonTypeRef()) -inline fun ObjectMapper.readValue(src: URL): T = readValue(src, jacksonTypeRef()) +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ +inline fun ObjectMapper.readValue(src: File): T = readValue(src, jacksonTypeRef()).checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ +inline fun ObjectMapper.readValue(src: URL): T = readValue(src, jacksonTypeRef()).checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.readValue(content: String): T = readValue(content, jacksonTypeRef()) -inline fun ObjectMapper.readValue(src: Reader): T = readValue(src, jacksonTypeRef()) + .checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ +inline fun ObjectMapper.readValue(src: Reader): T = readValue(src, jacksonTypeRef()).checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.readValue(src: InputStream): T = readValue(src, jacksonTypeRef()) + .checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.readValue(src: ByteArray): T = readValue(src, jacksonTypeRef()) - + .checkTypeMismatch() + +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.treeToValue(n: TreeNode): T = readValue(this.treeAsTokens(n), jacksonTypeRef()) + .checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.convertValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectMapper.convertValue(from: Any?): T = convertValue(from, jacksonTypeRef()) - + .checkTypeMismatch() + +/** + * Shorthand for [ObjectMapper.readValue]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ inline fun ObjectReader.readValueTyped(jp: JsonParser): T = readValue(jp, jacksonTypeRef()) -inline fun ObjectReader.readValuesTyped(jp: JsonParser): Iterator = readValues(jp, jacksonTypeRef()) + .checkTypeMismatch() +/** + * Shorthand for [ObjectMapper.readValues]. + * @throws RuntimeJsonMappingException Especially if [T] is non-null and the value read is null. + * Other cases where the read value is of a different type than [T] + * due to an incorrect customization to [ObjectMapper]. + */ +inline fun ObjectReader.readValuesTyped(jp: JsonParser): Iterator { + val values = readValues(jp, jacksonTypeRef()) + + return object : Iterator by values { + override fun next(): T = values.next().checkTypeMismatch() + } +} inline fun ObjectReader.treeToValue(n: TreeNode): T? = readValue(this.treeAsTokens(n), jacksonTypeRef()) inline fun ObjectMapper.addMixIn(): ObjectMapper = this.addMixIn(T::class.java, U::class.java) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValueTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValueTest.kt new file mode 100644 index 000000000..777efbaec --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValueTest.kt @@ -0,0 +1,89 @@ +package com.fasterxml.jackson.module.kotlin + +import com.fasterxml.jackson.databind.RuntimeJsonMappingException +import com.fasterxml.jackson.databind.node.NullNode +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.io.StringReader + +class ReadValueTest { + @Nested + inner class CheckTypeMismatchTest { + @Test + fun jsonParser() { + val src = defaultMapper.createParser("null") + assertThrows { + defaultMapper.readValue(src) + }.printStackTrace() + } + + @Test + fun file() { + val src = createTempJson("null") + assertThrows { + defaultMapper.readValue(src) + } + } + + // Not implemented because a way to test without mocks was not found + // @Test + // fun url() { + // } + + @Test + fun string() { + val src = "null" + assertThrows { + defaultMapper.readValue(src) + } + } + + @Test + fun reader() { + val src = StringReader("null") + assertThrows { + defaultMapper.readValue(src) + } + } + + @Test + fun inputStream() { + val src = "null".byteInputStream() + assertThrows { + defaultMapper.readValue(src) + } + } + + @Test + fun byteArray() { + val src = "null".toByteArray() + assertThrows { + defaultMapper.readValue(src) + } + } + + @Test + fun treeToValueTreeNode() { + assertThrows { + defaultMapper.treeToValue(NullNode.instance) + } + } + + @Test + fun convertValueAny() { + assertThrows { + defaultMapper.convertValue(null) + } + } + + @Test + fun readValueTypedJsonParser() { + val reader = defaultMapper.reader() + val src = reader.createParser("null") + assertThrows { + reader.readValueTyped(src) + } + } + } +} diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValuesTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValuesTest.kt new file mode 100644 index 000000000..2d2aa10b5 --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValuesTest.kt @@ -0,0 +1,65 @@ +package com.fasterxml.jackson.module.kotlin + +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.RuntimeJsonMappingException +import com.fasterxml.jackson.databind.deser.std.StdDeserializer +import com.fasterxml.jackson.databind.module.SimpleModule +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import kotlin.test.assertEquals + +class ReadValuesTest { + class MyStrDeser : StdDeserializer(String::class.java) { + override fun deserialize( + p: JsonParser, + ctxt: DeserializationContext + ): String? = p.valueAsString.takeIf { it != "bar" } + } + + @Nested + inner class CheckTypeMismatchTest { + val mapper = jacksonObjectMapper().registerModule( + object : SimpleModule() { + init { + addDeserializer(String::class.java, MyStrDeser()) + } + } + )!! + + @Test + fun readValuesJsonParserNext() { + val src = mapper.createParser(""""foo"${"\n"}"bar"""") + val itr = mapper.readValues(src) + + assertEquals("foo", itr.next()) + assertThrows { + itr.next() + } + } + + @Test + fun readValuesJsonParserNextValue() { + val src = mapper.createParser(""""foo"${"\n"}"bar"""") + val itr = mapper.readValues(src) + + assertEquals("foo", itr.nextValue()) + assertThrows { + itr.nextValue() + } + } + + @Test + fun readValuesTypedJsonParser() { + val reader = mapper.reader() + val src = reader.createParser(""""foo"${"\n"}"bar"""") + val itr = reader.readValuesTyped(src) + + assertEquals("foo", itr.next()) + assertThrows { + itr.next() + } + } + } +} diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt index 3bd3e5bd8..327fc3a55 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt @@ -5,6 +5,10 @@ import com.fasterxml.jackson.core.util.DefaultIndenter import com.fasterxml.jackson.core.util.DefaultPrettyPrinter import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.ObjectWriter +import java.io.File +import java.io.FileOutputStream +import java.io.OutputStreamWriter +import java.nio.charset.StandardCharsets import kotlin.reflect.KParameter import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor @@ -30,3 +34,16 @@ internal inline fun assertReflectEquals(expected: T, actual: T assertEquals(it.get(expected), it.get(actual)) } } + +internal fun createTempJson(json: String): File { + val file = File.createTempFile("temp", ".json") + file.deleteOnExit() + OutputStreamWriter( + FileOutputStream(file), + StandardCharsets.UTF_8 + ).use { writer -> + writer.write(json) + writer.flush() + } + return file +} diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/WithoutCustomDeserializeMethodTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/WithoutCustomDeserializeMethodTest.kt index 0423d2125..ab5709599 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/WithoutCustomDeserializeMethodTest.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/WithoutCustomDeserializeMethodTest.kt @@ -9,6 +9,7 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.Test import java.lang.reflect.InvocationTargetException +import kotlin.test.assertNotEquals class WithoutCustomDeserializeMethodTest { companion object { @@ -42,10 +43,8 @@ class WithoutCustomDeserializeMethodTest { // failing @Test fun nullString() { - org.junit.jupiter.api.assertThrows("#209 has been fixed.") { - val result = defaultMapper.readValue("null") - assertEquals(NullableObject(null), result) - } + val result = defaultMapper.readValue("null") + assertNotEquals(NullableObject(null), result, "kogera #209 has been fixed.") } } } diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/deserializer/SpecifiedForObjectMapperTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/deserializer/SpecifiedForObjectMapperTest.kt index c57f41596..643883a74 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/deserializer/SpecifiedForObjectMapperTest.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/deserializer/SpecifiedForObjectMapperTest.kt @@ -9,6 +9,7 @@ import com.fasterxml.jackson.module.kotlin.kogeraIntegration.deser.valueClass.Pr import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import kotlin.test.assertNotEquals class SpecifiedForObjectMapperTest { companion object { @@ -48,10 +49,8 @@ class SpecifiedForObjectMapperTest { // failing @Test fun nullString() { - org.junit.jupiter.api.assertThrows("#209 has been fixed.") { - val result = mapper.readValue("null") - assertEquals(NullableObject("null-value-deser"), result) - } + val result = mapper.readValue("null") + assertNotEquals(NullableObject("null-value-deser"), result, "kogera #209 has been fixed.") } } }