diff --git a/core/src/main/kotlin/net/corda/core/serialization/SerializationAPI.kt b/core/src/main/kotlin/net/corda/core/serialization/SerializationAPI.kt index 9781e7db8c..5fb794c78e 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/SerializationAPI.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/SerializationAPI.kt @@ -192,7 +192,7 @@ interface SerializationContext { /** * The use case that we are serializing for, since it influences the implementations chosen. */ - enum class UseCase { P2P, RPCServer, RPCClient, Storage, Checkpoint } + enum class UseCase { P2P, RPCServer, RPCClient, Storage, Checkpoint, Testing } } /** diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 9f66bd36ed..7c1abfc4f3 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -7,6 +7,16 @@ Unreleased Here are brief summaries of what's changed between each snapshot release. This includes guidance on how to upgrade code from the previous milestone release. +* Serializing an inner class (non-static nested class in Java, inner class in Kotlin) will be rejected explicitly by the serialization + framework. Prior to this change it didn't work, but the error thrown was opaque (complaining about too few arguments + to a constructor). Whilst this was possible in the older Kryo implementation (Kryo passing null as the synthesised + reference to the outer class) as per the Java documentation `here `_ + we are disallowing this as the paradigm in general makes little sense for Contract States + +* Fix CORDA-1258. Custom serializers were spuriously registered every time a serialization factory was fetched from the cache rather than + only once when it was created. Whilst registering serializers that already exist is essentially a no-op, it's a performance overhead for + a very frequent operation that hits a synchronisation point (and is thus flagged as contended by our perfomance suite) + * Update the fast-classpath-scanner dependent library version from 2.0.21 to 2.12.3 .. note:: Whilst this is not the latest version of this library, that being 2.18.1 at time of writing, versions later @@ -14,6 +24,10 @@ from the previous milestone release. * Node can be shut down abruptly by ``shutdown`` function in `CordaRPCOps` or gracefully (draining flows first) through ``gracefulShutdown`` command from shell. +* Carpenter Exceptions will be caught internally by the Serializer and rethrown as a ``NotSerializableException`` + + * Specific details of the error encountered are logged to the node's log file. More information can be enabled by setting the debug level to ``trace`` ; this will cause the full stack trace of the error to be dumped into the log. + * Parsing of ``NodeConfiguration`` will now fail if unknown configuration keys are found. * The web server now has its own ``web-server.conf`` file, separate from ``node.conf``. diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPExceptions.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPExceptions.kt new file mode 100644 index 0000000000..0f93f0947e --- /dev/null +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPExceptions.kt @@ -0,0 +1,7 @@ +package net.corda.nodeapi.internal.serialization.amqp + +import java.io.NotSerializableException +import java.lang.reflect.Type + +class SyntheticParameterException(val type: Type) : NotSerializableException("Type '${type.typeName} has synthetic " + + "fields and is likely a nested inner class. This is not support by the Corda AMQP serialization framework") \ No newline at end of file diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPSerializationScheme.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPSerializationScheme.kt index caf8c94217..78c0597060 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPSerializationScheme.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPSerializationScheme.kt @@ -39,8 +39,15 @@ fun SerializerFactory.addToWhitelist(vararg types: Class<*>) { } } -abstract class AbstractAMQPSerializationScheme(val cordappLoader: List) : SerializationScheme { +open class SerializerFactoryFactory { + open fun make(context: SerializationContext) = + SerializerFactory(context.whitelist, context.deserializationClassLoader) +} +abstract class AbstractAMQPSerializationScheme( + val cordappLoader: List, + val sff : SerializerFactoryFactory = SerializerFactoryFactory() +) : SerializationScheme { // TODO: This method of initialisation for the Whitelist and plugin serializers will have to change // when we have per-cordapp contexts and dynamic app reloading but for now it's the easiest way companion object { @@ -126,9 +133,11 @@ abstract class AbstractAMQPSerializationScheme(val cordappLoader: List) rpcClientSerializerFactory(context) SerializationContext.UseCase.RPCServer -> rpcServerSerializerFactory(context) - else -> SerializerFactory(context.whitelist, context.deserializationClassLoader) + else -> sff.make(context) + }.also { + registerCustomSerializers(it) } - }.also { registerCustomSerializers(it) } + } } override fun deserialize(byteSequence: ByteSequence, clazz: Class, context: SerializationContext): T { diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt index e66f984816..78d4402516 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt @@ -163,9 +163,12 @@ class DeserializationInput @JvmOverloads constructor(private val serializerFacto is DescribedType -> { // Look up serializer in factory by descriptor val serializer = serializerFactory.get(obj.descriptor, schemas) - if (SerializerFactory.AnyType != type && serializer.type != type && with(serializer.type) { !isSubClassOf(type) && !materiallyEquivalentTo(type) }) + if (SerializerFactory.AnyType != type && serializer.type != type && with(serializer.type) { + !isSubClassOf(type) && !materiallyEquivalentTo(type) + }) { throw NotSerializableException("Described type with descriptor ${obj.descriptor} was " + "expected to be of type $type but was ${serializer.type}") + } serializer.readObject(obj.described, schemas, this) } is Binary -> obj.array diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt index 3c1d357b23..447ca44181 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt @@ -11,7 +11,6 @@ package net.corda.nodeapi.internal.serialization.amqp import net.corda.core.utilities.contextLogger -import net.corda.core.utilities.debug import net.corda.core.utilities.trace import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory.Companion.nameForType import org.apache.qpid.proton.amqp.Symbol @@ -68,13 +67,22 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS data: Data, type: Type, output: SerializationOutput, - debugIndent: Int) = ifThrowsAppend({ clazz.typeName }) { + debugIndent: Int) = ifThrowsAppend({ clazz.typeName } + ) { + if (propertySerializers.size != javaConstructor?.parameterCount && + javaConstructor?.parameterCount ?: 0 > 0 + ) { + throw NotSerializableException("Serialization constructor for class $type expects " + + "${javaConstructor?.parameterCount} parameters but we have ${propertySerializers.size} " + + "properties to serialize.") + } + // Write described data.withDescribed(typeNotation.descriptor) { // Write list withList { propertySerializers.serializationOrder.forEach { property -> - property.getter.writeProperty(obj, this, output, debugIndent+1) + property.getter.writeProperty(obj, this, output, debugIndent + 1) } } } @@ -102,23 +110,23 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS private fun readObjectBuildViaConstructor( obj: List<*>, schemas: SerializationSchemas, - input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ + input: DeserializationInput): Any = ifThrowsAppend({ clazz.typeName }) { logger.trace { "Calling construction based construction for ${clazz.typeName}" } - return construct (propertySerializers.serializationOrder + return construct(propertySerializers.serializationOrder .zip(obj) .map { Pair(it.first.initialPosition, it.first.getter.readProperty(it.second, schemas, input)) } - .sortedWith(compareBy({it.first})) + .sortedWith(compareBy({ it.first })) .map { it.second }) } private fun readObjectBuildViaSetters( obj: List<*>, schemas: SerializationSchemas, - input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ + input: DeserializationInput): Any = ifThrowsAppend({ clazz.typeName }) { logger.trace { "Calling setter based construction for ${clazz.typeName}" } - val instance : Any = javaConstructor?.newInstance() ?: throw NotSerializableException ( + val instance: Any = javaConstructor?.newInstance() ?: throw NotSerializableException( "Failed to instantiate instance of object $clazz") // read the properties out of the serialised form, since we're invoking the setters the order we @@ -146,7 +154,13 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS fun construct(properties: List): Any { logger.trace { "Calling constructor: '$javaConstructor' with properties '$properties'" } - return javaConstructor?.newInstance(*properties.toTypedArray()) ?: - throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.") + if (properties.size != javaConstructor?.parameterCount) { + throw NotSerializableException("Serialization constructor for class $type expects " + + "${javaConstructor?.parameterCount} parameters but we have ${properties.size} " + + "serialized properties.") + } + + return javaConstructor?.newInstance(*properties.toTypedArray()) + ?: throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.") } } \ No newline at end of file diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt index b0726793ac..625e6a3aa0 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt @@ -27,6 +27,7 @@ import kotlin.reflect.KParameter import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.primaryConstructor import kotlin.reflect.jvm.isAccessible +import kotlin.reflect.jvm.javaConstructor import kotlin.reflect.jvm.javaType /** @@ -43,6 +44,7 @@ internal fun constructorForDeserialization(type: Type): KFunction? { var annotatedCount = 0 val kotlinConstructors = clazz.kotlin.constructors val hasDefault = kotlinConstructors.any { it.parameters.isEmpty() } + for (kotlinConstructor in kotlinConstructors) { if (preferredCandidate == null && kotlinConstructors.size == 1) { preferredCandidate = kotlinConstructor @@ -168,7 +170,7 @@ fun Class.propertyDescriptors(): Map { PropertyDescriptorsRegex.re.find(func.name)?.apply { // matching means we have an func getX where the property could be x or X // so having pre-loaded all of the properties we try to match to either case. If that - // fails the getter doesn't refer to a property directly, but may to a cosntructor + // fails the getter doesn't refer to a property directly, but may refer to a constructor // parameter that shadows a property val properties = classProperties[groups[2]!!.value] ?: @@ -229,19 +231,26 @@ internal fun propertiesForSerializationFromConstructor( val classProperties = clazz.propertyDescriptors() + // Annoyingly there isn't a better way to ascertain that the constructor for the class + // has a synthetic parameter inserted to capture the reference to the outer class. You'd + // think you could inspect the parameter and check the isSynthetic flag but that is always + // false so given the naming convention is specified by the standard we can just check for + // this + if (kotlinConstructor.javaConstructor?.parameterCount ?: 0 > 0 && + kotlinConstructor.javaConstructor?.parameters?.get(0)?.name == "this$0" + ) { + throw SyntheticParameterException(type) + } + if (classProperties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) { return propertiesForSerializationFromSetters(classProperties, type, factory) } return mutableListOf().apply { kotlinConstructor.parameters.withIndex().forEach { param -> - // If a parameter doesn't have a name *at all* then chances are it's a synthesised - // one. A good example of this is non static nested classes in Java where instances - // of the nested class require access to the outer class without breaking - // encapsulation. Thus a parameter is inserted into the constructor that passes a - // reference to the enclosing class. In this case we can't do anything with - // it so just ignore it as it'll be supplied at runtime anyway on invocation - val name = param.value.name ?: return@forEach + // name cannot be null, if it is then this is a synthetic field and we will have bailed + // out prior to this + val name = param.value.name!! // We will already have disambiguated getA for property A or a but we still need to cope // with the case we don't know the case of A when the parameter doesn't match a property diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt index bf787b4af9..53ebafe871 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt @@ -12,11 +12,12 @@ package net.corda.nodeapi.internal.serialization.amqp import com.google.common.primitives.Primitives import com.google.common.reflect.TypeResolver +import net.corda.core.internal.getStackTraceAsString import net.corda.core.internal.uncheckedCast 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.contextLogger +import net.corda.core.utilities.loggerFor +import net.corda.nodeapi.internal.serialization.carpenter.* import org.apache.qpid.proton.amqp.* import java.io.NotSerializableException import java.lang.reflect.* @@ -208,7 +209,7 @@ open class SerializerFactory( * Register a custom serializer for any type that cannot be serialized or deserialized by the default serializer * that expects to find getters and a constructor with a parameter for each property. */ - fun register(customSerializer: CustomSerializer) { + open fun register(customSerializer: CustomSerializer) { if (!serializersByDescriptor.containsKey(customSerializer.typeDescriptor)) { customSerializers += customSerializer serializersByDescriptor[customSerializer.typeDescriptor] = customSerializer @@ -248,7 +249,19 @@ open class SerializerFactory( if (metaSchema.isNotEmpty()) { val mc = MetaCarpenter(metaSchema, classCarpenter) - mc.build() + try { + mc.build() + } catch (e: MetaCarpenterException) { + // preserve the actual message locally + loggerFor().apply { + error ("${e.message} [hint: enable trace debugging for the stack trace]") + trace (e.getStackTraceAsString()) + } + + // prevent carpenter exceptions escaping into the world, convert things into a nice + // NotSerializableException for when this escapes over the wire + throw NotSerializableException(e.name) + } processSchema(schemaAndDescriptor, true) } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt index db039342c1..f93ce58ff4 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt @@ -143,11 +143,12 @@ fun AMQPField.getTypeAsClass(classloader: ClassLoader) = typeStrToType[Pair(type else -> classloader.loadClass(type.stripGenerics()) } -fun AMQPField.validateType(classloader: ClassLoader) = when (type) { - "byte", "int", "string", "short", "long", "char", "boolean", "double", "float" -> true - "*" -> classloader.exists(requires[0]) - else -> classloader.exists(type) -} +fun AMQPField.validateType(classloader: ClassLoader) = + when (type) { + "byte", "int", "string", "short", "long", "char", "boolean", "double", "float" -> true + "*" -> classloader.exists(requires[0]) + else -> classloader.exists(type) + } private fun ClassLoader.exists(clazz: String) = run { try { diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt index 54ee7b2171..45efb11d5c 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt @@ -18,6 +18,7 @@ import org.objectweb.asm.Opcodes.* import org.objectweb.asm.Type import java.lang.Character.isJavaIdentifierPart import java.lang.Character.isJavaIdentifierStart +import java.lang.reflect.Method import java.util.* /** @@ -35,6 +36,12 @@ class CarpenterClassLoader(parentClassLoader: ClassLoader = Thread.currentThread fun load(name: String, bytes: ByteArray) = defineClass(name, bytes, 0, bytes.size) } +class InterfaceMismatchNonGetterException (val clazz: Class<*>, val method: Method) : InterfaceMismatchException( + "Requested interfaces must consist only of methods that start with 'get': ${clazz.name}.${method.name}") + +class InterfaceMismatchMissingAMQPFieldException (val clazz: Class<*>, val field: String) : InterfaceMismatchException( + "Interface ${clazz.name} requires a field named $field but that isn't found in the schema or any superclass schemas") + /** * Which version of the java runtime are we constructing objects against */ @@ -415,7 +422,7 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader * whitelisted classes */ private fun validateSchema(schema: Schema) { - if (schema.name in _loaded) throw DuplicateNameException() + if (schema.name in _loaded) throw DuplicateNameException(schema.name) fun isJavaName(n: String) = n.isNotBlank() && isJavaIdentifierStart(n.first()) && n.all(::isJavaIdentifierPart) require(isJavaName(schema.name.split(".").last())) { "Not a valid Java name: ${schema.name}" } schema.fields.forEach { @@ -430,20 +437,17 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader itf.methods.forEach { val fieldNameFromItf = when { it.name.startsWith("get") -> it.name.substring(3).decapitalize() - else -> throw InterfaceMismatchException( - "Requested interfaces must consist only of methods that start " - + "with 'get': ${itf.name}.${it.name}") + else -> throw InterfaceMismatchNonGetterException(itf, it) } - // If we're trying to carpent a class that prior to serialisation / deserialisation + // If we're trying to carpent a class that prior to serialisation / deserialization // was made by a carpenter then we can ignore this (it will implement a plain get // method from SimpleFieldAccess). if (fieldNameFromItf.isEmpty() && SimpleFieldAccess::class.java in schema.interfaces) return@forEach - if ((schema is ClassSchema) and (fieldNameFromItf !in allFields)) - throw InterfaceMismatchException( - "Interface ${itf.name} requires a field named $fieldNameFromItf but that " - + "isn't found in the schema or any superclass schemas") + if ((schema is ClassSchema) and (fieldNameFromItf !in allFields)) { + throw InterfaceMismatchMissingAMQPFieldException(itf, fieldNameFromItf) + } } } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/Exceptions.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/Exceptions.kt index 0ef4e413a2..460f73197f 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/Exceptions.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/Exceptions.kt @@ -10,15 +10,35 @@ package net.corda.nodeapi.internal.serialization.carpenter -import net.corda.core.CordaException import net.corda.core.CordaRuntimeException +import org.objectweb.asm.Type -class DuplicateNameException : CordaRuntimeException( - "An attempt was made to register two classes with the same name within the same ClassCarpenter namespace.") +/** + * The general exception type thrown by the [ClassCarpenter] + */ +abstract class ClassCarpenterException(msg: String) : CordaRuntimeException(msg) -class InterfaceMismatchException(msg: String) : CordaRuntimeException(msg) +/** + * Thrown by the [ClassCarpenter] when trying to build + */ +abstract class InterfaceMismatchException(msg: String) : ClassCarpenterException(msg) -class NullablePrimitiveException(msg: String) : CordaRuntimeException(msg) +class DuplicateNameException(val name : String) : ClassCarpenterException ( + "An attempt was made to register two classes with the name '$name' within the same ClassCarpenter namespace.") + +class NullablePrimitiveException(val name: String, val field: Class) : ClassCarpenterException( + "Field $name is primitive type ${Type.getDescriptor(field)} and thus cannot be nullable") class UncarpentableException(name: String, field: String, type: String) : - CordaException("Class $name is loadable yet contains field $field of unknown type $type") + ClassCarpenterException("Class $name is loadable yet contains field $field of unknown type $type") + +/** + * A meta exception used by the [MetaCarpenter] to wrap any exceptions generated during the build + * process and associate those with the current schema being processed. This makes for cleaner external + * error hand + * + * @property name The name of the schema, and thus the class being created, when the error was occured + * @property e The [ClassCarpenterException] this is wrapping + */ +class MetaCarpenterException(val name: String, val e: ClassCarpenterException) : CordaRuntimeException( + "Whilst processing class '$name' - ${e.message}") diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt index 7a5a41a8f1..3f39f4b2cd 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt @@ -15,13 +15,13 @@ import net.corda.nodeapi.internal.serialization.amqp.RestrictedType import net.corda.nodeapi.internal.serialization.amqp.TypeNotation /** - * Generated from an AMQP schema this class represents the classes unknown to the deserialiser and that thusly - * require carpenting up in bytecode form. This is a multi step process as carpenting one object may be depedent + * Generated from an AMQP schema this class represents the classes unknown to the deserializer and that thusly + * require carpenting up in bytecode form. This is a multi step process as carpenting one object may be dependent * upon the creation of others, this information is tracked in the dependency tree represented by * [dependencies] and [dependsOn]. Creatable classes are stored in [carpenterSchemas]. * * The state of this class after initial generation is expected to mutate as classes are built by the carpenter - * enablaing the resolution of dependencies and thus new carpenter schemas added whilst those already + * enabling the resolution of dependencies and thus new carpenter schemas added whilst those already * carpented schemas are removed. * * @property carpenterSchemas The list of carpentable classes @@ -65,7 +65,7 @@ data class CarpenterMetaSchema( /** * Take a dependency tree of [CarpenterMetaSchema] and reduce it to zero by carpenting those classes that - * require it. As classes are carpented check for depdency resolution, if now free generate a [Schema] for + * require it. As classes are carpented check for dependency resolution, if now free generate a [Schema] for * that class and add it to the list of classes ([CarpenterMetaSchema.carpenterSchemas]) that require * carpenting * @@ -105,7 +105,11 @@ class MetaCarpenter(schemas: CarpenterMetaSchema, cc: ClassCarpenter) : MetaCarp override fun build() { while (schemas.carpenterSchemas.isNotEmpty()) { val newObject = schemas.carpenterSchemas.removeAt(0) - step(newObject) + try { + step(newObject) + } catch (e: ClassCarpenterException) { + throw MetaCarpenterException(newObject.name, e) + } } } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt index 586ac15d22..b78bcceb01 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt @@ -108,8 +108,7 @@ class NullableField(field: Class) : ClassField(field) { init { if (field.isPrimitive) { - throw NullablePrimitiveException( - "Field $name is primitive type ${Type.getDescriptor(field)} and thus cannot be nullable") + throw NullablePrimitiveException(name, field) } } diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/ForbiddenLambdaSerializationTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/ForbiddenLambdaSerializationTests.java index 0dc35a2e24..8aff41edbf 100644 --- a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/ForbiddenLambdaSerializationTests.java +++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/ForbiddenLambdaSerializationTests.java @@ -14,9 +14,9 @@ import com.google.common.collect.Maps; import net.corda.core.serialization.SerializationContext; import net.corda.core.serialization.SerializationFactory; import net.corda.core.serialization.SerializedBytes; -import net.corda.testing.core.SerializationEnvironmentRule; import net.corda.nodeapi.internal.serialization.kryo.CordaClosureBlacklistSerializer; import net.corda.nodeapi.internal.serialization.kryo.KryoSerializationSchemeKt; +import net.corda.testing.core.SerializationEnvironmentRule; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -29,6 +29,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.ThrowableAssert.catchThrowable; public final class ForbiddenLambdaSerializationTests { + private EnumSet contexts = EnumSet.complementOf( + EnumSet.of(SerializationContext.UseCase.Checkpoint, SerializationContext.UseCase.Testing)); @Rule public final SerializationEnvironmentRule testSerialization = new SerializationEnvironmentRule(); private SerializationFactory factory; @@ -39,11 +41,10 @@ public final class ForbiddenLambdaSerializationTests { } @Test - public final void serialization_fails_for_serializable_java_lambdas() throws Exception { - EnumSet contexts = EnumSet.complementOf(EnumSet.of(SerializationContext.UseCase.Checkpoint)); - + public final void serialization_fails_for_serializable_java_lambdas() { contexts.forEach(ctx -> { - SerializationContext context = new SerializationContextImpl(KryoSerializationSchemeKt.getKryoMagic(), this.getClass().getClassLoader(), AllWhitelist.INSTANCE, Maps.newHashMap(), true, ctx, null); + SerializationContext context = new SerializationContextImpl(KryoSerializationSchemeKt.getKryoMagic(), + this.getClass().getClassLoader(), AllWhitelist.INSTANCE, Maps.newHashMap(), true, ctx, null); String value = "Hey"; Callable target = (Callable & Serializable) () -> value; @@ -61,11 +62,10 @@ public final class ForbiddenLambdaSerializationTests { @Test @SuppressWarnings("unchecked") - public final void serialization_fails_for_not_serializable_java_lambdas() throws Exception { - EnumSet contexts = EnumSet.complementOf(EnumSet.of(SerializationContext.UseCase.Checkpoint)); - + public final void serialization_fails_for_not_serializable_java_lambdas() { contexts.forEach(ctx -> { - SerializationContext context = new SerializationContextImpl(KryoSerializationSchemeKt.getKryoMagic(), this.getClass().getClassLoader(), AllWhitelist.INSTANCE, Maps.newHashMap(), true, ctx, null); + SerializationContext context = new SerializationContextImpl(KryoSerializationSchemeKt.getKryoMagic(), + this.getClass().getClassLoader(), AllWhitelist.INSTANCE, Maps.newHashMap(), true, ctx, null); String value = "Hey"; Callable target = () -> value; diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedClassesTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedClassesTests.java new file mode 100644 index 0000000000..66cd995220 --- /dev/null +++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedClassesTests.java @@ -0,0 +1,224 @@ +package net.corda.nodeapi.internal.serialization.amqp; + +import com.google.common.collect.ImmutableList; +import kotlin.Suppress; +import net.corda.core.contracts.ContractState; +import net.corda.core.identity.AbstractParty; +import net.corda.core.serialization.SerializedBytes; +import net.corda.nodeapi.internal.serialization.AllWhitelist; +import org.assertj.core.api.Assertions; +import org.jetbrains.annotations.NotNull; +import org.junit.Test; + +import java.io.NotSerializableException; +import java.util.List; + +class OuterClass1 { + protected SerializationOutput ser; + DeserializationInput desExisting; + DeserializationInput desRegen; + + OuterClass1() { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializerFactory factory2 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + this.ser = new SerializationOutput(factory1); + this.desExisting = new DeserializationInput(factory1); + this.desRegen = new DeserializationInput(factory2); + } + + class DummyState implements ContractState { + @Override @NotNull public List getParticipants() { + return ImmutableList.of(); + } + } + + public void run() throws NotSerializableException { + SerializedBytes b = ser.serialize(new DummyState()); + desExisting.deserialize(b, DummyState.class); + desRegen.deserialize(b, DummyState.class); + } +} + +class Inherator1 extends OuterClass1 { + public void iRun() throws NotSerializableException { + SerializedBytes b = ser.serialize(new DummyState()); + desExisting.deserialize(b, DummyState.class); + desRegen.deserialize(b, DummyState.class); + } +} + +class OuterClass2 { + protected SerializationOutput ser; + DeserializationInput desExisting; + DeserializationInput desRegen; + + OuterClass2() { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializerFactory factory2 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + this.ser = new SerializationOutput(factory1); + this.desExisting = new DeserializationInput(factory1); + this.desRegen = new DeserializationInput(factory2); + } + + protected class DummyState implements ContractState { + private Integer count; + + DummyState(Integer count) { + this.count = count; + } + + @Override @NotNull public List getParticipants() { + return ImmutableList.of(); + } + } + + public void run() throws NotSerializableException { + SerializedBytes b = ser.serialize(new DummyState(12)); + desExisting.deserialize(b, DummyState.class); + desRegen.deserialize(b, DummyState.class); + } +} + +class Inherator2 extends OuterClass2 { + public void iRun() throws NotSerializableException { + SerializedBytes b = ser.serialize(new DummyState(12)); + desExisting.deserialize(b, DummyState.class); + desRegen.deserialize(b, DummyState.class); + } +} + +// Make the base class abstract +abstract class AbstractClass2 { + protected SerializationOutput ser; + + AbstractClass2() { + SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + this.ser = new SerializationOutput(factory); + } + + protected class DummyState implements ContractState { + @Override @NotNull public List getParticipants() { + return ImmutableList.of(); + } + } +} + +class Inherator4 extends AbstractClass2 { + public void run() throws NotSerializableException { + ser.serialize(new DummyState()); + } +} + +abstract class AbstractClass3 { + protected class DummyState implements ContractState { + @Override @NotNull public List getParticipants() { + return ImmutableList.of(); + } + } +} + +class Inherator5 extends AbstractClass3 { + public void run() throws NotSerializableException { + SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializationOutput ser = new SerializationOutput(factory); + ser.serialize(new DummyState()); + } +} + +class Inherator6 extends AbstractClass3 { + public class Wrapper { + //@Suppress("UnusedDeclaration"]) + private ContractState cState; + + Wrapper(ContractState cState) { + this.cState = cState; + } + } + + public void run() throws NotSerializableException { + SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializationOutput ser = new SerializationOutput(factory); + ser.serialize(new Wrapper(new DummyState())); + } +} + +public class JavaNestedClassesTests { + @Test + public void publicNested() { + Assertions.assertThatThrownBy(() -> new OuterClass1().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void privateNested() { + Assertions.assertThatThrownBy(() -> new OuterClass2().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void publicNestedInherited() { + Assertions.assertThatThrownBy(() -> new Inherator1().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + + Assertions.assertThatThrownBy(() -> new Inherator1().iRun()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void protectedNestedInherited() { + Assertions.assertThatThrownBy(() -> new Inherator2().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + + Assertions.assertThatThrownBy(() -> new Inherator2().iRun()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void abstractNested() { + Assertions.assertThatThrownBy(() -> new Inherator4().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void abstractNestedFactoryOnNested() { + Assertions.assertThatThrownBy(() -> new Inherator5().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void abstractNestedFactoryOnNestedInWrapper() { + Assertions.assertThatThrownBy(() -> new Inherator6().run()).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } +} + diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedInheritenceTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedInheritenceTests.java new file mode 100644 index 0000000000..548f3625a3 --- /dev/null +++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaNestedInheritenceTests.java @@ -0,0 +1,70 @@ +package net.corda.nodeapi.internal.serialization.amqp; + +import com.google.common.collect.ImmutableList; +import net.corda.core.contracts.ContractState; +import net.corda.core.identity.AbstractParty; +import net.corda.nodeapi.internal.serialization.AllWhitelist; +import org.assertj.core.api.Assertions; +import org.jetbrains.annotations.NotNull; +import org.junit.Test; + +import java.io.NotSerializableException; +import java.util.List; + +abstract class JavaNestedInheritenceTestsBase { + class DummyState implements ContractState { + @Override @NotNull public List getParticipants() { + return ImmutableList.of(); + } + } +} + +class Wrapper { + private ContractState cs; + Wrapper(ContractState cs) { this.cs = cs; } +} + +class TemplateWrapper { + public T obj; + TemplateWrapper(T obj) { this.obj = obj; } +} + +public class JavaNestedInheritenceTests extends JavaNestedInheritenceTestsBase { + @Test + public void serializeIt() { + SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializationOutput ser = new SerializationOutput(factory); + + Assertions.assertThatThrownBy(() -> ser.serialize(new DummyState())).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void serializeIt2() { + SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializationOutput ser = new SerializationOutput(factory); + Assertions.assertThatThrownBy(() -> ser.serialize(new Wrapper (new DummyState()))).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } + + @Test + public void serializeIt3() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(), + new EvolutionSerializerGetter(), + new SerializerFingerPrinter()); + + SerializationOutput ser = new SerializationOutput(factory1); + + Assertions.assertThatThrownBy(() -> ser.serialize(new TemplateWrapper (new DummyState()))).isInstanceOf( + NotSerializableException.class).hasMessageContaining( + "has synthetic fields and is likely a nested inner class"); + } +} diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaSerializationOutputTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaSerializationOutputTests.java index 69e2931446..99176eefdb 100644 --- a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaSerializationOutputTests.java +++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaSerializationOutputTests.java @@ -18,6 +18,7 @@ import net.corda.nodeapi.internal.serialization.AllWhitelist; import net.corda.core.serialization.SerializedBytes; import org.apache.qpid.proton.codec.DecoderImpl; import org.apache.qpid.proton.codec.EncoderImpl; +import org.jetbrains.annotations.NotNull; import org.junit.Test; import javax.annotation.Nonnull; @@ -121,10 +122,12 @@ public class JavaSerializationOutputTests { this.count = count; } + @SuppressWarnings("unused") public String getFred() { return fred; } + @SuppressWarnings("unused") public Integer getCount() { return count; } @@ -252,24 +255,4 @@ public class JavaSerializationOutputTests { BoxedFooNotNull obj = new BoxedFooNotNull("Hello World!", 123); serdes(obj); } - - protected class DummyState implements ContractState { - @Override - public List getParticipants() { - return ImmutableList.of(); - } - } - - @Test - public void dummyStateSerialize() throws NotSerializableException { - SerializerFactory factory1 = new SerializerFactory( - AllWhitelist.INSTANCE, - ClassLoader.getSystemClassLoader(), - new EvolutionSerializerGetter(), - new SerializerFingerPrinter()); - - SerializationOutput serializer = new SerializationOutput(factory1); - - serializer.serialize(new DummyState()); - } } diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumEvolveTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumEvolveTests.kt index db65c987be..f4ecc1b2a2 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumEvolveTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumEvolveTests.kt @@ -10,6 +10,7 @@ package net.corda.nodeapi.internal.serialization.amqp +import net.corda.core.internal.packageName import net.corda.core.serialization.CordaSerializationTransformEnumDefault import net.corda.core.serialization.CordaSerializationTransformEnumDefaults import net.corda.core.serialization.SerializedBytes @@ -19,6 +20,8 @@ import org.junit.Test import java.io.File import java.io.NotSerializableException import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue // NOTE: To recreate the test files used by these tests uncomment the original test classes and comment // the new ones out, then change each test to write out the serialized bytes rather than read @@ -356,6 +359,9 @@ class EnumEvolveTests { Pair("$resource.5.G", MultiOperations.C)) fun load(l: List>) = l.map { + assertNotNull (EvolvabilityTests::class.java.getResource(it.first)) + assertTrue (File(EvolvabilityTests::class.java.getResource(it.first).toURI()).exists()) + Pair(DeserializationInput(sf).deserialize(SerializedBytes( File(EvolvabilityTests::class.java.getResource(it.first).toURI()).readBytes())), it.second) } diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt index 99d9434b4a..e024898eb7 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt @@ -1241,7 +1241,86 @@ class SerializationOutputTests(private val compression: CordaSerializationEncodi val testExcp = TestException("hello", Throwable().apply { stackTrace = Thread.currentThread().stackTrace }) val factory = testDefaultFactoryNoEvolution() SerializationOutput(factory).serialize(testExcp) - } + + @Test + fun nestedInner() { + class C(val a: Int) { + inner class D(val b: Int) + + fun serialize() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(D(4)) + } + } + + // By the time we escape the serializer we should just have a general + // NotSerializable Exception + assertThatExceptionOfType(NotSerializableException::class.java).isThrownBy { + C(12).serialize() + }.withMessageContaining("has synthetic fields and is likely a nested inner class") + } + + @Test + fun nestedNestedInner() { + class C(val a: Int) { + inner class D(val b: Int) { + inner class E(val c: Int) + + fun serialize() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(E(4)) + } + } + + fun serializeD() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(D(4)) + } + + fun serializeE() { + D(1).serialize() + } + } + + // By the time we escape the serializer we should just have a general + // NotSerializable Exception + assertThatExceptionOfType(NotSerializableException::class.java).isThrownBy { + C(12).serializeD() + }.withMessageContaining("has synthetic fields and is likely a nested inner class") + + assertThatExceptionOfType(NotSerializableException::class.java).isThrownBy { + C(12).serializeE() + }.withMessageContaining("has synthetic fields and is likely a nested inner class") + } + + @Test + fun multiNestedInner() { + class C(val a: Int) { + inner class D(val b: Int) + inner class E(val c: Int) + + fun serializeD() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(D(4)) + } + + fun serializeE() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(E(4)) + } + } + + // By the time we escape the serializer we should just have a general + // NotSerializable Exception + assertThatExceptionOfType(NotSerializableException::class.java).isThrownBy { + C(12).serializeD() + }.withMessageContaining("has synthetic fields and is likely a nested inner class") + + assertThatExceptionOfType(NotSerializableException::class.java).isThrownBy { + C(12).serializeE() + }.withMessageContaining("has synthetic fields and is likely a nested inner class") + } + } diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationSchemaTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationSchemaTests.kt new file mode 100644 index 0000000000..066278b2c7 --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationSchemaTests.kt @@ -0,0 +1,98 @@ +package net.corda.nodeapi.internal.serialization.amqp + +import net.corda.core.serialization.* +import net.corda.core.utilities.ByteSequence +import net.corda.nodeapi.internal.serialization.* +import org.junit.Test +import kotlin.test.assertEquals + +// Make sure all serialization calls in this test don't get stomped on by anything else +val TESTING_CONTEXT = SerializationContextImpl(amqpMagic, + SerializationDefaults.javaClass.classLoader, + GlobalTransientClassWhiteList(BuiltInExceptionsWhitelist()), + emptyMap(), + true, + SerializationContext.UseCase.Testing, + null) + +// Test factory that lets us count the number of serializer registration attempts +class TestSerializerFactory( + wl : ClassWhitelist, + cl : ClassLoader +) : SerializerFactory(wl, cl) { + var registerCount = 0 + + override fun register(customSerializer: CustomSerializer) { + ++registerCount + return super.register(customSerializer) + } +} + +// Instance of our test factory counting registration attempts. Sucks its global, but for testing purposes this +// is the easiest way of getting access to the object. +val testFactory = TestSerializerFactory(TESTING_CONTEXT.whitelist, TESTING_CONTEXT.deserializationClassLoader) + +// Serializer factory factory, plugs into the SerializationScheme and controls which factory type +// we make for each use case. For our tests we need to make sure if its the Testing use case we return +// the global factory object created above that counts registrations. +class TestSerializerFactoryFactory : SerializerFactoryFactory() { + override fun make(context: SerializationContext) = + when (context.useCase) { + SerializationContext.UseCase.Testing -> testFactory + else -> super.make(context) + } +} + +class AMQPTestSerializationScheme : AbstractAMQPSerializationScheme(emptyList(), TestSerializerFactoryFactory()) { + override fun rpcClientSerializerFactory(context: SerializationContext): SerializerFactory { + throw UnsupportedOperationException() + } + + override fun rpcServerSerializerFactory(context: SerializationContext): SerializerFactory { + throw UnsupportedOperationException() + } + + override fun canDeserializeVersion(magic: CordaSerializationMagic, target: SerializationContext.UseCase) = true +} + +// Test SerializationFactory that wraps a serialization scheme that just allows us to call .serialize. +// Returns the testing scheme we created above that wraps the testing factory. +class TestSerializationFactory : SerializationFactory() { + private val scheme = AMQPTestSerializationScheme() + + override fun deserialize( + byteSequence: ByteSequence, + clazz: Class, context: + SerializationContext + ): T { + throw UnsupportedOperationException() + } + + override fun deserializeWithCompatibleContext( + byteSequence: ByteSequence, + clazz: Class, + context: SerializationContext + ): ObjectWithCompatibleContext { + throw UnsupportedOperationException() + } + + override fun serialize(obj: T, context: SerializationContext) = scheme.serialize(obj, context) +} + +// The actual test +class SerializationSchemaTests { + @Test + fun onlyRegisterCustomSerializersOnce() { + @CordaSerializable data class C(val a: Int) + + val c = C(1) + val testSerializationFactory = TestSerializationFactory() + val expectedCustomSerializerCount = 40 + + assertEquals (0, testFactory.registerCount) + c.serialize (testSerializationFactory, TESTING_CONTEXT) + assertEquals (expectedCustomSerializerCount, testFactory.registerCount) + c.serialize (testSerializationFactory, TESTING_CONTEXT) + assertEquals (expectedCustomSerializerCount, testFactory.registerCount) + } +} \ No newline at end of file diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/CarpenterExceptionTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/CarpenterExceptionTests.kt new file mode 100644 index 0000000000..c3ce2469a5 --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/CarpenterExceptionTests.kt @@ -0,0 +1,96 @@ +package net.corda.nodeapi.internal.serialization.carpenter + +import com.google.common.reflect.TypeToken +import junit.framework.Assert.assertTrue +import net.corda.nodeapi.internal.serialization.AllWhitelist +import net.corda.nodeapi.internal.serialization.amqp.* +import org.assertj.core.api.Assertions +import org.junit.Test +import java.io.NotSerializableException +import java.lang.reflect.Type +import java.net.URL +import kotlin.reflect.jvm.jvmName +import kotlin.test.assertEquals + +// Simple way to ensure we end up trying to carpent a class, "remove" it from the class loader (if only +// actually doing that was simple) +class TestClassLoader (private var exclude: List) : ClassLoader() { + override fun loadClass(p0: String?, p1: Boolean): Class<*> { + if (p0 in exclude) { + throw ClassNotFoundException("Pretending we can't find class $p0") + } + + return super.loadClass(p0, p1) + } +} + +interface TestInterface { + fun runThing() : Int +} + +// Create a custom serialization factory where we need to be able to both specify a carpenter +// but also have the class loader used by the carpenter be substantially different from the +// one used by the factory so as to ensure we can control their behaviour independently. +class TestFactory(override val classCarpenter: ClassCarpenter, cl: ClassLoader) + : SerializerFactory (classCarpenter.whitelist, cl) + +class CarpenterExceptionTests { + companion object { + val VERBOSE: Boolean get() = false + } + + @Test + fun checkClassComparison() { + class clA : ClassLoader() { + override fun loadClass(name: String?, resolve: Boolean): Class<*> { + return super.loadClass(name, resolve) + } + } + + class clB : ClassLoader() { + override fun loadClass(name: String?, resolve: Boolean): Class<*> { + return super.loadClass(name, resolve) + } + } + + data class A(val a: Int, val b: Int) + + val a3 = ClassLoader.getSystemClassLoader().loadClass(A::class.java.name) + val a1 = clA().loadClass(A::class.java.name) + val a2 = clB().loadClass(A::class.java.name) + + assertTrue (TypeToken.of(a1).isSubtypeOf(a2)) + assertTrue (a1 is Type) + assertTrue (a2 is Type) + assertTrue (a3 is Type) + assertEquals(a1, a2) + assertEquals(a1, a3) + assertEquals(a2, a3) + } + + @Test + fun carpenterExceptionRethrownAsNotSerializableException() { + data class C2 (val i: Int) : TestInterface { + override fun runThing() = 1 + } + + data class C1 (val i: Int, val c: C2) + + // We need two factories to ensure when we deserialize the blob we don't just use the serializer + // we built to serialise things + val ser = TestSerializationOutput(VERBOSE, testDefaultFactory()).serialize(C1(1, C2(2))) + + // Our second factory is "special" + // The class loader given to the factory rejects the outer class, this will trigger an attempt to + // carpent that class up. However, when looking at the fields specified as properties of that class + // we set the class loader of the ClassCarpenter to reject one of them, resulting in a CarpentryError + // which we then want the code to wrap in a NotSerializeableException + val cc = ClassCarpenter(TestClassLoader(listOf(C2::class.jvmName)), AllWhitelist) + val factory = TestFactory(cc, TestClassLoader(listOf(C1::class.jvmName))) + + Assertions.assertThatThrownBy { + DeserializationInput(factory).deserialize(ser) + }.isInstanceOf(NotSerializableException::class.java) + .hasMessageContaining(C2::class.java.name) + } +} \ No newline at end of file diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/kryo/KryoTests.kt similarity index 99% rename from node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt rename to node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/kryo/KryoTests.kt index 8d4b58b7fd..2fe6f4f341 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/kryo/KryoTests.kt @@ -8,7 +8,7 @@ * Distribution of this file or any portion thereof via any medium without the express permission of R3 is strictly prohibited. */ -package net.corda.nodeapi.internal.serialization +package net.corda.nodeapi.internal.serialization.kryo import com.esotericsoftware.kryo.Kryo import com.esotericsoftware.kryo.KryoException @@ -26,7 +26,7 @@ import net.corda.core.utilities.ProgressTracker import net.corda.core.utilities.sequence import net.corda.node.serialization.KryoServerSerializationScheme import net.corda.node.services.persistence.NodeAttachmentService -import net.corda.nodeapi.internal.serialization.kryo.kryoMagic +import net.corda.nodeapi.internal.serialization.* import net.corda.testing.core.ALICE_NAME import net.corda.testing.core.TestIdentity import net.corda.testing.internal.rigorousMock @@ -343,4 +343,4 @@ class KryoTests(private val compression: CordaSerializationEncoding?) { assertEquals(encodingNotPermittedFormat.format(compression), message) } } -} \ No newline at end of file +}