From e2945f23cff285d119bf59cd25a148607a869739 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Tue, 7 May 2024 08:20:32 -0400 Subject: [PATCH 1/3] Add ability to disallow repeated keys in CBOR Fixes https://github.com/Kotlin/kotlinx.serialization/issues/2662 by adding a `visitKey` method to `CompositeDecoder`; map and set serializers should call this so that decoders have an opportunity to throw an error when a duplicate key is detected. A new config option `Cbor.allowDuplicateKeys` can be set to false to enable this new behavior. This can form the basis of a Strict Mode in the future. Also fixes a typo in an unrelated method docstring. --- core/api/kotlinx-serialization-core.api | 8 ++++++++ core/api/kotlinx-serialization-core.klib.api | 4 ++++ .../serialization/SerializationExceptions.kt | 7 +++++++ .../serialization/encoding/Decoding.kt | 14 ++++++++++++- .../internal/CollectionSerializers.kt | 1 + .../cbor/api/kotlinx-serialization-cbor.api | 4 +++- .../api/kotlinx-serialization-cbor.klib.api | 5 ++++- .../src/kotlinx/serialization/cbor/Cbor.kt | 20 +++++++++++++++---- .../serialization/cbor/internal/Encoding.kt | 15 +++++++++++++- .../serialization/cbor/CborStrictModeTest.kt | 20 +++++++++++++++++++ .../json/api/kotlinx-serialization-json.api | 1 + 11 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt diff --git a/core/api/kotlinx-serialization-core.api b/core/api/kotlinx-serialization-core.api index be7625f3cd..db026f1541 100644 --- a/core/api/kotlinx-serialization-core.api +++ b/core/api/kotlinx-serialization-core.api @@ -19,6 +19,10 @@ public abstract interface class kotlinx/serialization/DeserializationStrategy { public abstract fun getDescriptor ()Lkotlinx/serialization/descriptors/SerialDescriptor; } +public final class kotlinx/serialization/DuplicateMapKeyException : kotlinx/serialization/SerializationException { + public fun (Ljava/lang/Object;)V +} + public abstract interface annotation class kotlinx/serialization/EncodeDefault : java/lang/annotation/Annotation { public abstract fun mode ()Lkotlinx/serialization/EncodeDefault$Mode; } @@ -384,6 +388,7 @@ public abstract class kotlinx/serialization/encoding/AbstractDecoder : kotlinx/s public final fun decodeStringElement (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String; public fun decodeValue ()Ljava/lang/Object; public fun endStructure (Lkotlinx/serialization/descriptors/SerialDescriptor;)V + public fun visitKey (Ljava/lang/Object;)V } public abstract class kotlinx/serialization/encoding/AbstractEncoder : kotlinx/serialization/encoding/CompositeEncoder, kotlinx/serialization/encoding/Encoder { @@ -448,6 +453,7 @@ public abstract interface class kotlinx/serialization/encoding/CompositeDecoder public abstract fun decodeStringElement (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String; public abstract fun endStructure (Lkotlinx/serialization/descriptors/SerialDescriptor;)V public abstract fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule; + public abstract fun visitKey (Ljava/lang/Object;)V } public final class kotlinx/serialization/encoding/CompositeDecoder$Companion { @@ -460,6 +466,7 @@ public final class kotlinx/serialization/encoding/CompositeDecoder$DefaultImpls public static synthetic fun decodeNullableSerializableElement$default (Lkotlinx/serialization/encoding/CompositeDecoder;Lkotlinx/serialization/descriptors/SerialDescriptor;ILkotlinx/serialization/DeserializationStrategy;Ljava/lang/Object;ILjava/lang/Object;)Ljava/lang/Object; public static fun decodeSequentially (Lkotlinx/serialization/encoding/CompositeDecoder;)Z public static synthetic fun decodeSerializableElement$default (Lkotlinx/serialization/encoding/CompositeDecoder;Lkotlinx/serialization/descriptors/SerialDescriptor;ILkotlinx/serialization/DeserializationStrategy;Ljava/lang/Object;ILjava/lang/Object;)Ljava/lang/Object; + public static fun visitKey (Lkotlinx/serialization/encoding/CompositeDecoder;Ljava/lang/Object;)V } public abstract interface class kotlinx/serialization/encoding/CompositeEncoder { @@ -1119,6 +1126,7 @@ public abstract class kotlinx/serialization/internal/TaggedDecoder : kotlinx/ser protected abstract fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/Object; protected final fun popTag ()Ljava/lang/Object; protected final fun pushTag (Ljava/lang/Object;)V + public fun visitKey (Ljava/lang/Object;)V } public abstract class kotlinx/serialization/internal/TaggedEncoder : kotlinx/serialization/encoding/CompositeEncoder, kotlinx/serialization/encoding/Encoder { diff --git a/core/api/kotlinx-serialization-core.klib.api b/core/api/kotlinx-serialization-core.klib.api index 49b89f3d5e..e8568ce0fe 100644 --- a/core/api/kotlinx-serialization-core.klib.api +++ b/core/api/kotlinx-serialization-core.klib.api @@ -288,6 +288,7 @@ abstract interface kotlinx.serialization.encoding/CompositeDecoder { // kotlinx. } open fun decodeCollectionSize(kotlinx.serialization.descriptors/SerialDescriptor): kotlin/Int // kotlinx.serialization.encoding/CompositeDecoder.decodeCollectionSize|decodeCollectionSize(kotlinx.serialization.descriptors.SerialDescriptor){}[0] open fun decodeSequentially(): kotlin/Boolean // kotlinx.serialization.encoding/CompositeDecoder.decodeSequentially|decodeSequentially(){}[0] + open fun visitKey(kotlin/Any?) // kotlinx.serialization.encoding/CompositeDecoder.visitKey|visitKey(kotlin.Any?){}[0] } abstract interface kotlinx.serialization.encoding/CompositeEncoder { // kotlinx.serialization.encoding/CompositeEncoder|null[0] abstract fun <#A1: kotlin/Any> encodeNullableSerializableElement(kotlinx.serialization.descriptors/SerialDescriptor, kotlin/Int, kotlinx.serialization/SerializationStrategy<#A1>, #A1?) // kotlinx.serialization.encoding/CompositeEncoder.encodeNullableSerializableElement|encodeNullableSerializableElement(kotlinx.serialization.descriptors.SerialDescriptor;kotlin.Int;kotlinx.serialization.SerializationStrategy<0:0>;0:0?){0§}[0] @@ -522,6 +523,9 @@ final class kotlinx.serialization.modules/SerializersModuleBuilder : kotlinx.ser final fun build(): kotlinx.serialization.modules/SerializersModule // kotlinx.serialization.modules/SerializersModuleBuilder.build|build(){}[0] final fun include(kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.modules/SerializersModuleBuilder.include|include(kotlinx.serialization.modules.SerializersModule){}[0] } +final class kotlinx.serialization/DuplicateMapKeyException : kotlinx.serialization/SerializationException { // kotlinx.serialization/DuplicateMapKeyException|null[0] + constructor (kotlin/Any?) // kotlinx.serialization/DuplicateMapKeyException.|(kotlin.Any?){}[0] +} final class kotlinx.serialization/MissingFieldException : kotlinx.serialization/SerializationException { // kotlinx.serialization/MissingFieldException|null[0] constructor (kotlin.collections/List, kotlin/String) // kotlinx.serialization/MissingFieldException.|(kotlin.collections.List;kotlin.String){}[0] constructor (kotlin.collections/List, kotlin/String?, kotlin/Throwable?) // kotlinx.serialization/MissingFieldException.|(kotlin.collections.List;kotlin.String?;kotlin.Throwable?){}[0] diff --git a/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt b/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt index 99f7d0a76a..28dd0b9953 100644 --- a/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt +++ b/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt @@ -133,3 +133,10 @@ internal constructor(message: String?) : SerializationException(message) { // This constructor is used by the generated serializers constructor(index: Int) : this("An unknown field for index $index") } + +/** + * Thrown when a map deserializer encounters a repeated map key (and configuration disallows this.) + */ +@ExperimentalSerializationApi +public class DuplicateMapKeyException(key: Any?) : + SerializationException("Duplicate keys not allowed in maps. Key appeared twice: $key") diff --git a/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt b/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt index dc4aa2ab9e..305a762e82 100644 --- a/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt +++ b/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt @@ -275,7 +275,7 @@ internal inline fun Decoder.decodeIfNullable(deserializer: Deserializa * [CompositeDecoder] is a part of decoding process that is bound to a particular structured part of * the serialized form, described by the serial descriptor passed to [Decoder.beginStructure]. * - * Typically, for unordered data, [CompositeDecoder] is used by a serializer withing a [decodeElementIndex]-based + * Typically, for unordered data, [CompositeDecoder] is used by a serializer within a [decodeElementIndex]-based * loop that decodes all the required data one-by-one in any order and then terminates by calling [endStructure]. * Please refer to [decodeElementIndex] for example of such loop. * @@ -558,6 +558,18 @@ public interface CompositeDecoder { deserializer: DeserializationStrategy, previousValue: T? = null ): T? + + /** + * Called after a key has been read. + * + * This could be a map or set key, or anything otherwise intended to be + * distinct within the collection under normal circumstances. + * + * Implementations might use this as a hook for throwing an exception when + * duplicate keys are encountered. + */ + @ExperimentalSerializationApi + public fun visitKey(key: Any?) { } } /** diff --git a/core/commonMain/src/kotlinx/serialization/internal/CollectionSerializers.kt b/core/commonMain/src/kotlinx/serialization/internal/CollectionSerializers.kt index fcf4cfa74f..f624c64ec3 100644 --- a/core/commonMain/src/kotlinx/serialization/internal/CollectionSerializers.kt +++ b/core/commonMain/src/kotlinx/serialization/internal/CollectionSerializers.kt @@ -98,6 +98,7 @@ public sealed class MapLikeSerializer (ZZLkotlinx/serialization/modules/SerializersModule;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public synthetic fun (ZZZLkotlinx/serialization/modules/SerializersModule;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public fun decodeFromByteArray (Lkotlinx/serialization/DeserializationStrategy;[B)Ljava/lang/Object; public fun encodeToByteArray (Lkotlinx/serialization/SerializationStrategy;Ljava/lang/Object;)[B public fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule; @@ -17,9 +17,11 @@ public final class kotlinx/serialization/cbor/Cbor$Default : kotlinx/serializati } public final class kotlinx/serialization/cbor/CborBuilder { + public final fun getAllowDuplicateKeys ()Z public final fun getEncodeDefaults ()Z public final fun getIgnoreUnknownKeys ()Z public final fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule; + public final fun setAllowDuplicateKeys (Z)V public final fun setEncodeDefaults (Z)V public final fun setIgnoreUnknownKeys (Z)V public final fun setSerializersModule (Lkotlinx/serialization/modules/SerializersModule;)V diff --git a/formats/cbor/api/kotlinx-serialization-cbor.klib.api b/formats/cbor/api/kotlinx-serialization-cbor.klib.api index abb078d3b4..f3543f5cc3 100644 --- a/formats/cbor/api/kotlinx-serialization-cbor.klib.api +++ b/formats/cbor/api/kotlinx-serialization-cbor.klib.api @@ -7,6 +7,9 @@ // Library unique name: final class kotlinx.serialization.cbor/CborBuilder { // kotlinx.serialization.cbor/CborBuilder|null[0] + final var allowDuplicateKeys // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys|(){}[0] + final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.|(){}[0] + final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.|(kotlin.Boolean){}[0] final var encodeDefaults // kotlinx.serialization.cbor/CborBuilder.encodeDefaults|(){}[0] final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.|(){}[0] final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.|(kotlin.Boolean){}[0] @@ -22,7 +25,7 @@ open annotation class kotlinx.serialization.cbor/ByteString : kotlin/Annotation constructor () // kotlinx.serialization.cbor/ByteString.|(){}[0] } sealed class kotlinx.serialization.cbor/Cbor : kotlinx.serialization/BinaryFormat { // kotlinx.serialization.cbor/Cbor|null[0] - constructor (kotlin/Boolean, kotlin/Boolean, kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.cbor/Cbor.|(kotlin.Boolean;kotlin.Boolean;kotlinx.serialization.modules.SerializersModule){}[0] + constructor (kotlin/Boolean, kotlin/Boolean, kotlin/Boolean, kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.cbor/Cbor.|(kotlin.Boolean;kotlin.Boolean;kotlin.Boolean;kotlinx.serialization.modules.SerializersModule){}[0] final object Default : kotlinx.serialization.cbor/Cbor // kotlinx.serialization.cbor/Cbor.Default|null[0] open fun <#A1: kotlin/Any?> decodeFromByteArray(kotlinx.serialization/DeserializationStrategy<#A1>, kotlin/ByteArray): #A1 // kotlinx.serialization.cbor/Cbor.decodeFromByteArray|decodeFromByteArray(kotlinx.serialization.DeserializationStrategy<0:0>;kotlin.ByteArray){0§}[0] open fun <#A1: kotlin/Any?> encodeToByteArray(kotlinx.serialization/SerializationStrategy<#A1>, #A1): kotlin/ByteArray // kotlinx.serialization.cbor/Cbor.encodeToByteArray|encodeToByteArray(kotlinx.serialization.SerializationStrategy<0:0>;0:0){0§}[0] diff --git a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt index 9e76a8fbd2..d0c6d06bfe 100644 --- a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt +++ b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt @@ -32,13 +32,14 @@ import kotlinx.serialization.modules.* public sealed class Cbor( internal val encodeDefaults: Boolean, internal val ignoreUnknownKeys: Boolean, + internal val allowDuplicateKeys: Boolean, override val serializersModule: SerializersModule ) : BinaryFormat { /** * The default instance of [Cbor] */ - public companion object Default : Cbor(false, false, EmptySerializersModule()) + public companion object Default : Cbor(false, false, true, EmptySerializersModule()) override fun encodeToByteArray(serializer: SerializationStrategy, value: T): ByteArray { val output = ByteArrayOutput() @@ -55,8 +56,11 @@ public sealed class Cbor( } @OptIn(ExperimentalSerializationApi::class) -private class CborImpl(encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, serializersModule: SerializersModule) : - Cbor(encodeDefaults, ignoreUnknownKeys, serializersModule) +private class CborImpl( + encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, allowDuplicateKeys: Boolean, + serializersModule: SerializersModule, +) : + Cbor(encodeDefaults, ignoreUnknownKeys, allowDuplicateKeys, serializersModule) /** * Creates an instance of [Cbor] configured from the optionally given [Cbor instance][from] @@ -66,7 +70,7 @@ private class CborImpl(encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, seri public fun Cbor(from: Cbor = Cbor, builderAction: CborBuilder.() -> Unit): Cbor { val builder = CborBuilder(from) builder.builderAction() - return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.serializersModule) + return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.allowDuplicateKeys, builder.serializersModule) } /** @@ -87,6 +91,14 @@ public class CborBuilder internal constructor(cbor: Cbor) { */ public var ignoreUnknownKeys: Boolean = cbor.ignoreUnknownKeys + /** + * Specifies whether it is an error to read a map with duplicate keys. + * + * If this is set to false, decoding a map with two keys that compare as equal + * will cause a [DuplicateMapKeyException] error to be thrown. + */ + public var allowDuplicateKeys: Boolean = cbor.allowDuplicateKeys + /** * Module with contextual and polymorphic serializers to be used in the resulting [Cbor] instance. */ diff --git a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt index b77a18c599..e1c268f47b 100644 --- a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt +++ b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt @@ -197,8 +197,21 @@ internal class CborEncoder(private val output: ByteArrayOutput) { } } -private class CborMapReader(cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) { +private class CborMapReader(val cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) { + /** Keys that have been seen so far while reading this map. */ + private val seenKeys = mutableSetOf() + override fun skipBeginToken() = setSize(decoder.startMap() * 2) + + override fun visitKey(key: Any?) { + if (cbor.allowDuplicateKeys) + return + + val added = seenKeys.add(key) + if (!added) { + throw DuplicateMapKeyException(key) + } + } } private open class CborListReader(cbor: Cbor, decoder: CborDecoder) : CborReader(cbor, decoder) { diff --git a/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt new file mode 100644 index 0000000000..dff41e326f --- /dev/null +++ b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt @@ -0,0 +1,20 @@ +package kotlinx.serialization.cbor + +import kotlinx.serialization.assertFailsWithMessage +import kotlinx.serialization.decodeFromByteArray +import kotlinx.serialization.HexConverter +import kotlinx.serialization.DuplicateMapKeyException +import kotlin.test.Test + +class CborStrictModeTest { + private val strict = Cbor { allowDuplicateKeys = false } + + /** Duplicate keys are rejected in generic maps. */ + @Test + fun testDuplicateKeysInMap() { + val duplicateKeys = HexConverter.parseHexBinary("A2617805617806") + assertFailsWithMessage("Duplicate keys not allowed in maps. Key appeared twice: x") { + strict.decodeFromByteArray>(duplicateKeys) + } + } +} diff --git a/formats/json/api/kotlinx-serialization-json.api b/formats/json/api/kotlinx-serialization-json.api index 8082ee36dc..79c9b2fc17 100644 --- a/formats/json/api/kotlinx-serialization-json.api +++ b/formats/json/api/kotlinx-serialization-json.api @@ -182,6 +182,7 @@ public final class kotlinx/serialization/json/JsonDecoder$DefaultImpls { public static fun decodeNullableSerializableValue (Lkotlinx/serialization/json/JsonDecoder;Lkotlinx/serialization/DeserializationStrategy;)Ljava/lang/Object; public static fun decodeSequentially (Lkotlinx/serialization/json/JsonDecoder;)Z public static fun decodeSerializableValue (Lkotlinx/serialization/json/JsonDecoder;Lkotlinx/serialization/DeserializationStrategy;)Ljava/lang/Object; + public static fun visitKey (Lkotlinx/serialization/json/JsonDecoder;Ljava/lang/Object;)V } public abstract class kotlinx/serialization/json/JsonElement { From 47cfbc14bed48843a4d6f9f431af9c6ed86ddbdf Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Sun, 19 May 2024 22:09:54 -0400 Subject: [PATCH 2/3] fixup! Rename to forbidDuplicateKeys; rename exception; related cleanup --- core/api/kotlinx-serialization-core.api | 2 +- core/api/kotlinx-serialization-core.klib.api | 4 ++-- .../serialization/SerializationExceptions.kt | 6 +++--- formats/cbor/api/kotlinx-serialization-cbor.api | 4 ++-- .../cbor/api/kotlinx-serialization-cbor.klib.api | 6 +++--- .../src/kotlinx/serialization/cbor/Cbor.kt | 16 ++++++++-------- .../serialization/cbor/internal/Encoding.kt | 14 +++++++------- .../serialization/cbor/CborStrictModeTest.kt | 6 +++--- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/core/api/kotlinx-serialization-core.api b/core/api/kotlinx-serialization-core.api index db026f1541..e1e4202e98 100644 --- a/core/api/kotlinx-serialization-core.api +++ b/core/api/kotlinx-serialization-core.api @@ -19,7 +19,7 @@ public abstract interface class kotlinx/serialization/DeserializationStrategy { public abstract fun getDescriptor ()Lkotlinx/serialization/descriptors/SerialDescriptor; } -public final class kotlinx/serialization/DuplicateMapKeyException : kotlinx/serialization/SerializationException { +public final class kotlinx/serialization/DuplicateKeyException : kotlinx/serialization/SerializationException { public fun (Ljava/lang/Object;)V } diff --git a/core/api/kotlinx-serialization-core.klib.api b/core/api/kotlinx-serialization-core.klib.api index e8568ce0fe..73fe921889 100644 --- a/core/api/kotlinx-serialization-core.klib.api +++ b/core/api/kotlinx-serialization-core.klib.api @@ -523,8 +523,8 @@ final class kotlinx.serialization.modules/SerializersModuleBuilder : kotlinx.ser final fun build(): kotlinx.serialization.modules/SerializersModule // kotlinx.serialization.modules/SerializersModuleBuilder.build|build(){}[0] final fun include(kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.modules/SerializersModuleBuilder.include|include(kotlinx.serialization.modules.SerializersModule){}[0] } -final class kotlinx.serialization/DuplicateMapKeyException : kotlinx.serialization/SerializationException { // kotlinx.serialization/DuplicateMapKeyException|null[0] - constructor (kotlin/Any?) // kotlinx.serialization/DuplicateMapKeyException.|(kotlin.Any?){}[0] +final class kotlinx.serialization/DuplicateKeyException : kotlinx.serialization/SerializationException { // kotlinx.serialization/DuplicateKeyException|null[0] + constructor (kotlin/Any?) // kotlinx.serialization/DuplicateKeyException.|(kotlin.Any?){}[0] } final class kotlinx.serialization/MissingFieldException : kotlinx.serialization/SerializationException { // kotlinx.serialization/MissingFieldException|null[0] constructor (kotlin.collections/List, kotlin/String) // kotlinx.serialization/MissingFieldException.|(kotlin.collections.List;kotlin.String){}[0] diff --git a/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt b/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt index 28dd0b9953..6808193db2 100644 --- a/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt +++ b/core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt @@ -135,8 +135,8 @@ internal constructor(message: String?) : SerializationException(message) { } /** - * Thrown when a map deserializer encounters a repeated map key (and configuration disallows this.) + * Thrown when a deserializer encounters a repeated key (and configuration disallows this.) */ @ExperimentalSerializationApi -public class DuplicateMapKeyException(key: Any?) : - SerializationException("Duplicate keys not allowed in maps. Key appeared twice: $key") +public class DuplicateKeyException(key: Any?) : + SerializationException("Duplicate keys not allowed. Key appeared twice: $key") diff --git a/formats/cbor/api/kotlinx-serialization-cbor.api b/formats/cbor/api/kotlinx-serialization-cbor.api index 48fc8afe5f..1fc4ad626d 100644 --- a/formats/cbor/api/kotlinx-serialization-cbor.api +++ b/formats/cbor/api/kotlinx-serialization-cbor.api @@ -17,12 +17,12 @@ public final class kotlinx/serialization/cbor/Cbor$Default : kotlinx/serializati } public final class kotlinx/serialization/cbor/CborBuilder { - public final fun getAllowDuplicateKeys ()Z public final fun getEncodeDefaults ()Z + public final fun getForbidDuplicateKeys ()Z public final fun getIgnoreUnknownKeys ()Z public final fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule; - public final fun setAllowDuplicateKeys (Z)V public final fun setEncodeDefaults (Z)V + public final fun setForbidDuplicateKeys (Z)V public final fun setIgnoreUnknownKeys (Z)V public final fun setSerializersModule (Lkotlinx/serialization/modules/SerializersModule;)V } diff --git a/formats/cbor/api/kotlinx-serialization-cbor.klib.api b/formats/cbor/api/kotlinx-serialization-cbor.klib.api index f3543f5cc3..9455e913eb 100644 --- a/formats/cbor/api/kotlinx-serialization-cbor.klib.api +++ b/formats/cbor/api/kotlinx-serialization-cbor.klib.api @@ -7,12 +7,12 @@ // Library unique name: final class kotlinx.serialization.cbor/CborBuilder { // kotlinx.serialization.cbor/CborBuilder|null[0] - final var allowDuplicateKeys // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys|(){}[0] - final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.|(){}[0] - final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.|(kotlin.Boolean){}[0] final var encodeDefaults // kotlinx.serialization.cbor/CborBuilder.encodeDefaults|(){}[0] final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.|(){}[0] final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.|(kotlin.Boolean){}[0] + final var forbidDuplicateKeys // kotlinx.serialization.cbor/CborBuilder.forbidDuplicateKeys|(){}[0] + final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.forbidDuplicateKeys.|(){}[0] + final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.forbidDuplicateKeys.|(kotlin.Boolean){}[0] final var ignoreUnknownKeys // kotlinx.serialization.cbor/CborBuilder.ignoreUnknownKeys|(){}[0] final fun (): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.ignoreUnknownKeys.|(){}[0] final fun (kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.ignoreUnknownKeys.|(kotlin.Boolean){}[0] diff --git a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt index d0c6d06bfe..5920eb454c 100644 --- a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt +++ b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt @@ -32,14 +32,14 @@ import kotlinx.serialization.modules.* public sealed class Cbor( internal val encodeDefaults: Boolean, internal val ignoreUnknownKeys: Boolean, - internal val allowDuplicateKeys: Boolean, + internal val forbidDuplicateKeys: Boolean, override val serializersModule: SerializersModule ) : BinaryFormat { /** * The default instance of [Cbor] */ - public companion object Default : Cbor(false, false, true, EmptySerializersModule()) + public companion object Default : Cbor(false, false, false, EmptySerializersModule()) override fun encodeToByteArray(serializer: SerializationStrategy, value: T): ByteArray { val output = ByteArrayOutput() @@ -57,10 +57,10 @@ public sealed class Cbor( @OptIn(ExperimentalSerializationApi::class) private class CborImpl( - encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, allowDuplicateKeys: Boolean, + encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, forbidDuplicateKeys: Boolean, serializersModule: SerializersModule, ) : - Cbor(encodeDefaults, ignoreUnknownKeys, allowDuplicateKeys, serializersModule) + Cbor(encodeDefaults, ignoreUnknownKeys, forbidDuplicateKeys, serializersModule) /** * Creates an instance of [Cbor] configured from the optionally given [Cbor instance][from] @@ -70,7 +70,7 @@ private class CborImpl( public fun Cbor(from: Cbor = Cbor, builderAction: CborBuilder.() -> Unit): Cbor { val builder = CborBuilder(from) builder.builderAction() - return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.allowDuplicateKeys, builder.serializersModule) + return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.forbidDuplicateKeys, builder.serializersModule) } /** @@ -94,10 +94,10 @@ public class CborBuilder internal constructor(cbor: Cbor) { /** * Specifies whether it is an error to read a map with duplicate keys. * - * If this is set to false, decoding a map with two keys that compare as equal - * will cause a [DuplicateMapKeyException] error to be thrown. + * If this is set to true, decoding a map with two keys that compare as equal + * will cause a [DuplicateKeyException] error to be thrown. */ - public var allowDuplicateKeys: Boolean = cbor.allowDuplicateKeys + public var forbidDuplicateKeys: Boolean = cbor.forbidDuplicateKeys /** * Module with contextual and polymorphic serializers to be used in the resulting [Cbor] instance. diff --git a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt index e1c268f47b..4c855b7166 100644 --- a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt +++ b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt @@ -198,18 +198,18 @@ internal class CborEncoder(private val output: ByteArrayOutput) { } private class CborMapReader(val cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) { - /** Keys that have been seen so far while reading this map. */ + /** + * Keys that have been seen so far while reading this map. + * + * Only used if [Cbor.forbidDuplicateKeys] is in effect. + */ private val seenKeys = mutableSetOf() override fun skipBeginToken() = setSize(decoder.startMap() * 2) override fun visitKey(key: Any?) { - if (cbor.allowDuplicateKeys) - return - - val added = seenKeys.add(key) - if (!added) { - throw DuplicateMapKeyException(key) + if (cbor.forbidDuplicateKeys) { + seenKeys.add(key) || throw DuplicateKeyException(key) } } } diff --git a/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt index dff41e326f..ec42eb1664 100644 --- a/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt +++ b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt @@ -3,17 +3,17 @@ package kotlinx.serialization.cbor import kotlinx.serialization.assertFailsWithMessage import kotlinx.serialization.decodeFromByteArray import kotlinx.serialization.HexConverter -import kotlinx.serialization.DuplicateMapKeyException +import kotlinx.serialization.DuplicateKeyException import kotlin.test.Test class CborStrictModeTest { - private val strict = Cbor { allowDuplicateKeys = false } + private val strict = Cbor { forbidDuplicateKeys = true } /** Duplicate keys are rejected in generic maps. */ @Test fun testDuplicateKeysInMap() { val duplicateKeys = HexConverter.parseHexBinary("A2617805617806") - assertFailsWithMessage("Duplicate keys not allowed in maps. Key appeared twice: x") { + assertFailsWithMessage("Duplicate keys not allowed. Key appeared twice: x") { strict.decodeFromByteArray>(duplicateKeys) } } From d8ddd99134c0e77bb0f95d40a2168895cb087ae8 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Tue, 21 May 2024 09:23:14 -0400 Subject: [PATCH 3/3] fixup! Catch duplicate keys in class deserialization (including unknown) --- .../serialization/cbor/internal/Encoding.kt | 30 ++++++++++--------- .../serialization/cbor/CborStrictModeTest.kt | 25 ++++++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt index 4c855b7166..aceb96eb10 100644 --- a/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt +++ b/formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt @@ -197,21 +197,8 @@ internal class CborEncoder(private val output: ByteArrayOutput) { } } -private class CborMapReader(val cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) { - /** - * Keys that have been seen so far while reading this map. - * - * Only used if [Cbor.forbidDuplicateKeys] is in effect. - */ - private val seenKeys = mutableSetOf() - +private class CborMapReader(cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) { override fun skipBeginToken() = setSize(decoder.startMap() * 2) - - override fun visitKey(key: Any?) { - if (cbor.forbidDuplicateKeys) { - seenKeys.add(key) || throw DuplicateKeyException(key) - } - } } private open class CborListReader(cbor: Cbor, decoder: CborDecoder) : CborReader(cbor, decoder) { @@ -232,6 +219,13 @@ internal open class CborReader(private val cbor: Cbor, protected val decoder: Cb private var decodeByteArrayAsByteString = false + /** + * Keys that have been seen so far while reading this map. + * + * Only used if [Cbor.forbidDuplicateKeys] is in effect. + */ + private val seenKeys = mutableSetOf() + protected fun setSize(size: Int) { if (size >= 0) { finiteMode = true @@ -259,12 +253,19 @@ internal open class CborReader(private val cbor: Cbor, protected val decoder: Cb if (!finiteMode) decoder.end() } + override fun visitKey(key: Any?) { + if (cbor.forbidDuplicateKeys) { + seenKeys.add(key) || throw DuplicateKeyException(key) + } + } + override fun decodeElementIndex(descriptor: SerialDescriptor): Int { val index = if (cbor.ignoreUnknownKeys) { val knownIndex: Int while (true) { if (isDone()) return CompositeDecoder.DECODE_DONE val elemName = decoder.nextString() + visitKey(elemName) readProperties++ val index = descriptor.getElementIndex(elemName) @@ -279,6 +280,7 @@ internal open class CborReader(private val cbor: Cbor, protected val decoder: Cb } else { if (isDone()) return CompositeDecoder.DECODE_DONE val elemName = decoder.nextString() + visitKey(elemName) readProperties++ descriptor.getElementIndexOrThrow(elemName) } diff --git a/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt index ec42eb1664..3b9404f545 100644 --- a/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt +++ b/formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt @@ -4,6 +4,7 @@ import kotlinx.serialization.assertFailsWithMessage import kotlinx.serialization.decodeFromByteArray import kotlinx.serialization.HexConverter import kotlinx.serialization.DuplicateKeyException +import kotlinx.serialization.Serializable import kotlin.test.Test class CborStrictModeTest { @@ -17,4 +18,28 @@ class CborStrictModeTest { strict.decodeFromByteArray>(duplicateKeys) } } + + @Serializable + data class ExampleClass(val x: Long) + + /** Duplicate keys are rejected in classes. */ + @Test + fun testDuplicateKeysInDataClass() { + // {"x": 5, "x", 6} + val duplicateKeys = HexConverter.parseHexBinary("A2617805617806") + assertFailsWithMessage("Duplicate keys not allowed. Key appeared twice: x") { + strict.decodeFromByteArray(duplicateKeys) + } + } + + /** Duplicate unknown keys are rejected as well. */ + @Test + fun testDuplicateUnknownKeys() { + // {"a": 1, "a", 2, "x", 6} + val duplicateKeys = HexConverter.parseHexBinary("A3616101616102617806") + val cbor = Cbor(strict) { ignoreUnknownKeys = true } + assertFailsWithMessage("Duplicate keys not allowed. Key appeared twice: a") { + cbor.decodeFromByteArray(duplicateKeys) + } + } }