mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 16:33:27 +00:00
fix: harden LIMITS parsing and dedup, per audit
Addresses the four findings from the branch audit:
- Make the kotlinx codec (LimitsKSerializer, the iOS/native incoming path)
as lenient as the Jackson path: mistyped fields degrade to null, non-int
array elements are skipped, and a payload-less ["LIMITS"] frame yields an
empty message instead of throwing. Guard the payload access in
MessageKSerializer and MessageDeserializer likewise.
- Make the Jackson reads (LimitsDeserializer) type-checked so an explicit
JSON null or wrong-typed value stays null ("unspecified — keep previous")
instead of coercing to false/0. Both codecs now behave identically.
- LimitsMessage -> data class, so StateFlow.distinctUntilChanged in
RelayLimitsTracker suppresses no-op emissions when a relay re-advertises
identical limits, and tests get value equality.
- Drop the stale "NIP-22" labels (LIMITS is nostr-protocol/nips#1434, not
NIP-22) from LimitsKSerializer, MessageKSerializer and MessageSerializer.
Tests: added mistyped/null-field and payload-less coverage on both the
Jackson and kotlinx paths, plus a value-equality check. quartz:jvmTest
(RelayWireErgonomicsTest 11, KotlinSerializationMapperTest 57,
RelayLimitsTrackerTest 5) and amethyst play compile green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01464jkunWPtYhTReoc3fUQQ
This commit is contained in:
+40
-27
@@ -23,19 +23,35 @@ package com.vitorpamplona.quartz.nip01Core.kotlinSerialization
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.LimitsMessage
|
||||
import kotlinx.serialization.json.JsonArray
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
import kotlinx.serialization.json.JsonPrimitive
|
||||
import kotlinx.serialization.json.add
|
||||
import kotlinx.serialization.json.addJsonArray
|
||||
import kotlinx.serialization.json.boolean
|
||||
import kotlinx.serialization.json.booleanOrNull
|
||||
import kotlinx.serialization.json.buildJsonObject
|
||||
import kotlinx.serialization.json.int
|
||||
import kotlinx.serialization.json.jsonArray
|
||||
import kotlinx.serialization.json.jsonPrimitive
|
||||
import kotlinx.serialization.json.long
|
||||
import kotlinx.serialization.json.contentOrNull
|
||||
import kotlinx.serialization.json.intOrNull
|
||||
import kotlinx.serialization.json.longOrNull
|
||||
import kotlinx.serialization.json.put
|
||||
import kotlinx.serialization.json.putJsonArray
|
||||
|
||||
/** kotlinx-serialization codec for the NIP-22 `LIMITS` object payload. */
|
||||
/** kotlinx-serialization codec for the `LIMITS` object payload. */
|
||||
object LimitsKSerializer {
|
||||
// Tolerant readers so a malformed/mistyped field degrades to null instead of
|
||||
// throwing: this is the iOS/native incoming-parse path, and it must match the
|
||||
// leniency of the Jackson path (LimitsDeserializer) used on jvmAndroid.
|
||||
private fun JsonObject.bool(key: String): Boolean? = (this[key] as? JsonPrimitive)?.booleanOrNull
|
||||
|
||||
private fun JsonObject.int(key: String): Int? = (this[key] as? JsonPrimitive)?.intOrNull
|
||||
|
||||
private fun JsonObject.long(key: String): Long? = (this[key] as? JsonPrimitive)?.longOrNull
|
||||
|
||||
private fun JsonObject.intList(key: String): List<Int>? = (this[key] as? JsonArray)?.mapNotNull { (it as? JsonPrimitive)?.intOrNull }
|
||||
|
||||
private fun JsonObject.tagList(key: String): List<List<String>>? =
|
||||
(this[key] as? JsonArray)?.map { tag ->
|
||||
(tag as? JsonArray)?.mapNotNull { (it as? JsonPrimitive)?.contentOrNull } ?: emptyList()
|
||||
}
|
||||
|
||||
fun serializeToElement(value: LimitsMessage): JsonObject =
|
||||
buildJsonObject {
|
||||
// Only emit the fields the relay actually set; absent limits stay absent.
|
||||
@@ -71,26 +87,23 @@ object LimitsKSerializer {
|
||||
|
||||
fun deserializeFromElement(jsonObject: JsonObject): LimitsMessage =
|
||||
LimitsMessage(
|
||||
canWrite = jsonObject["can_write"]?.jsonPrimitive?.boolean,
|
||||
canRead = jsonObject["can_read"]?.jsonPrimitive?.boolean,
|
||||
authForRead = jsonObject["auth_for_read"]?.jsonPrimitive?.boolean,
|
||||
authForWrite = jsonObject["auth_for_write"]?.jsonPrimitive?.boolean,
|
||||
acceptedEventKinds = jsonObject["accepted_event_kinds"]?.jsonArray?.map { it.jsonPrimitive.int },
|
||||
blockedEventKinds = jsonObject["blocked_event_kinds"]?.jsonArray?.map { it.jsonPrimitive.int },
|
||||
minPowDifficulty = jsonObject["min_pow_difficulty"]?.jsonPrimitive?.int,
|
||||
maxMessageLength = jsonObject["max_message_length"]?.jsonPrimitive?.int,
|
||||
maxSubscriptions = jsonObject["max_subscriptions"]?.jsonPrimitive?.int,
|
||||
maxFilters = jsonObject["max_filters"]?.jsonPrimitive?.int,
|
||||
maxLimit = jsonObject["max_limit"]?.jsonPrimitive?.int,
|
||||
maxEventTags = jsonObject["max_event_tags"]?.jsonPrimitive?.int,
|
||||
maxContentLength = jsonObject["max_content_length"]?.jsonPrimitive?.int,
|
||||
createdAtMsecsAgo = jsonObject["created_at_msecs_ago"]?.jsonPrimitive?.long,
|
||||
createdAtMsecsAhead = jsonObject["created_at_msecs_ahead"]?.jsonPrimitive?.long,
|
||||
filterRateLimit = jsonObject["filter_rate_limit"]?.jsonPrimitive?.long,
|
||||
publishingRateLimit = jsonObject["publishing_rate_limit"]?.jsonPrimitive?.long,
|
||||
requiredTags =
|
||||
jsonObject["required_tags"]?.jsonArray?.map { tag ->
|
||||
(tag as JsonArray).map { it.jsonPrimitive.content }
|
||||
},
|
||||
canWrite = jsonObject.bool("can_write"),
|
||||
canRead = jsonObject.bool("can_read"),
|
||||
authForRead = jsonObject.bool("auth_for_read"),
|
||||
authForWrite = jsonObject.bool("auth_for_write"),
|
||||
acceptedEventKinds = jsonObject.intList("accepted_event_kinds"),
|
||||
blockedEventKinds = jsonObject.intList("blocked_event_kinds"),
|
||||
minPowDifficulty = jsonObject.int("min_pow_difficulty"),
|
||||
maxMessageLength = jsonObject.int("max_message_length"),
|
||||
maxSubscriptions = jsonObject.int("max_subscriptions"),
|
||||
maxFilters = jsonObject.int("max_filters"),
|
||||
maxLimit = jsonObject.int("max_limit"),
|
||||
maxEventTags = jsonObject.int("max_event_tags"),
|
||||
maxContentLength = jsonObject.int("max_content_length"),
|
||||
createdAtMsecsAgo = jsonObject.long("created_at_msecs_ago"),
|
||||
createdAtMsecsAhead = jsonObject.long("created_at_msecs_ahead"),
|
||||
filterRateLimit = jsonObject.long("filter_rate_limit"),
|
||||
publishingRateLimit = jsonObject.long("publishing_rate_limit"),
|
||||
requiredTags = jsonObject.tagList("required_tags"),
|
||||
)
|
||||
}
|
||||
|
||||
+5
-2
@@ -39,6 +39,7 @@ import kotlinx.serialization.encoding.Decoder
|
||||
import kotlinx.serialization.encoding.Encoder
|
||||
import kotlinx.serialization.json.JsonDecoder
|
||||
import kotlinx.serialization.json.JsonEncoder
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
import kotlinx.serialization.json.JsonPrimitive
|
||||
import kotlinx.serialization.json.boolean
|
||||
import kotlinx.serialization.json.buildJsonArray
|
||||
@@ -99,7 +100,7 @@ object MessageKSerializer : KSerializer<Message> {
|
||||
}
|
||||
|
||||
is LimitsMessage -> {
|
||||
// NIP-22 wire format: ["LIMITS", { <limit_properties> }]
|
||||
// LIMITS wire format: ["LIMITS", { <limit_properties> }]
|
||||
add(LimitsKSerializer.serializeToElement(value))
|
||||
}
|
||||
|
||||
@@ -167,7 +168,9 @@ object MessageKSerializer : KSerializer<Message> {
|
||||
}
|
||||
|
||||
LimitsMessage.LABEL -> {
|
||||
LimitsKSerializer.deserializeFromElement(array[1].jsonObject)
|
||||
// Tolerate a payload-less or malformed ["LIMITS"] frame instead of throwing.
|
||||
val payload = array.getOrNull(1) as? JsonObject ?: JsonObject(emptyMap())
|
||||
LimitsKSerializer.deserializeFromElement(payload)
|
||||
}
|
||||
|
||||
NegMsgMessage.LABEL -> {
|
||||
|
||||
+1
-1
@@ -35,7 +35,7 @@ import androidx.compose.runtime.Immutable
|
||||
* payload on the relay connection and apply it when sending `EVENT`s and `REQ`s.
|
||||
*/
|
||||
@Immutable
|
||||
class LimitsMessage(
|
||||
data class LimitsMessage(
|
||||
/** Whether clients may publish events to this relay. */
|
||||
val canWrite: Boolean? = null,
|
||||
/** Whether clients may send `REQ` commands to this relay. */
|
||||
|
||||
+35
@@ -172,4 +172,39 @@ class RelayWireErgonomicsTest {
|
||||
assertNull(parsed.canRead)
|
||||
assertNull(parsed.maxLimit)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun limitsMessageToleratesPayloadlessFrame() {
|
||||
// A malformed ["LIMITS"] with no object must not throw; it yields an empty message.
|
||||
val parsed = Message.fromJson("""["LIMITS"]""")
|
||||
assertTrue(parsed is LimitsMessage)
|
||||
assertNull(parsed.canRead)
|
||||
assertNull(parsed.maxLimit)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun limitsMessageToleratesMistypedAndNullFields() {
|
||||
// Wrong-typed and explicitly-null fields degrade to null ("unspecified"),
|
||||
// never to false/0, and never throw.
|
||||
val json =
|
||||
"""["LIMITS",{"can_write":null,"max_limit":"lots","max_filters":true,""" +
|
||||
""""accepted_event_kinds":[1,"x",3],"required_tags":["oops",["t","nostr"]]}]"""
|
||||
val parsed = Message.fromJson(json)
|
||||
assertTrue(parsed is LimitsMessage)
|
||||
assertNull(parsed.canWrite, "explicit null stays null, not false")
|
||||
assertNull(parsed.maxLimit, "a string is not an int -> null, not 0")
|
||||
assertNull(parsed.maxFilters, "a boolean is not an int -> null")
|
||||
assertEquals(listOf(1, 3), parsed.acceptedEventKinds, "non-int array elements are skipped")
|
||||
// The bare "oops" string is not a tag array -> empty; the real pair survives.
|
||||
assertEquals(listOf(emptyList(), listOf("t", "nostr")), parsed.requiredTags)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun limitsMessageHasValueEquality() {
|
||||
// data class equality lets StateFlow.distinctUntilChanged suppress no-op re-advertisements.
|
||||
assertEquals(
|
||||
LimitsMessage(canWrite = true, maxLimit = 200, acceptedEventKinds = listOf(1, 2)),
|
||||
LimitsMessage(canWrite = true, maxLimit = 200, acceptedEventKinds = listOf(1, 2)),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+27
-17
@@ -24,32 +24,42 @@ import com.fasterxml.jackson.databind.JsonNode
|
||||
|
||||
class LimitsDeserializer {
|
||||
companion object {
|
||||
private fun JsonNode.intList(field: String): List<Int>? = get(field)?.takeIf { it.isArray }?.map { it.asInt() }
|
||||
// Type-checked readers: a missing field, an explicit JSON null, or a
|
||||
// wrong-typed value all yield null ("unspecified — keep the previous
|
||||
// value") rather than coercing to false/0. Keeps this in step with the
|
||||
// kotlinx path (LimitsKSerializer) used on iOS/native.
|
||||
private fun JsonNode.bool(field: String): Boolean? = get(field)?.takeIf { it.isBoolean }?.booleanValue()
|
||||
|
||||
private fun JsonNode.int(field: String): Int? = get(field)?.takeIf { it.isNumber }?.intValue()
|
||||
|
||||
private fun JsonNode.long(field: String): Long? = get(field)?.takeIf { it.isNumber }?.longValue()
|
||||
|
||||
private fun JsonNode.intList(field: String): List<Int>? = get(field)?.takeIf { it.isArray }?.mapNotNull { it.takeIf { n -> n.isNumber }?.intValue() }
|
||||
|
||||
private fun JsonNode.requiredTags(field: String): List<List<String>>? =
|
||||
get(field)?.takeIf { it.isArray }?.map { tag ->
|
||||
tag.map { it.asText() }
|
||||
if (tag.isArray) tag.mapNotNull { it.takeIf { n -> n.isValueNode }?.asText() } else emptyList()
|
||||
}
|
||||
|
||||
fun fromJson(jsonObject: JsonNode): LimitsMessage =
|
||||
LimitsMessage(
|
||||
canWrite = jsonObject.get("can_write")?.asBoolean(),
|
||||
canRead = jsonObject.get("can_read")?.asBoolean(),
|
||||
authForRead = jsonObject.get("auth_for_read")?.asBoolean(),
|
||||
authForWrite = jsonObject.get("auth_for_write")?.asBoolean(),
|
||||
canWrite = jsonObject.bool("can_write"),
|
||||
canRead = jsonObject.bool("can_read"),
|
||||
authForRead = jsonObject.bool("auth_for_read"),
|
||||
authForWrite = jsonObject.bool("auth_for_write"),
|
||||
acceptedEventKinds = jsonObject.intList("accepted_event_kinds"),
|
||||
blockedEventKinds = jsonObject.intList("blocked_event_kinds"),
|
||||
minPowDifficulty = jsonObject.get("min_pow_difficulty")?.asInt(),
|
||||
maxMessageLength = jsonObject.get("max_message_length")?.asInt(),
|
||||
maxSubscriptions = jsonObject.get("max_subscriptions")?.asInt(),
|
||||
maxFilters = jsonObject.get("max_filters")?.asInt(),
|
||||
maxLimit = jsonObject.get("max_limit")?.asInt(),
|
||||
maxEventTags = jsonObject.get("max_event_tags")?.asInt(),
|
||||
maxContentLength = jsonObject.get("max_content_length")?.asInt(),
|
||||
createdAtMsecsAgo = jsonObject.get("created_at_msecs_ago")?.asLong(),
|
||||
createdAtMsecsAhead = jsonObject.get("created_at_msecs_ahead")?.asLong(),
|
||||
filterRateLimit = jsonObject.get("filter_rate_limit")?.asLong(),
|
||||
publishingRateLimit = jsonObject.get("publishing_rate_limit")?.asLong(),
|
||||
minPowDifficulty = jsonObject.int("min_pow_difficulty"),
|
||||
maxMessageLength = jsonObject.int("max_message_length"),
|
||||
maxSubscriptions = jsonObject.int("max_subscriptions"),
|
||||
maxFilters = jsonObject.int("max_filters"),
|
||||
maxLimit = jsonObject.int("max_limit"),
|
||||
maxEventTags = jsonObject.int("max_event_tags"),
|
||||
maxContentLength = jsonObject.int("max_content_length"),
|
||||
createdAtMsecsAgo = jsonObject.long("created_at_msecs_ago"),
|
||||
createdAtMsecsAhead = jsonObject.long("created_at_msecs_ahead"),
|
||||
filterRateLimit = jsonObject.long("filter_rate_limit"),
|
||||
publishingRateLimit = jsonObject.long("publishing_rate_limit"),
|
||||
requiredTags = jsonObject.requiredTags("required_tags"),
|
||||
)
|
||||
}
|
||||
|
||||
+7
-4
@@ -104,10 +104,13 @@ class MessageDeserializer : StdDeserializer<Message>(Message::class.java) {
|
||||
}
|
||||
|
||||
LimitsMessage.LABEL -> {
|
||||
jp.nextToken()
|
||||
val result: JsonNode = jp.codec.readTree(jp)
|
||||
|
||||
LimitsDeserializer.fromJson(result)
|
||||
// Tolerate a payload-less ["LIMITS"] frame instead of throwing.
|
||||
if (jp.nextToken() == JsonToken.START_OBJECT) {
|
||||
val result: JsonNode = jp.codec.readTree(jp)
|
||||
LimitsDeserializer.fromJson(result)
|
||||
} else {
|
||||
LimitsMessage()
|
||||
}
|
||||
}
|
||||
|
||||
NegMsgMessage.LABEL -> {
|
||||
|
||||
+1
-1
@@ -81,7 +81,7 @@ class MessageSerializer : StdSerializer<Message>(Message::class.java) {
|
||||
}
|
||||
|
||||
is LimitsMessage -> {
|
||||
// NIP-22 wire format: ["LIMITS", { <limit_properties> }]. Only
|
||||
// LIMITS wire format: ["LIMITS", { <limit_properties> }]. Only
|
||||
// the fields the relay set are emitted; absent limits stay absent.
|
||||
gen.writeStartObject()
|
||||
msg.canWrite?.let { gen.writeBooleanField("can_write", it) }
|
||||
|
||||
+20
@@ -543,6 +543,26 @@ class KotlinSerializationMapperTest {
|
||||
assertNull(deserialized.maxFilters)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deserializeLimitsMessageToleratesMistypedAndMissingPayload() {
|
||||
// The kotlinx path is the iOS/native incoming parser; a mistyped field or a
|
||||
// payload-less ["LIMITS"] must degrade to null / empty, matching Jackson,
|
||||
// rather than throwing.
|
||||
val mistyped =
|
||||
KotlinSerializationMapper.fromJsonToMessage(
|
||||
"""["LIMITS",{"can_write":null,"max_limit":"lots","accepted_event_kinds":[1,"x",3],"required_tags":["oops",["t","nostr"]]}]""",
|
||||
)
|
||||
assertTrue(mistyped is LimitsMessage)
|
||||
assertNull(mistyped.canWrite)
|
||||
assertNull(mistyped.maxLimit)
|
||||
assertEquals(listOf(1, 3), mistyped.acceptedEventKinds)
|
||||
assertEquals(listOf(emptyList(), listOf("t", "nostr")), mistyped.requiredTags)
|
||||
|
||||
val payloadless = KotlinSerializationMapper.fromJsonToMessage("""["LIMITS"]""")
|
||||
assertTrue(payloadless is LimitsMessage)
|
||||
assertNull(payloadless.maxLimit)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun crossDeserializationLimitsMessage() {
|
||||
val msg =
|
||||
|
||||
Reference in New Issue
Block a user