From b2bd568f77a7e58fef4159dfa6d3de1446e4884d Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 31 Oct 2025 17:08:06 +0500 Subject: [PATCH 1/3] Also check collisions for JsonNamingStrategy-transformed names during serialization --- .../serialization/features/JsonNamingStrategyTest.kt | 5 +++++ .../kotlinx/serialization/json/internal/JsonNamesMap.kt | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt index 9e2cf1d4f7..d12b1b0b2d 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt @@ -182,6 +182,11 @@ class JsonNamingStrategyTest : JsonTestBase() { val json = Json(jsonWithNaming) { ignoreUnknownKeys = true } + parametrizedTest { mode -> + assertFailsWithMessage("The transformed name 'test_case' for property test_case already exists") { + json.encodeToString(CollisionCheckPrimary("a", "b")) + } + } parametrizedTest { mode -> assertFailsWithMessage("The suggested name 'test_case' for property test_case is already one of the names for property testCase") { json.decodeFromString("""{"test_case":"a"}""", mode) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt index 8a4a5865dd..857775f0b2 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt @@ -55,9 +55,15 @@ internal fun Json.deserializationNamesMap(descriptor: SerialDescriptor): Map = json.schemaCache.getOrPut(this, JsonSerializationNamesKey) { + val trackingSet = mutableSetOf() Array(elementsCount) { i -> val baseName = getElementName(i) - strategy.serialNameForJson(this, i, baseName) + val name = strategy.serialNameForJson(this, i, baseName) + if (!trackingSet.add(name)) throw JsonException( + "The transformed name '$name' for property $baseName already exists " + + "in ${this@serializationNamesIndices}" + ) + name } } From 915d58ec9ca766e15c86d3dd04378ee3d5ebbd11 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 31 Oct 2025 17:51:43 +0500 Subject: [PATCH 2/3] Disallow more conflicts with class discriminator on encoding: - from JsonNamingStrategy - from ALL_JSON_OBJECTS - from default polymorphic serializer Changed the type of thrown exception to SerializationException because it is our contract for .encodeToString. Stop recommending array polymorphism because it is outdated. --- .../DefaultPolymorphicSerializerTest.kt | 14 +++++++++ .../features/JsonNamingStrategyTest.kt | 25 +++++++++++++++ .../JsonClassDiscriminatorModeBaseTest.kt | 6 +++- .../JsonClassDiscriminatorModeTest.kt | 13 ++++++++ .../SerialNameCollisionInSealedClassesTest.kt | 4 +-- .../json/internal/JsonNamesMap.kt | 11 +++++-- .../json/internal/Polymorphic.kt | 31 ++++++++++--------- 7 files changed, 83 insertions(+), 21 deletions(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/DefaultPolymorphicSerializerTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/DefaultPolymorphicSerializerTest.kt index 9d35a29040..d4d51fd84d 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/features/DefaultPolymorphicSerializerTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/DefaultPolymorphicSerializerTest.kt @@ -6,6 +6,7 @@ package kotlinx.serialization.features import kotlinx.serialization.* import kotlinx.serialization.json.* import kotlinx.serialization.modules.* +import kotlinx.serialization.test.assertFailsWithMessage import kotlin.test.* class DefaultPolymorphicSerializerTest : JsonTestBase() { @@ -33,4 +34,17 @@ class DefaultPolymorphicSerializerTest : JsonTestBase() { json.decodeFromString(""" {"type":"unknown","name":"example"}""", it)) } + @Test + fun defaultSerializerConflictWithDiscriminatorNotAllowed() { + @Suppress("UNCHECKED_CAST") val module = SerializersModule { + polymorphicDefaultSerializer(Project::class) { + DefaultProject.serializer() as KSerializer + } + } + val j = Json { serializersModule = module } + assertFailsWithMessage("Class 'kotlinx.serialization.features.DefaultPolymorphicSerializerTest.DefaultProject' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 'type'.") { + j.encodeToString(DefaultProject("example", "custom")) + } + } + } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt index d12b1b0b2d..292e461f40 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt @@ -214,6 +214,7 @@ class JsonNamingStrategyTest : JsonTestBase() { } @Serializable + @SerialName("SealedBase") sealed interface SealedBase { @Serializable @JsonClassDiscriminator("typeSub") @@ -244,4 +245,28 @@ class JsonNamingStrategyTest : JsonTestBase() { json ) } + + @Test + fun testClashWithDiscriminator() { + val correctJson = Json(jsonWithNaming) { + classDiscriminator = "test_base" + } + val holder = Holder(SealedBase.SealedSub2(), SealedBase.SealedMid.SealedSub1) + + // Should pass because same name is only on different levels + assertJsonFormAndRestored( + Holder.serializer(), + holder, + """{"test_base":{"test_base":"SealedSub2","test_case":0},"test_mid":{"typeSub":"SealedSub1"}}""", + correctJson + ) + + val incorrectJson = Json(jsonWithNaming) { + classDiscriminator = "test_case" + } + + assertFailsWithMessage("Class 'SealedSub2' cannot be serialized as base class 'SealedBase' because it has property name that conflicts with JSON class discriminator 'test_case'.") { + incorrectJson.encodeToString(holder) + } + } } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeBaseTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeBaseTest.kt index 8fcd549977..51a1ab0499 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeBaseTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeBaseTest.kt @@ -80,7 +80,7 @@ abstract class JsonClassDiscriminatorModeBaseTest( @SerialName("mixed") data class MixedPolyAndRegular(val sb: SealedBase, val sc: SealedContainer, val i: Inner) - private inline fun doTest(expected: String, obj: T) { + internal inline fun doTest(expected: String, obj: T) { parametrizedTest { mode -> val serialized = json.encodeToString(serializer(), obj, mode) assertEquals(expected, serialized, "Failed with mode = $mode") @@ -150,4 +150,8 @@ abstract class JsonClassDiscriminatorModeBaseTest( val nm = NullableMixed(null, null) doTest(expected, nm) } + + @Serializable + @SerialName("Conflict") + class Conflict(val type: String) } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt index d55e7559ed..c8578b07c6 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt @@ -7,6 +7,7 @@ package kotlinx.serialization.json.polymorphic import kotlinx.serialization.* import kotlinx.serialization.json.* import kotlinx.serialization.modules.* +import kotlinx.serialization.test.assertFailsWithMessage import kotlin.test.* class ClassDiscriminatorModeAllObjectsTest : @@ -46,6 +47,13 @@ class ClassDiscriminatorModeAllObjectsTest : @Test fun testNullable() = testNullable("""{"type":"NullableMixed","sb":null,"sc":null}""") + @Test + fun testConflictWithDiscriminator() { + assertFailsWithMessage("Class 'Conflict' cannot be serialized in ALL_JSON_OBJECTS class discriminator mode because it has property name that conflicts with JSON class discriminator 'type'") { + json.encodeToString(Conflict("foo")) + } + } + } class ClassDiscriminatorModeNoneTest : @@ -83,6 +91,11 @@ class ClassDiscriminatorModeNoneTest : @Test fun testNullable() = testNullable("""{"sb":null,"sc":null}""") + @Test + fun testConflictWithDiscriminator() { + doTest("""{"type":"foo"}""", Conflict("foo")) + } + interface CommandType @Serializable // For Kotlin/JS diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionInSealedClassesTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionInSealedClassesTest.kt index 18a49a5252..9ddeb683ea 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionInSealedClassesTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionInSealedClassesTest.kt @@ -24,8 +24,8 @@ class SerialNameCollisionInSealedClassesTest { @Test fun testCollisionWithDiscriminator() { - assertFailsWith { Json("type").encodeToString(Base.serializer(), Base.Child("a")) } - assertFailsWith { Json("type2").encodeToString(Base.serializer(), Base.Child("a")) } + assertFailsWith { Json("type").encodeToString(Base.serializer(), Base.Child("a")) } + assertFailsWith { Json("type2").encodeToString(Base.serializer(), Base.Child("a")) } Json("f").encodeToString(Base.serializer(), Base.Child("a")) } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt index 857775f0b2..a49f91e8be 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt @@ -8,8 +8,8 @@ package kotlinx.serialization.json.internal import kotlinx.serialization.* import kotlinx.serialization.descriptors.* import kotlinx.serialization.encoding.* +import kotlinx.serialization.internal.jsonCachedSerialNames import kotlinx.serialization.json.* -import kotlin.native.concurrent.* internal val JsonDeserializationNamesKey = DescriptorSchemaCache.Key>() @@ -19,7 +19,7 @@ private fun SerialDescriptor.buildDeserializationNamesMap(json: Json): Map.putOrThrow(name: String, index: Int) { val entity = if (kind == SerialKind.ENUM) "enum value" else "property" if (name in this) { - throw JsonException( + throw JsonDecodingException( "The suggested name '$name' for $entity ${getElementName(index)} is already one of the names for $entity " + "${getElementName(getValue(name))} in ${this@buildDeserializationNamesMap}" ) @@ -72,6 +72,12 @@ internal fun SerialDescriptor.getJsonElementName(json: Json, index: Int): String return if (strategy == null) getElementName(index) else serializationNamesIndices(json, strategy)[index] } +// Emits only names used for encoding, i.e. from naming strategy, but not from @JsonNames +internal fun SerialDescriptor.getJsonEncodedNames(json: Json): Set { + val strategy = namingStrategy(json) + return if (strategy == null) jsonCachedSerialNames() else serializationNamesIndices(json, strategy).toSet() +} + internal fun SerialDescriptor.namingStrategy(json: Json) = if (kind == StructureKind.CLASS) json.configuration.namingStrategy else null @@ -85,7 +91,6 @@ private fun Json.decodeCaseInsensitive(descriptor: SerialDescriptor) = * Serves same purpose as [SerialDescriptor.getElementIndex] but respects [JsonNames] annotation * and [JsonConfiguration] settings. */ -@OptIn(ExperimentalSerializationApi::class) internal fun SerialDescriptor.getJsonNameIndex(json: Json, name: String): Int { if (json.decodeCaseInsensitive(this)) { return getJsonNameIndexSlowPath(json, name.lowercase()) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt index 26d752661f..87ff7f8f82 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt @@ -9,8 +9,6 @@ import kotlinx.serialization.* import kotlinx.serialization.descriptors.* import kotlinx.serialization.internal.* import kotlinx.serialization.json.* -import kotlinx.serialization.modules.* -import kotlin.jvm.* @Suppress("UNCHECKED_CAST") internal inline fun JsonEncoder.encodePolymorphically( @@ -37,32 +35,35 @@ internal inline fun JsonEncoder.encodePolymorphically( val casted = serializer as AbstractPolymorphicSerializer requireNotNull(value) { "Value for serializer ${serializer.descriptor} should always be non-null. Please report issue to the kotlinx.serialization tracker." } val actual = casted.findPolymorphicSerializer(this, value) - if (baseClassDiscriminator != null) { - validateIfSealed(serializer, actual, baseClassDiscriminator) - checkKind(actual.descriptor.kind) - } actual as SerializationStrategy } else serializer - if (baseClassDiscriminator != null) ifPolymorphic(baseClassDiscriminator, actualSerializer.descriptor.serialName) + if (baseClassDiscriminator != null) { + json.checkEncodingConflicts(serializer, actualSerializer, baseClassDiscriminator) + checkKind(actualSerializer.descriptor.kind) + ifPolymorphic(baseClassDiscriminator, actualSerializer.descriptor.serialName) + } actualSerializer.serialize(this, value) } -private fun validateIfSealed( +private fun Json.checkEncodingConflicts( serializer: SerializationStrategy<*>, actualSerializer: SerializationStrategy<*>, classDiscriminator: String ) { - if (serializer !is SealedClassSerializer<*>) return - @Suppress("DEPRECATION_ERROR") - if (classDiscriminator in actualSerializer.descriptor.jsonCachedSerialNames()) { + if (classDiscriminator in actualSerializer.descriptor.getJsonEncodedNames(this)) { val baseName = serializer.descriptor.serialName val actualName = actualSerializer.descriptor.serialName - error( - "Sealed class '$actualName' cannot be serialized as base class '$baseName' because" + + + val text = when { + configuration.classDiscriminatorMode == ClassDiscriminatorMode.ALL_JSON_OBJECTS && baseName == actualName -> "in ALL_JSON_OBJECTS class discriminator mode" + else -> "as base class '$baseName'" + } + throw JsonEncodingException( + "Class '$actualName' cannot be serialized $text because" + " it has property name that conflicts with JSON class discriminator '$classDiscriminator'. " + - "You can either change class discriminator in JsonConfiguration, " + - "rename property with @SerialName annotation or fall back to array polymorphism" + "You can either change class discriminator in JsonConfiguration, or " + + "rename property with @SerialName annotation." ) } } From 94eba9ce497671cdaced58b2bf4184c0db98b79f Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Tue, 4 Nov 2025 19:57:30 +0700 Subject: [PATCH 3/3] Unify Polymorphic and Sealed class serializers check: now it is allowed to have a conflicting name on deserialization, conflicts are reported only during encoding. --- .../features/JsonClassDiscriminatorTest.kt | 18 +++++++++++ .../modules/SerialNameCollisionTest.kt | 30 ++++++++++++++----- .../JsonSerializersModuleValidator.kt | 22 -------------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonClassDiscriminatorTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonClassDiscriminatorTest.kt index 5eebe2189f..5e33deb33f 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonClassDiscriminatorTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonClassDiscriminatorTest.kt @@ -8,6 +8,7 @@ import kotlinx.serialization.* import kotlinx.serialization.builtins.* import kotlinx.serialization.json.* import kotlinx.serialization.modules.* +import kotlinx.serialization.test.assertFailsWithMessage import kotlin.test.* class JsonClassDiscriminatorTest : JsonTestBase() { @@ -110,4 +111,21 @@ class JsonClassDiscriminatorTest : JsonTestBase() { json ) } + + @Serializable + @JsonClassDiscriminator("type2") + @SerialName("Foo") + sealed interface Foo + + @Serializable + @SerialName("FooImpl") + data class FooImpl(val type2: String) : Foo + + @Test + fun testCannotHaveConflictWithJsonClassDiscriminator() { + assertFailsWithMessage("Class 'FooImpl' cannot be serialized as base class 'Foo' because it has property name that conflicts with JSON class discriminator 'type2'") { + Json.encodeToString( FooImpl("foo")) + } + } + } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionTest.kt index 4a88cb120f..6b14f12cbd 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionTest.kt @@ -6,6 +6,7 @@ package kotlinx.serialization.modules import kotlinx.serialization.* import kotlinx.serialization.json.* +import kotlinx.serialization.test.assertFailsWithMessage import kotlin.test.* private const val prefix = "kotlinx.serialization.modules.SerialNameCollisionTest" @@ -34,7 +35,6 @@ class SerialNameCollisionTest { classDiscriminator = discriminator this.useArrayPolymorphism = useArrayPolymorphism serializersModule = context - } @Test @@ -45,9 +45,15 @@ class SerialNameCollisionTest { } } - assertFailsWith { Json("type", module) } - assertFailsWith { Json("type2", module) } - Json("type3", module) // OK + assertFailsWithMessage("Class 'kotlinx.serialization.modules.SerialNameCollisionTest.Derived' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 'type'.") { + Json("type", module).encodeToString(Derived("foo", "bar")) + } + assertFailsWithMessage("Class 'kotlinx.serialization.modules.SerialNameCollisionTest.Derived' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 'type2'.") { + Json("type2", module).encodeToString(Derived("foo", "bar")) + } + assertEquals("{\"type3\":\"kotlinx.serialization.modules.SerialNameCollisionTest.Derived\",\"type\":\"foo\",\"type2\":\"bar\"}", + Json("type3", module).encodeToString(Derived("foo", "bar")) + ) } @Test @@ -68,10 +74,18 @@ class SerialNameCollisionTest { } } - assertFailsWith { Json("type", module) } - assertFailsWith { Json("type2", module) } - assertFailsWith { Json("t3", module) } - Json("t4", module) // OK + assertFailsWithMessage("Class 'kotlinx.serialization.modules.SerialNameCollisionTest.DerivedCustomized' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 'type'.") { + Json("type", module).encodeToString(DerivedCustomized("foo", "bar", "t3")) + } + assertFailsWithMessage("Class 'kotlinx.serialization.modules.SerialNameCollisionTest.DerivedCustomized' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 'type2'.") { + Json("type2", module).encodeToString(DerivedCustomized("foo", "bar", "t3")) + } + assertFailsWithMessage("Class 'kotlinx.serialization.modules.SerialNameCollisionTest.DerivedCustomized' cannot be serialized as base class 'kotlinx.serialization.Polymorphic' because it has property name that conflicts with JSON class discriminator 't3'.") { + Json("t3", module).encodeToString(DerivedCustomized("foo", "bar", "t3")) + } + assertEquals("{\"t4\":\"kotlinx.serialization.modules.SerialNameCollisionTest.DerivedCustomized\",\"type\":\"foo\",\"type2\":\"bar\",\"t3\":\"t3\"}", + Json("t4", module).encodeToString(DerivedCustomized("foo", "bar", "t3")) + ) } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt index 0b00f9da28..c3c0b0fdef 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt @@ -15,7 +15,6 @@ internal class JsonSerializersModuleValidator( configuration: JsonConfiguration, ) : SerializersModuleCollector { - private val discriminator: String = configuration.classDiscriminator private val useArrayPolymorphism: Boolean = configuration.useArrayPolymorphism private val isDiscriminatorRequired = configuration.classDiscriminatorMode != ClassDiscriminatorMode.NONE @@ -33,10 +32,6 @@ internal class JsonSerializersModuleValidator( ) { val descriptor = actualSerializer.descriptor checkKind(descriptor, actualClass) - if (!useArrayPolymorphism && isDiscriminatorRequired) { - // Collisions with "type" can happen only for JSON polymorphism - checkDiscriminatorCollisions(descriptor, actualClass) - } } private fun checkKind(descriptor: SerialDescriptor, actualClass: KClass<*>) { @@ -62,23 +57,6 @@ internal class JsonSerializersModuleValidator( } } - private fun checkDiscriminatorCollisions( - descriptor: SerialDescriptor, - actualClass: KClass<*> - ) { - for (i in 0 until descriptor.elementsCount) { - val name = descriptor.getElementName(i) - if (name == discriminator) { - throw IllegalArgumentException( - "Polymorphic serializer for $actualClass has property '$name' that conflicts " + - "with JSON class discriminator. You can either change class discriminator in JsonConfiguration, " + - "rename property with @SerialName annotation " + - "or fall back to array polymorphism" - ) - } - } - } - override fun polymorphicDefaultSerializer( baseClass: KClass, defaultSerializerProvider: (value: Base) -> SerializationStrategy?