mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix: close the write half of the Jackson reflective-binding trap
fromJsonTo has refused unregistered types since2d1e3989, but toJson still took anything. 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 the debug build looks perfect and the release build writes {"a":…} onto the wire. Same trap, other direction. The guard is an `is` chain rather than a set of classes, and that asymmetry with the read side is deliberate: Jackson's SimpleSerializers walks the hierarchy, so Event's serializer serves every event kind and Command's serves NegMsgMessage, while SimpleDeserializers does NOT walk up, which is why the deserializer check compares exact classes. Comparing exact classes on the write side would have rejected every relay command the client sends. The test pins both halves. BunkerRequest and BunkerResponse are listed instead of their BunkerMessage parent on purpose — the parent has no serializer of its own, so a third subclass should fail the guard rather than quietly bean-serialize. Also corrects -keepattributes, which was documented wrongly and measured never: * `*Annotation*` is gone. AGP's proguard-android-optimize.txt already keeps AnnotationDefault, Signature, InnerClasses, EnclosingMethod and the three RuntimeVisible* attributes, so our glob only added the RuntimeInvisible* variants — unreadable at runtime by definition. Building without it produced a byte-identical DEX: 31,390,048 both ways. * `Exceptions` is gone. @Throws metadata for 31 methods, read by Java-interop compilers and nothing at runtime. 692 bytes. * Signature, InnerClasses and EnclosingMethod stay spelled out even though AGP duplicates them: the duplicate is free, and jacksonTypeRefOf() needs Signature at 18 call sites — without it List<Event> erases to List and every element comes back a LinkedHashMap. Not something to leave to another project's default file. The comment claimed jackson-module-kotlin read @kotlin.Metadata through this line; that module stopped shipping ine7d3bcdc, and the mixins it also named went with the NWC Jackson path. Verified: 21/21 contract checks on a full playRelease assemble carrying these rules, and the guard test passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DEoxktEZyTrAS33vVZBiwm
This commit is contained in:
Vendored
+29
-4
@@ -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<Event> 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:
|
||||
|
||||
+47
-2
@@ -125,7 +125,12 @@ class JacksonMapper {
|
||||
fun fromJsonToEventList(json: String): List<Event> = 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 <reified T : OptimizedSerializable> fromJsonTo(json: String): T {
|
||||
checkRegistered(T::class)
|
||||
return mapper.readValue(json, jacksonTypeRefOf<T>())
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
+67
@@ -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<IllegalArgumentException> { 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\""))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user