mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 11:18:24 +00:00
fix(nwc): two lenient-reader bugs that answered with a wrong number
Audit of the branch before merge. Both are in the readers behind the NIP-47 and
CLINK parsers, where every byte comes from somebody else's wallet, and both
break the one rule that file states: a field of the wrong shape reads as null
and the rest of the message still parses — never a wrong value.
* intOrNull narrowed with Long.toInt(), which truncates to the low 32 bits.
`"limit":4294967296` came back 0 and `"offset":3000000000` came back
negative. A plausible-looking wrong number is the one outcome worse than no
number, and these feed list_transactions paging and CLINK error codes. Out
of Int range now reads as null like every other unrepresentable value.
* doubleOrNull accepted "NaN" and "Infinity", which Kotlin parses happily. A
NaN read out of a peer's JSON propagates silently through every comparison
it touches. Finite values only.
Both were written as failing tests first and watched fail (LenientJsonTest,
new), then fixed. The malformed-input suites for NIP-47 and CLINK still pass.
Also records, in the JacksonMapper serializer guard, the two
OptimizedSerializables deliberately not listed there — CLINK's SatRange, only
ever written as a field by its parent message's kotlinx serializer, and NIP-55's
IntentResult, which goes through JsonMapperNip55. Neither reaches that mapper
today; both would throw if they did, and the next person should not have to
re-derive why that is fine.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DEoxktEZyTrAS33vVZBiwm
This commit is contained in:
+18
-2
@@ -89,9 +89,25 @@ fun JsonObject.longOrNull(key: String): Long? {
|
||||
return raw.toLongOrNull() ?: raw.toDoubleOrNull()?.takeIf { it.isFinite() }?.toLong()
|
||||
}
|
||||
|
||||
fun JsonObject.intOrNull(key: String): Int? = longOrNull(key)?.toInt()
|
||||
/**
|
||||
* Out of Int range reads as null, not as the low 32 bits. `Long.toInt()` truncates:
|
||||
* a `limit` of 2^32 would come back 0 and an `offset` of 3_000_000_000 negative.
|
||||
* Every other reader here answers a value it cannot represent with null, and a
|
||||
* plausible-looking wrong number is the one outcome worse than no number.
|
||||
*/
|
||||
fun JsonObject.intOrNull(key: String): Int? = longOrNull(key)?.takeIf { it >= Int.MIN_VALUE.toLong() && it <= Int.MAX_VALUE.toLong() }?.toInt()
|
||||
|
||||
fun JsonObject.doubleOrNull(key: String): Double? = this[key].asPrimitiveOrNull()?.content?.toDoubleOrNull()
|
||||
/**
|
||||
* Finite values only. `"NaN"` and `"Infinity"` parse as Doubles in Kotlin, and a
|
||||
* NaN read out of a peer's JSON propagates silently through every comparison it
|
||||
* touches — it is not a number the caller can do anything with.
|
||||
*/
|
||||
fun JsonObject.doubleOrNull(key: String): Double? =
|
||||
this[key]
|
||||
.asPrimitiveOrNull()
|
||||
?.content
|
||||
?.toDoubleOrNull()
|
||||
?.takeIf { it.isFinite() }
|
||||
|
||||
/** Accepts `true`/`false`, `"true"`/`"false"`, and the `1`/`0` some wallets send. */
|
||||
fun JsonObject.booleanOrNull(key: String): Boolean? {
|
||||
|
||||
+75
@@ -0,0 +1,75 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.quartz.nip01Core.kotlinSerialization
|
||||
|
||||
import kotlinx.serialization.json.Json
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
|
||||
/**
|
||||
* The readers behind the NIP-47 and CLINK parsers, where every byte comes from
|
||||
* somebody else's wallet. Their contract is that a field of the wrong shape reads
|
||||
* as null and the rest of the message still parses — never a wrong value.
|
||||
*/
|
||||
class LenientJsonTest {
|
||||
private fun obj(json: String) = Json.parseToJsonElement(json) as JsonObject
|
||||
|
||||
@Test
|
||||
fun intOutOfRangeReadsAsNullRatherThanWrapping() {
|
||||
// 2^32 truncates to 0 and 3_000_000_000 to a negative int if the Long is
|
||||
// narrowed with toInt(). A limit of 0 or a negative offset is worse than
|
||||
// no value at all: it is a plausible-looking wrong answer.
|
||||
assertNull(obj("""{"limit":4294967296}""").intOrNull("limit"))
|
||||
assertNull(obj("""{"offset":3000000000}""").intOrNull("offset"))
|
||||
assertNull(obj("""{"n":-3000000000}""").intOrNull("n"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun intInRangeStillReads() {
|
||||
assertEquals(42, obj("""{"n":42}""").intOrNull("n"))
|
||||
assertEquals(Int.MAX_VALUE, obj("""{"n":2147483647}""").intOrNull("n"))
|
||||
assertEquals(Int.MIN_VALUE, obj("""{"n":-2147483648}""").intOrNull("n"))
|
||||
// the same string/float tolerance longOrNull has
|
||||
assertEquals(12, obj("""{"n":"12"}""").intOrNull("n"))
|
||||
assertEquals(12, obj("""{"n":12.0}""").intOrNull("n"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun longAcceptsTheThreeShapesWalletsSend() {
|
||||
assertEquals(12L, obj("""{"n":12}""").longOrNull("n"))
|
||||
assertEquals(12L, obj("""{"n":"12"}""").longOrNull("n"))
|
||||
assertEquals(12L, obj("""{"n":12.0}""").longOrNull("n"))
|
||||
assertNull(obj("""{"n":{"nested":1}}""").longOrNull("n"))
|
||||
assertNull(obj("""{"n":null}""").longOrNull("n"))
|
||||
assertNull(obj("""{}""").longOrNull("n"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doubleRejectsNonFiniteText() {
|
||||
// "NaN" and "Infinity" parse as Doubles in Kotlin but are not values any
|
||||
// caller can do arithmetic with.
|
||||
assertNull(obj("""{"n":"NaN"}""").doubleOrNull("n"))
|
||||
assertNull(obj("""{"n":"Infinity"}""").doubleOrNull("n"))
|
||||
assertEquals(1.5, obj("""{"n":1.5}""").doubleOrNull("n"))
|
||||
}
|
||||
}
|
||||
+6
@@ -199,6 +199,12 @@ class JacksonMapper {
|
||||
* [BunkerRequest] and [BunkerResponse] are named instead of their [BunkerMessage]
|
||||
* parent on purpose: the parent has no serializer of its own, so a third subclass
|
||||
* should fail here rather than quietly bean-serialize.
|
||||
*
|
||||
* Two OptimizedSerializables are deliberately absent, and both would throw if
|
||||
* they ever arrived here: CLINK's SatRange, which is only ever written as a
|
||||
* field of an Offer/Debit/Manage message by that message's kotlinx serializer,
|
||||
* and NIP-55's IntentResult, which goes through JsonMapperNip55 and its own
|
||||
* registered serializer. Neither reaches this mapper today.
|
||||
*/
|
||||
private fun checkSerializable(value: OptimizedSerializable) {
|
||||
val hasSerializer =
|
||||
|
||||
Reference in New Issue
Block a user