Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON serialization incorrect behavior #2885

Open
FerrumBrain opened this issue Dec 16, 2024 · 0 comments
Open

JSON serialization incorrect behavior #2885

FerrumBrain opened this issue Dec 16, 2024 · 0 comments
Assignees

Comments

@FerrumBrain
Copy link

FerrumBrain commented Dec 16, 2024

0. Setup

We created the following class hierarchy for testing serialization library:

Value (org.plan.research)
    ArrayValue (org.plan.research)
    BooleanArrayValue (org.plan.research)
    BooleanValue (org.plan.research)
    ByteArrayValue (org.plan.research)
    ByteValue (org.plan.research)
    CharArrayValue (org.plan.research)
    CharValue (org.plan.research)
    CompositeNullableValue (org.plan.research)
    DefaultValueAlways (org.plan.research)
    DefaultValueNever (org.plan.research)
    DoubleArrayValue (org.plan.research)
    DoubleValue (org.plan.research)
    EnumValue (org.plan.research)
    FloatArrayValue (org.plan.research)
    FloatValue (org.plan.research)
    IntArrayValue (org.plan.research)
    IntValue (org.plan.research)
    ListValue (org.plan.research)
    LongArrayValue (org.plan.research)
    LongValue (org.plan.research)
    NullValue (org.plan.research)
    ObjectValue (org.plan.research)
    ShortArrayValue (org.plan.research)
    ShortValue (org.plan.research)
    StringValue (org.plan.research)

Value hierarchy tries to use most of the available serialization API and test it on all main data types available on Kotlin/JVM.
The exact implementation details are not important in most cases. We will highlight interesting implementation details whenever necessary.

1. Unhandled conflict of classDesctiptor config option and JsonNames annotation values

Value class has a property with custom JSON names specified:

@OptIn(ExperimentalSerializationApi::class)
@Serializable
sealed class Value {
    @JsonNames(
        "THIS_IS_STATUS",
        "STATUS",
        "IS_OPEN"
    )
    var status = "open"
    @Suppress("unused")
    val randomStr: String get() = status
}

If we specify classDiscriminator equal to one of the JSON names, it will create a name conflict that is not detected.
Moreover, this conflict will actually affect the serialization:

@Test
fun `json class descriptor name conflict`() {
    val serializer = Json {
        classDiscriminator = "THIS_IS_STATUS"
    }
    val value: Value = CompositeNullableValue(
        StringValue("foo"),
        NullValue,
        NullValue
    )
    val str = serializer.encodeToString(value)
    val decodedValue = serializer.decodeFromString<Value>(str)
    assertTrue { value == decodedValue }
    // value.status == "open"
    // decodedValue.status == "org.plan.research.CompositeNullableValue"
    // test fail
    assertTrue { value.status == decodedValue.status }
}

2. Undocumented behaviour of decodeToSequence with allowTrailingComma option

While allowTrailingComma=true works well with decodeFromString, it does not affect decodeToSequence in any way.
This behaviour is not documented anywhere.

@OptIn(ExperimentalSerializationApi::class)
@Test
fun `json decode sequence cant parse array of enums with trailing comma`() {
    val string = """[{
    "type": "org.plan.research.EnumValue",
    "value": "SIXTH"
},]"""
    val inputStream = string.byteInputStream()
    val serializer = Json {
        allowTrailingComma = true
    }
    // works OK
    val directlyDecodedList = serializer.decodeFromString<List<Value>>(string)
    // `decodeToSequence` fails with `kotlinx.serialization.json.internal.JsonDecodingException`
    val values = mutableListOf<Value>()
    for (element in serializer.decodeToSequence<Value>(inputStream, DecodeSequenceMode.ARRAY_WRAPPED)) {
        values.add(element)
    }
    assertEquals(directlyDecodedList, values)
}

3. Wrong error message in combination of decodeToSequence and allowTrailingComma=false

@OptIn(ExperimentalSerializationApi::class)
@Test
fun `json decode sequence fails with wrong message because of trailing comma`() {
    val string = """[{
"type": "org.plan.research.NullValue"
},]"""
    val inputStream = string.byteInputStream()
    val serializer = Json {
        allowTrailingComma = false
    }
    try {
        val values = mutableListOf<Value>()
        for (element in serializer.decodeToSequence<Value>(inputStream, DecodeSequenceMode.ARRAY_WRAPPED)) {
            values.add(element)
        }
        println(values.joinToString("\n"))
    } catch (e: SerializationException) {
        // Fails with "Unexpected JSON token at offset 47: Cannot read Json element because of unexpected end of the array ']'"
        // error message
        assertTrue {
            e.javaClass.name == "kotlinx.serialization.json.internal.JsonDecodingException"
                && e.message.orEmpty().startsWith("Unexpected JSON token at offset")
                && e.message.orEmpty().contains("Trailing comma before the end of JSON array at path:")
        }
    }
}

Bugs are found by fuzzing team @ PLAN Lab

Environment

  • Kotlin version: 2.0.20
  • Library version: 1.7.3
  • Kotlin platforms: JVM
  • Gradle version: 8.8
@shanshin shanshin self-assigned this Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants