Skip to content

Commit e2945f2

Browse files
timmctimmc
committed
Add ability to disallow repeated keys in CBOR
Fixes Kotlin#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.
1 parent 194a188 commit e2945f2

File tree

11 files changed

+91
-8
lines changed

11 files changed

+91
-8
lines changed

‎core/api/kotlinx-serialization-core.api

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ public abstract interface class kotlinx/serialization/DeserializationStrategy {
1919
public abstract fun getDescriptor ()Lkotlinx/serialization/descriptors/SerialDescriptor;
2020
}
2121

22+
public final class kotlinx/serialization/DuplicateMapKeyException : kotlinx/serialization/SerializationException {
23+
public fun <init> (Ljava/lang/Object;)V
24+
}
25+
2226
public abstract interface annotation class kotlinx/serialization/EncodeDefault : java/lang/annotation/Annotation {
2327
public abstract fun mode ()Lkotlinx/serialization/EncodeDefault$Mode;
2428
}
@@ -384,6 +388,7 @@ public abstract class kotlinx/serialization/encoding/AbstractDecoder : kotlinx/s
384388
public final fun decodeStringElement (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String;
385389
public fun decodeValue ()Ljava/lang/Object;
386390
public fun endStructure (Lkotlinx/serialization/descriptors/SerialDescriptor;)V
391+
public fun visitKey (Ljava/lang/Object;)V
387392
}
388393

389394
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
448453
public abstract fun decodeStringElement (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String;
449454
public abstract fun endStructure (Lkotlinx/serialization/descriptors/SerialDescriptor;)V
450455
public abstract fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule;
456+
public abstract fun visitKey (Ljava/lang/Object;)V
451457
}
452458

453459
public final class kotlinx/serialization/encoding/CompositeDecoder$Companion {
@@ -460,6 +466,7 @@ public final class kotlinx/serialization/encoding/CompositeDecoder$DefaultImpls
460466
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;
461467
public static fun decodeSequentially (Lkotlinx/serialization/encoding/CompositeDecoder;)Z
462468
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;
469+
public static fun visitKey (Lkotlinx/serialization/encoding/CompositeDecoder;Ljava/lang/Object;)V
463470
}
464471

465472
public abstract interface class kotlinx/serialization/encoding/CompositeEncoder {
@@ -1119,6 +1126,7 @@ public abstract class kotlinx/serialization/internal/TaggedDecoder : kotlinx/ser
11191126
protected abstract fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/Object;
11201127
protected final fun popTag ()Ljava/lang/Object;
11211128
protected final fun pushTag (Ljava/lang/Object;)V
1129+
public fun visitKey (Ljava/lang/Object;)V
11221130
}
11231131

