diff --git a/amethyst/proguard-rules.pro b/amethyst/proguard-rules.pro index a9decf35f4..5f6e347e2f 100644 --- a/amethyst/proguard-rules.pro +++ b/amethyst/proguard-rules.pro @@ -63,10 +63,35 @@ # Console only auto-deobfuscates the AAB it was given. -keepattributes SourceFile,LineNumberTable -# Annotations (jackson-module-kotlin reads @kotlin.Metadata; Jackson mixins and -# kotlinx.serialization read their own), generic signatures, and the -# inner/enclosing-class links that R8 requires alongside Signature. --keepattributes *Annotation*,Signature,Exceptions,InnerClasses,EnclosingMethod +# Generic signatures, plus the inner/enclosing-class links that travel with them. +# +# Signature is the load-bearing one: jacksonTypeRefOf() resolves generic types +# through it at 18 call sites, and without it List erases to List and every +# element comes back a LinkedHashMap. +# +# All three are ALSO in AGP's proguard-android-optimize.txt, which keeps +# AnnotationDefault, EnclosingMethod, InnerClasses, Signature and the three +# RuntimeVisible* annotation attributes. They stay spelled out here anyway: the +# duplicate is free (measured at 0 bytes) and it means a change to AGP's default +# file cannot quietly take Signature away from Jackson. +# +# Two attributes this line used to carry are gone, both measured on the arm64 +# release DEX: +# +# * `*Annotation*` — over AGP's default its only contribution was the +# RuntimeInvisible* variants, which by definition cannot be read at runtime. +# Dropping it produced a byte-identical DEX (31,390,048 either way). Nothing +# we ship reads an annotation reflectively, and the libraries that do +# (kotlinx.serialization, appfunctions, AppSearch) match on RuntimeVisible*, +# which AGP already keeps. +# * `Exceptions` — @Throws metadata for 31 methods in shipped code, read by +# Java-interop compilers and by nothing at runtime. Worth 692 bytes. +# +# For the record, since it was wrong here for a while: this line used to credit +# jackson-module-kotlin with reading @kotlin.Metadata through it. That module no +# longer ships, and the Jackson mixins it also named were deleted along with the +# NWC Jackson path. +-keepattributes Signature,InnerClasses,EnclosingMethod # LocalVariableTable, LocalVariableTypeTable, MethodParameters and # -keepparameternames used to be kept here as well. They are debug metadata: diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt index ce818cf7f3..570eb1d21c 100644 --- a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt @@ -125,7 +125,12 @@ class JacksonMapper { fun fromJsonToEventList(json: String): List = mapper.readValue(json, eventListTypeInstance) /** - * The types this mapper has a registered deserializer for. + * The types this mapper has a registered deserializer for, by EXACT class. + * + * Exact is right here: `SimpleModule.addDeserializer(Event::class)` binds that + * class alone — Jackson's SimpleDeserializers does not walk up a hierarchy on + * the read side — so a subclass passed to [fromJsonTo] really would be + * unbound. The serializer side is the mirror image; see [checkSerializable]. * * [fromJsonTo] is generic, so nothing in the type system stops a new * [OptimizedSerializable] being passed to it. Jackson would answer by binding @@ -165,6 +170,43 @@ class JacksonMapper { } } + /** + * The write-side twin of [checkRegistered], and the reason it is an `is` chain + * rather than a set: Jackson's SimpleSerializers DOES walk the hierarchy, so + * the serializer registered for [Event] serves every event kind and [Command]'s + * serves NegMsgMessage. Comparing exact classes here would reject all of them. + * + * Without this, [toJson] has the same hole [fromJsonTo] had. Hand it an + * [OptimizedSerializable] with no registered serializer and Jackson falls back + * to bean introspection, naming each field after its getter — names R8 renames, + * so a release build writes `{"a":…}` onto the wire while the debug build looks + * perfect. + * + * [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. + */ + private fun checkSerializable(value: OptimizedSerializable) { + val hasSerializer = + value is Event || + value is Filter || + value is Message || + value is Command || + value is EventTemplate<*> || + value is Rumor || + value is BunkerRequest || + value is BunkerResponse + + if (!hasSerializer) { + throw IllegalArgumentException( + "No Jackson serializer is registered for ${value::class}, so Jackson would " + + "serialize it reflectively and emit obfuscated field names in a release " + + "build. Register one in JacksonMapper, or route the type at " + + "KotlinSerializationMapper in OptimizedJsonMapper.", + ) + } + } + inline fun fromJsonTo(json: String): T { checkRegistered(T::class) return mapper.readValue(json, jacksonTypeRefOf()) @@ -206,7 +248,10 @@ class JacksonMapper { fun toJson(event: ObjectNode?): String = mapper.writeValueAsString(event) - fun toJson(value: OptimizedSerializable): String = mapper.writeValueAsString(value) + fun toJson(value: OptimizedSerializable): String { + checkSerializable(value) + return mapper.writeValueAsString(value) + } fun toJson(tags: TagArray): String = mapper.writeValueAsString(tags) } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonSerializerGuardTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonSerializerGuardTest.kt new file mode 100644 index 0000000000..8c01e0e83e --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonSerializerGuardTest.kt @@ -0,0 +1,67 @@ +/* + * 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.jackson + +import com.vitorpamplona.quartz.nip01Core.core.OptimizedSerializable +import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.CloseCmd +import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * [JacksonMapper.toJson] refuses a type it has no registered serializer for. + * + * Bean introspection would otherwise name every field after its getter, and R8 + * renames getters — so the JSON is correct in debug and one-letter garbage in a + * release build. The failure has to happen here, on the first call, or it happens + * on a user's device. + */ +class JacksonSerializerGuardTest { + private class NotRegistered( + val someField: String = "value", + ) : OptimizedSerializable + + @Test + fun unregisteredTypeIsRefused() { + val e = assertFailsWith { JacksonMapper.toJson(NotRegistered()) } + assertTrue(e.message!!.contains("No Jackson serializer is registered"), e.message!!) + } + + /** + * The guard is an `is` chain because Jackson's SimpleSerializers walks the + * hierarchy: CLOSE has no serializer of its own, it rides Command's. An + * exact-class check would reject it — and with it every relay command the + * client sends. + */ + @Test + fun subclassesOfARegisteredRootStillSerialize() { + assertEquals("""["CLOSE","sub-1"]""", JacksonMapper.toJson(CloseCmd("sub-1"))) + } + + @Test + fun registeredRootsStillSerialize() { + assertEquals("""{"kinds":[1]}""", JacksonMapper.toJson(Filter(kinds = listOf(1)))) + assertTrue(JacksonMapper.toJson(BunkerRequest("id-1", "connect", arrayOf("a"))).contains("\"connect\"")) + } +}