CORDA-1545 - Arrays of primitive byte arrays don't deserialize (#3249)

At serialization time we incorrectly encode the type as byte[p][] instead
of binary[]

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ArraySerializer.kt

@@ -1,5 +1,9 @@
 package net.corda.nodeapi.internal.serialization.amqp
 
+import net.corda.core.utilities.contextLogger
+import net.corda.core.utilities.debug
+import net.corda.core.utilities.loggerFor
+import net.corda.core.utilities.trace
 import org.apache.qpid.proton.amqp.Symbol
 import org.apache.qpid.proton.codec.Data
 import java.io.NotSerializableException
@@ -10,28 +14,48 @@ import java.lang.reflect.Type
  */
 open class ArraySerializer(override val type: Type, factory: SerializerFactory) : AMQPSerializer<Any> {
     companion object {
-        fun make(type: Type, factory: SerializerFactory) = when (type) {
+        fun make(type: Type, factory: SerializerFactory) : AMQPSerializer<Any> {
-            Array<Char>::class.java -> CharArraySerializer(factory)
+            contextLogger().debug { "Making array serializer, typename=${type.typeName}" }
-            else -> ArraySerializer(type, factory)
+            return when (type) {
+                Array<Char>::class.java -> CharArraySerializer(factory)
+                else -> ArraySerializer(type, factory)
+            }
         }
     }
 
+    private val logger = loggerFor<ArraySerializer>()
+
     // because this might be an array of array of primitives (to any recursive depth) and
     // because we care that the lowest type is unboxed we can't rely on the inbuilt type
     // id to generate it properly (it will always return [[[Ljava.lang.type -> type[][][]
     // for example).
     //
-    // We *need* to retain knowledge for AMQP deserialization weather that lowest primitive
+    // We *need* to retain knowledge for AMQP deserialization whether that lowest primitive
     // was boxed or unboxed so just infer it recursively.
-    private fun calcTypeName(type: Type): String =
+    private fun calcTypeName(type: Type, debugOffset : Int = 0): String {
-            if (type.componentType().isArray()) {
+        logger.trace { "${"".padStart(debugOffset, ' ') }  calcTypeName - ${type.typeName}" }
-                val typeName = calcTypeName(type.componentType()); "$typeName[]"
+
+        return if (type.componentType().isArray()) {
+            // Special case handler for primitive byte arrays. This is needed because we can silently
+            // coerce a byte[] to our own binary type. Normally, if the component type was itself an
+            // array we'd keep walking down the chain but for byte[] stop here and use binary instead
+            val typeName =  if (SerializerFactory.isPrimitive(type.componentType())) {
+                SerializerFactory.nameForType(type.componentType())
             } else {
-                val arrayType = if (type.asClass()!!.componentType.isPrimitive) "[p]" else "[]"
+                calcTypeName(type.componentType(), debugOffset + 4)
-                "${type.componentType().typeName}$arrayType"
             }
 
-    override val typeDescriptor by lazy { Symbol.valueOf("$DESCRIPTOR_DOMAIN:${fingerprintForType(type, factory)}") }
+            "$typeName[]"
+        } else {
+            val arrayType = if (type.asClass()!!.componentType.isPrimitive) "[p]" else "[]"
+            "${type.componentType().typeName}$arrayType"
+        }
+    }
+
+    override val typeDescriptor by lazy {
+        Symbol.valueOf("$DESCRIPTOR_DOMAIN:${fingerprintForType(type, factory)}")
+    }
+
     internal val elementType: Type by lazy { type.componentType() }
     internal open val typeName by lazy { calcTypeName(type) }
 

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt

@@ -7,6 +7,8 @@ import net.corda.core.serialization.ClassWhitelist
 import net.corda.nodeapi.internal.serialization.carpenter.CarpenterMetaSchema
 import net.corda.nodeapi.internal.serialization.carpenter.ClassCarpenter
 import net.corda.nodeapi.internal.serialization.carpenter.MetaCarpenter
+import net.corda.core.utilities.debug
+import net.corda.core.utilities.loggerFor
 import org.apache.qpid.proton.amqp.*
 import java.io.NotSerializableException
 import java.lang.reflect.*
@@ -59,6 +61,8 @@ open class SerializerFactory(
 
     fun getTransformsCache() = transformsCache
 
+    private val logger by lazy { loggerFor<SerializerFactory>() }
+
     /**
      * Look up, and manufacture if necessary, a serializer for the given type.
      *
@@ -216,6 +220,7 @@ open class SerializerFactory(
     private fun processSchema(schemaAndDescriptor: FactorySchemaAndDescriptor, sentinel: Boolean = false) {
         val metaSchema = CarpenterMetaSchema.newInstance()
         for (typeNotation in schemaAndDescriptor.schemas.schema.types) {
+            logger.debug { "descriptor=${schemaAndDescriptor.typeDescriptor}, typeNotation=${typeNotation.name}" }
             try {
                 val serialiser = processSchemaEntry(typeNotation)
                 // if we just successfully built a serializer for the type but the type fingerprint

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeSimpleTypesTests.kt

@@ -249,6 +249,14 @@ class DeserializeSimpleTypesTests {
         assertEquals(c.c[0], deserializedC.c[0])
         assertEquals(c.c[1], deserializedC.c[1])
         assertEquals(c.c[2], deserializedC.c[2])
+
+        val di = DeserializationInput(sf2)
+        val deserializedC2 = di.deserialize(serialisedC)
+
+        assertEquals(c.c.size, deserializedC2.c.size)
+        assertEquals(c.c[0], deserializedC2.c[0])
+        assertEquals(c.c[1], deserializedC2.c[1])
+        assertEquals(c.c[2], deserializedC2.c[2])
     }
 
     @Test
@@ -490,7 +498,33 @@ class DeserializeSimpleTypesTests {
         assertEquals(3, da2.a?.a?.b)
         assertEquals(2, da2.a?.a?.a?.b)
         assertEquals(1, da2.a?.a?.a?.a?.b)
-
     }
+
+    // Replicates CORDA-1545
+    @Test
+    fun arrayOfByteArray() {
+        class A(val a : Array<ByteArray>)
+
+        val ba1 = ByteArray(3)
+        ba1[0] = 0b0001; ba1[1] = 0b0101; ba1[2] = 0b1111
+
+        val ba2 = ByteArray(3)
+        ba2[0] = 0b1000; ba2[1] = 0b1100; ba2[2] = 0b1110
+
+        val a = A(arrayOf(ba1, ba2))
+
+        val serializedA = TestSerializationOutput(VERBOSE, sf1).serializeAndReturnSchema(a)
+
+        serializedA.schema.types.forEach {
+            println(it)
+        }
+
+        // This not throwing is the point of the test
+        DeserializationInput(sf1).deserialize(serializedA.obj)
+
+        // This not throwing is the point of the test
+        DeserializationInput(sf2).deserialize(serializedA.obj)
+    }
+
 }