From f818be0e8197a6818b55ac5fd2d269d7cb0f9660 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Wed, 20 Jan 2021 12:14:01 +0300 Subject: Throw SerializationException when encountering an invalid boolean in JSON Also, narrow down the type of swallowed exceptions Fixes #1261 --- .../src/kotlinx/serialization/json/JsonElement.kt | 2 +- .../serialization/json/internal/StreamingJsonDecoder.kt | 12 +++++++----- .../src/kotlinx/serialization/json/internal/StringOps.kt | 15 --------------- .../serialization/json/internal/TreeJsonDecoder.kt | 6 ++++-- .../serialization/json/JsonParserFailureModesTest.kt | 9 +++++++++ 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt b/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt index 0766b88c..7d213304 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt @@ -221,7 +221,7 @@ public val JsonPrimitive.floatOrNull: Float? get() = content.toFloatOrNull() * Returns content of current element as boolean * @throws IllegalStateException if current element doesn't represent boolean */ -public val JsonPrimitive.boolean: Boolean get() = content.toBooleanStrict() +public val JsonPrimitive.boolean: Boolean get() = content.toBooleanStrictOrNull() ?: throw IllegalStateException("$this does not represent a Boolean") /** * Returns content of current element as boolean or `null` if current element is not a valid representation of boolean diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt index 864f0a8e..97e66454 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -173,11 +173,13 @@ internal class StreamingJsonDecoder internal constructor( * We prohibit non true/false boolean literals at all as it is considered way too error-prone, * but allow quoted literal in relaxed mode for booleans. */ - return if (configuration.isLenient) { - reader.takeString().toBooleanStrict() + val string = if (configuration.isLenient) { + reader.takeString() } else { - reader.takeBooleanStringUnquoted().toBooleanStrict() + reader.takeBooleanStringUnquoted() } + string.toBooleanStrictOrNull()?.let { return it } + reader.fail("Failed to parse type 'boolean' for input '$string'") } /* @@ -216,8 +218,8 @@ internal class StreamingJsonDecoder internal constructor( private inline fun String.parse(type: String, block: String.() -> T): T { try { return block() - } catch (e: Throwable) { - reader.fail("Failed to parse '$type'") + } catch (e: IllegalArgumentException) { + reader.fail("Failed to parse type '$type' for input '$this'") } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StringOps.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StringOps.kt index a16bd1b4..29e463f5 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StringOps.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StringOps.kt @@ -52,12 +52,6 @@ internal fun StringBuilder.printQuoted(value: String) { append(STRING) } -/** - * Returns `true` if the contents of this string is equal to the word "true", ignoring case, `false` if content equals "false", - * and throws [IllegalStateException] otherwise. - */ -internal fun String.toBooleanStrict(): Boolean = toBooleanStrictOrNull() ?: throw IllegalStateException("$this does not represent a Boolean") - /** * Returns `true` if the contents of this string is equal to the word "true", ignoring case, `false` if content equals "false", * and returns `null` otherwise. @@ -67,12 +61,3 @@ internal fun String.toBooleanStrictOrNull(): Boolean? = when { this.equals("false", ignoreCase = true) -> false else -> null } - -internal fun shouldBeQuoted(str: String): Boolean { - if (str == NULL) return true - for (ch in str) { - if (charToTokenClass(ch) != TC_OTHER) return true - } - - return false -} diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt index 6175c3e6..1f86043b 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt @@ -97,7 +97,9 @@ private sealed class AbstractJsonTreeDecoder( -1, "Boolean literal for key '$tag' should be unquoted.\n$lenientHint", currentObject().toString() ) } - return value.boolean + return value.primitive("boolean") { + booleanOrNull ?: throw IllegalArgumentException() /* Will be handled by 'primitive' */ + } } override fun decodeTaggedByte(tag: String) = getValue(tag).primitive("byte") { int.toByte() } @@ -124,7 +126,7 @@ private sealed class AbstractJsonTreeDecoder( private inline fun JsonPrimitive.primitive(primitive: String, block: JsonPrimitive.() -> T): T { try { return block() - } catch (e: Throwable) { + } catch (e: IllegalArgumentException) { throw JsonDecodingException(-1, "Failed to parse '$primitive'", currentObject().toString()) } } diff --git a/formats/json/commonTest/src/kotlinx/serialization/json/JsonParserFailureModesTest.kt b/formats/json/commonTest/src/kotlinx/serialization/json/JsonParserFailureModesTest.kt index e27de70c..b20f0385 100644 --- a/formats/json/commonTest/src/kotlinx/serialization/json/JsonParserFailureModesTest.kt +++ b/formats/json/commonTest/src/kotlinx/serialization/json/JsonParserFailureModesTest.kt @@ -64,4 +64,13 @@ class JsonParserFailureModesTest : JsonTestBase() { assertFailsWith { default.decodeFromString(Holder.serializer(), """{""", it) } } } + + @Serializable + class BooleanHolder(val b: Boolean) + + @Test + fun testBoolean() = parametrizedTest { + assertFailsWith { default.decodeFromString(BooleanHolder.serializer(), """{"b": fals}""", it) } + assertFailsWith { default.decodeFromString(BooleanHolder.serializer(), """{"b": 123}""", it) } + } } -- cgit v1.2.3