fixup! Catch duplicate keys in class deserialization (including unknown)

Author: timmc

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<Any?>()
-
+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<Any?>()
+
     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)
         }

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<Map<String, Long>>(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<DuplicateKeyException>("Duplicate keys not allowed. Key appeared twice: x") {
+            strict.decodeFromByteArray<ExampleClass>(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<DuplicateKeyException>("Duplicate keys not allowed. Key appeared twice: a") {
+            cbor.decodeFromByteArray<ExampleClass>(duplicateKeys)
+        }
+    }
 }