11241132
public abstract class kotlinx/serialization/internal/TaggedEncoder : kotlinx/serialization/encoding/CompositeEncoder, kotlinx/serialization/encoding/Encoder {

‎core/api/kotlinx-serialization-core.klib.api

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ abstract interface kotlinx.serialization.encoding/CompositeDecoder { // kotlinx.
288288
}
289289
open fun decodeCollectionSize(kotlinx.serialization.descriptors/SerialDescriptor): kotlin/Int // kotlinx.serialization.encoding/CompositeDecoder.decodeCollectionSize|decodeCollectionSize(kotlinx.serialization.descriptors.SerialDescriptor){}[0]
290290
open fun decodeSequentially(): kotlin/Boolean // kotlinx.serialization.encoding/CompositeDecoder.decodeSequentially|decodeSequentially(){}[0]
291+
open fun visitKey(kotlin/Any?) // kotlinx.serialization.encoding/CompositeDecoder.visitKey|visitKey(kotlin.Any?){}[0]
291292
}
292293
abstract interface kotlinx.serialization.encoding/CompositeEncoder { // kotlinx.serialization.encoding/CompositeEncoder|null[0]
293294
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§<kotlin.Any>}[0]
@@ -522,6 +523,9 @@ final class kotlinx.serialization.modules/SerializersModuleBuilder : kotlinx.ser
522523
final fun build(): kotlinx.serialization.modules/SerializersModule // kotlinx.serialization.modules/SerializersModuleBuilder.build|build(){}[0]
523524
final fun include(kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.modules/SerializersModuleBuilder.include|include(kotlinx.serialization.modules.SerializersModule){}[0]
524525
}
526+
final class kotlinx.serialization/DuplicateMapKeyException : kotlinx.serialization/SerializationException { // kotlinx.serialization/DuplicateMapKeyException|null[0]
527+
constructor <init>(kotlin/Any?) // kotlinx.serialization/DuplicateMapKeyException.<init>|<init>(kotlin.Any?){}[0]
528+
}
525529
final class kotlinx.serialization/MissingFieldException : kotlinx.serialization/SerializationException { // kotlinx.serialization/MissingFieldException|null[0]
526530
constructor <init>(kotlin.collections/List<kotlin/String>, kotlin/String) // kotlinx.serialization/MissingFieldException.<init>|<init>(kotlin.collections.List<kotlin.String>;kotlin.String){}[0]
527531
constructor <init>(kotlin.collections/List<kotlin/String>, kotlin/String?, kotlin/Throwable?) // kotlinx.serialization/MissingFieldException.<init>|<init>(kotlin.collections.List<kotlin.String>;kotlin.String?;kotlin.Throwable?){}[0]

‎core/commonMain/src/kotlinx/serialization/SerializationExceptions.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,10 @@ internal constructor(message: String?) : SerializationException(message) {
133133
// This constructor is used by the generated serializers
134134
constructor(index: Int) : this("An unknown field for index $index")
135135
}
136+
137+
/**
138+
* Thrown when a map deserializer encounters a repeated map key (and configuration disallows this.)
139+
*/
140+
@ExperimentalSerializationApi
141+
public class DuplicateMapKeyException(key: Any?) :
142+
SerializationException("Duplicate keys not allowed in maps. Key appeared twice: $key")

‎core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ internal inline fun <T : Any> Decoder.decodeIfNullable(deserializer: Deserializa
275275
* [CompositeDecoder] is a part of decoding process that is bound to a particular structured part of
276276
* the serialized form, described by the serial descriptor passed to [Decoder.beginStructure].
277277
*
278-
* Typically, for unordered data, [CompositeDecoder] is used by a serializer withing a [decodeElementIndex]-based
278+
* Typically, for unordered data, [CompositeDecoder] is used by a serializer within a [decodeElementIndex]-based
279279
* loop that decodes all the required data one-by-one in any order and then terminates by calling [endStructure].
280280
* Please refer to [decodeElementIndex] for example of such loop.
281281
*
@@ -558,6 +558,18 @@ public interface CompositeDecoder {
558558
deserializer: DeserializationStrategy<T?>,
559559
previousValue: T? = null
560560
): T?
561+
562+
/**
563+
* Called after a key has been read.
564+
*
565+
* This could be a map or set key, or anything otherwise intended to be
566+
* distinct within the collection under normal circumstances.
567+
*
568+
* Implementations might use this as a hook for throwing an exception when
569+
* duplicate keys are encountered.
570+
*/
571+
@ExperimentalSerializationApi
572+
public fun visitKey(key: Any?) { }
561573
}
562574

563575
/**

‎core/commonMain/src/kotlinx/serialization/internal/CollectionSerializers.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ public sealed class MapLikeSerializer<Key, Value, Collection, Builder : MutableM
9898

9999
final override fun readElement(decoder: CompositeDecoder, index: Int, builder: Builder, checkIndex: Boolean) {
100100
val key: Key = decoder.decodeSerializableElement(descriptor, index, keySerializer)
101+
decoder.visitKey(key)
101102
val vIndex = if (checkIndex) {
102103
decoder.decodeElementIndex(descriptor).also {
103104
require(it == index + 1) { "Value must follow key in a map, index for key: $index, returned index for value: $it" }

‎formats/cbor/api/kotlinx-serialization-cbor.api

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ public synthetic class kotlinx/serialization/cbor/ByteString$Impl : kotlinx/seri
77

88
public abstract class kotlinx/serialization/cbor/Cbor : kotlinx/serialization/BinaryFormat {
99
public static final field Default Lkotlinx/serialization/cbor/Cbor$Default;
10-
public synthetic fun <init> (ZZLkotlinx/serialization/modules/SerializersModule;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
10+
public synthetic fun <init> (ZZZLkotlinx/serialization/modules/SerializersModule;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
1111
public fun decodeFromByteArray (Lkotlinx/serialization/DeserializationStrategy;[B)Ljava/lang/Object;
1212
public fun encodeToByteArray (Lkotlinx/serialization/SerializationStrategy;Ljava/lang/Object;)[B
1313
public fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule;
@@ -17,9 +17,11 @@ public final class kotlinx/serialization/cbor/Cbor$Default : kotlinx/serializati
1717
}
1818

1919
public final class kotlinx/serialization/cbor/CborBuilder {
20+
public final fun getAllowDuplicateKeys ()Z
2021
public final fun getEncodeDefaults ()Z
2122
public final fun getIgnoreUnknownKeys ()Z
2223
public final fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule;
24+
public final fun setAllowDuplicateKeys (Z)V
2325
public final fun setEncodeDefaults (Z)V
2426
public final fun setIgnoreUnknownKeys (Z)V
2527
public final fun setSerializersModule (Lkotlinx/serialization/modules/SerializersModule;)V

‎formats/cbor/api/kotlinx-serialization-cbor.klib.api

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
// Library unique name: <org.jetbrains.kotlinx:kotlinx-serialization-cbor>
99
final class kotlinx.serialization.cbor/CborBuilder { // kotlinx.serialization.cbor/CborBuilder|null[0]
10+
final var allowDuplicateKeys // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys|<get-allowDuplicateKeys>(){}[0]
11+
final fun <get-allowDuplicateKeys>(): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.<get-allowDuplicateKeys>|<get-allowDuplicateKeys>(){}[0]
12+
final fun <set-allowDuplicateKeys>(kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.allowDuplicateKeys.<set-allowDuplicateKeys>|<set-allowDuplicateKeys>(kotlin.Boolean){}[0]
1013
final var encodeDefaults // kotlinx.serialization.cbor/CborBuilder.encodeDefaults|<get-encodeDefaults>(){}[0]
1114
final fun <get-encodeDefaults>(): kotlin/Boolean // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.<get-encodeDefaults>|<get-encodeDefaults>(){}[0]
1215
final fun <set-encodeDefaults>(kotlin/Boolean) // kotlinx.serialization.cbor/CborBuilder.encodeDefaults.<set-encodeDefaults>|<set-encodeDefaults>(kotlin.Boolean){}[0]
@@ -22,7 +25,7 @@ open annotation class kotlinx.serialization.cbor/ByteString : kotlin/Annotation
2225
constructor <init>() // kotlinx.serialization.cbor/ByteString.<init>|<init>(){}[0]
2326
}
2427
sealed class kotlinx.serialization.cbor/Cbor : kotlinx.serialization/BinaryFormat { // kotlinx.serialization.cbor/Cbor|null[0]
25-
constructor <init>(kotlin/Boolean, kotlin/Boolean, kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.cbor/Cbor.<init>|<init>(kotlin.Boolean;kotlin.Boolean;kotlinx.serialization.modules.SerializersModule){}[0]
28+
constructor <init>(kotlin/Boolean, kotlin/Boolean, kotlin/Boolean, kotlinx.serialization.modules/SerializersModule) // kotlinx.serialization.cbor/Cbor.<init>|<init>(kotlin.Boolean;kotlin.Boolean;kotlin.Boolean;kotlinx.serialization.modules.SerializersModule){}[0]
2629
final object Default : kotlinx.serialization.cbor/Cbor // kotlinx.serialization.cbor/Cbor.Default|null[0]
2730
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§<kotlin.Any?>}[0]
2831
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§<kotlin.Any?>}[0]

‎formats/cbor/commonMain/src/kotlinx/serialization/cbor/Cbor.kt

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,14 @@ import kotlinx.serialization.modules.*
3232
public sealed class Cbor(
3333
internal val encodeDefaults: Boolean,
3434
internal val ignoreUnknownKeys: Boolean,
35+
internal val allowDuplicateKeys: Boolean,
3536
override val serializersModule: SerializersModule
3637
) : BinaryFormat {
3738

3839
/**
3940
* The default instance of [Cbor]
4041
*/
41-
public companion object Default : Cbor(false, false, EmptySerializersModule())
42+
public companion object Default : Cbor(false, false, true, EmptySerializersModule())
4243

4344
override fun <T> encodeToByteArray(serializer: SerializationStrategy<T>, value: T): ByteArray {
4445
val output = ByteArrayOutput()
@@ -55,8 +56,11 @@ public sealed class Cbor(
5556
}
5657

5758
@OptIn(ExperimentalSerializationApi::class)
58-
private class CborImpl(encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, serializersModule: SerializersModule) :
59-
Cbor(encodeDefaults, ignoreUnknownKeys, serializersModule)
59+
private class CborImpl(
60+
encodeDefaults: Boolean, ignoreUnknownKeys: Boolean, allowDuplicateKeys: Boolean,
61+
serializersModule: SerializersModule,
62+
) :
63+
Cbor(encodeDefaults, ignoreUnknownKeys, allowDuplicateKeys, serializersModule)
6064

6165
/**
6266
* 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
6670
public fun Cbor(from: Cbor = Cbor, builderAction: CborBuilder.() -> Unit): Cbor {
6771
val builder = CborBuilder(from)
6872
builder.builderAction()
69-
return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.serializersModule)
73+
return CborImpl(builder.encodeDefaults, builder.ignoreUnknownKeys, builder.allowDuplicateKeys, builder.serializersModule)
7074
}
7175

7276
/**
@@ -87,6 +91,14 @@ public class CborBuilder internal constructor(cbor: Cbor) {
8791
*/
8892
public var ignoreUnknownKeys: Boolean = cbor.ignoreUnknownKeys
8993

94+
/**
95+
* Specifies whether it is an error to read a map with duplicate keys.
96+
*
97+
* If this is set to false, decoding a map with two keys that compare as equal
98+
* will cause a [DuplicateMapKeyException] error to be thrown.
99+
*/
100+
public var allowDuplicateKeys: Boolean = cbor.allowDuplicateKeys
101+
90102
/**
91103
* Module with contextual and polymorphic serializers to be used in the resulting [Cbor] instance.
92104
*/

‎formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Encoding.kt

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,21 @@ internal class CborEncoder(private val output: ByteArrayOutput) {
197197
}
198198
}
199199

200-
private class CborMapReader(cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) {
200+
private class CborMapReader(val cbor: Cbor, decoder: CborDecoder) : CborListReader(cbor, decoder) {
201+
/** Keys that have been seen so far while reading this map. */
202+
private val seenKeys = mutableSetOf<Any?>()
203+
201204
override fun skipBeginToken() = setSize(decoder.startMap() * 2)
205+
206+
override fun visitKey(key: Any?) {
207+
if (cbor.allowDuplicateKeys)
208+
return
209+
210+
val added = seenKeys.add(key)
211+
if (!added) {
212+
throw DuplicateMapKeyException(key)
213+
}
214+
}
202215
}
203216

204217
private open class CborListReader(cbor: Cbor, decoder: CborDecoder) : CborReader(cbor, decoder) {

‎formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborStrictModeTest.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package kotlinx.serialization.cbor
2+
3+
import kotlinx.serialization.assertFailsWithMessage
4+
import kotlinx.serialization.decodeFromByteArray
5+
import kotlinx.serialization.HexConverter
6+
import kotlinx.serialization.DuplicateMapKeyException
7+
import kotlin.test.Test
8+
9+
class CborStrictModeTest {
10+
private val strict = Cbor { allowDuplicateKeys = false }
11+
12+
/** Duplicate keys are rejected in generic maps. */
13+
@Test
14+
fun testDuplicateKeysInMap() {
15+
val duplicateKeys = HexConverter.parseHexBinary("A2617805617806")
16+
assertFailsWithMessage<DuplicateMapKeyException>("Duplicate keys not allowed in maps. Key appeared twice: x") {
17+
strict.decodeFromByteArray<Map<String, Long>>(duplicateKeys)
18+
}
19+
}
20+
}

‎formats/json/api/kotlinx-serialization-json.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ public final class kotlinx/serialization/json/JsonDecoder$DefaultImpls {
182182
public static fun decodeNullableSerializableValue (Lkotlinx/serialization/json/JsonDecoder;Lkotlinx/serialization/DeserializationStrategy;)Ljava/lang/Object;
183183
public static fun decodeSequentially (Lkotlinx/serialization/json/JsonDecoder;)Z
184184
public static fun decodeSerializableValue (Lkotlinx/serialization/json/JsonDecoder;Lkotlinx/serialization/DeserializationStrategy;)Ljava/lang/Object;
185+
public static fun visitKey (Lkotlinx/serialization/json/JsonDecoder;Ljava/lang/Object;)V
185186
}
186187

187188
public abstract class kotlinx/serialization/json/JsonElement {

0 commit comments

Comments
 (0)