Merge pull request #622 from corda/kat-merge-27-03-18

Kat merge 27-03-18

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 }
 }
 
 /**

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 <https://docs.oracle.com/javase/tutorial/java/javaOO/nested.html>`_
+  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``.

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")

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<Cordapp>) : SerializationScheme {
+open class SerializerFactoryFactory {
+    open fun make(context: SerializationContext) =
+        SerializerFactory(context.whitelist, context.deserializationClassLoader)
+}
 
+abstract class AbstractAMQPSerializationScheme(
+        val cordappLoader: List<Cordapp>,
+        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<Cordapp>)
                     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 <T : Any> deserialize(byteSequence: ByteSequence, clazz: Class<T>, context: SerializationContext): T {

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

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?>): Any {
         logger.trace { "Calling constructor: '$javaConstructor' with properties '$properties'" }
 
-        return javaConstructor?.newInstance(*properties.toTypedArray()) ?:
+        if (properties.size != javaConstructor?.parameterCount) {
-                throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.")
+            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.")
     }
 }

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<Any>? {
         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<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
             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 <T : Any> 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<PropertyAccessor>().apply {
         kotlinConstructor.parameters.withIndex().forEach { param ->
-            // If a parameter doesn't have a name *at all* then chances are it's a synthesised
+            // name cannot be null, if it is then this is a synthetic field and we will have bailed
-            // one. A good example of this is non static nested classes in Java where instances
+            // out prior to this
-            // of the nested class require access to the outer class without breaking
+            val name = param.value.name!!
-            // 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
 
             // 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

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.core.utilities.contextLogger
-import net.corda.nodeapi.internal.serialization.carpenter.ClassCarpenter
+import net.corda.core.utilities.loggerFor
-import net.corda.nodeapi.internal.serialization.carpenter.MetaCarpenter
+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<out Any>) {
+    open fun register(customSerializer: CustomSerializer<out Any>) {
         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<SerializerFactory>().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)
         }
     }

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) {
+fun AMQPField.validateType(classloader: ClassLoader) =
-    "byte", "int", "string", "short", "long", "char", "boolean", "double", "float" -> true
+        when (type) {
-    "*" -> classloader.exists(requires[0])
+            "byte", "int", "string", "short", "long", "char", "boolean", "double", "float" -> true
-    else -> classloader.exists(type)
+            "*" -> classloader.exists(requires[0])
-}
+            else -> classloader.exists(type)
+        }
 
 private fun ClassLoader.exists(clazz: String) = run {
     try {

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(
+                    else -> throw InterfaceMismatchNonGetterException(itf, it)
-                            "Requested interfaces must consist only of methods that start "
-                                    + "with 'get': ${itf.name}.${it.name}")
                 }
 
-                // 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))
+                if ((schema is ClassSchema) and (fieldNameFromItf !in allFields)) {
-                    throw InterfaceMismatchException(
+                    throw InterfaceMismatchMissingAMQPFieldException(itf, fieldNameFromItf)
-                            "Interface ${itf.name} requires a field named $fieldNameFromItf but that "
+                }
-                                    + "isn't found in the schema or any superclass schemas")
             }
         }
     }

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<out Any>) : 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}")

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
+ * 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 depedent
+ * 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)
+            }
         }
     }
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt

@@ -108,8 +108,7 @@ class NullableField(field: Class<out Any?>) : ClassField(field) {
 
     init {
         if (field.isPrimitive) {
-            throw NullablePrimitiveException(
+            throw NullablePrimitiveException(name, field)
-                    "Field $name is primitive type ${Type.getDescriptor(field)} and thus cannot be nullable")
         }
     }
 

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<SerializationContext.UseCase> 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 {
+    public final void serialization_fails_for_serializable_java_lambdas() {
-        EnumSet<SerializationContext.UseCase> contexts = EnumSet.complementOf(EnumSet.of(SerializationContext.UseCase.Checkpoint));
-
         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<String> target = (Callable<String> & 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 {
+    public final void serialization_fails_for_not_serializable_java_lambdas() {
-        EnumSet<SerializationContext.UseCase> contexts = EnumSet.complementOf(EnumSet.of(SerializationContext.UseCase.Checkpoint));
-
         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<String> target = () -> value;
 

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<AbstractParty> 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<AbstractParty> 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<AbstractParty> 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<AbstractParty> 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");
+    }
+}
+

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<AbstractParty> getParticipants() {
+            return ImmutableList.of();
+        }
+    }
+}
+
+class Wrapper {
+    private ContractState cs;
+    Wrapper(ContractState cs) { this.cs = cs; }
+}
+
+class TemplateWrapper<T> {
+    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<ContractState> (new DummyState()))).isInstanceOf(
+                NotSerializableException.class).hasMessageContaining(
+                "has synthetic fields and is likely a nested inner class");
+    }
+}

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<AbstractParty> 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());
-    }
 }

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<Pair<String, MultiOperations>>) = l.map {
+            assertNotNull (EvolvabilityTests::class.java.getResource(it.first))
+            assertTrue (File(EvolvabilityTests::class.java.getResource(it.first).toURI()).exists())
+
             Pair(DeserializationInput(sf).deserialize(SerializedBytes<C>(
                     File(EvolvabilityTests::class.java.getResource(it.first).toURI()).readBytes())), it.second)
         }

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")
+    }
+
 }
 

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<out Any>) {
+        ++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 <OBJ>.serialize.
+// Returns the testing scheme we created above that wraps the testing factory.
+class TestSerializationFactory : SerializationFactory() {
+    private val scheme = AMQPTestSerializationScheme()
+
+    override fun <T : Any> deserialize(
+            byteSequence: ByteSequence,
+            clazz: Class<T>, context:
+            SerializationContext
+    ): T {
+        throw UnsupportedOperationException()
+    }
+
+    override fun <T : Any> deserializeWithCompatibleContext(
+            byteSequence: ByteSequence,
+            clazz: Class<T>,
+            context: SerializationContext
+    ): ObjectWithCompatibleContext<T> {
+        throw UnsupportedOperationException()
+    }
+
+    override fun <T : Any> 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)
+    }
+}

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<String>) : 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)
+    }
+}

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)
         }
     }
-}
+}