From 6b1429d30d1d502dd2d3e2f44576f47a8652cefd Mon Sep 17 00:00:00 2001 From: JamesHR3 <45565019+JamesHR3@users.noreply.github.com> Date: Fri, 1 Mar 2019 10:57:37 +0000 Subject: [PATCH] CORDA-2687 - Ensure collections of unknown types are not carpented (#4826) * Prevent carpenting a collection of an unknown type * Add a comment explaining the condition used to detect collections * Add a unit test and clarify comment around what is being carpented --- .../serialization/amqp/SerializerFactory.kt | 10 +++-- ...ticInitialisationOfSerializedObjectTest.kt | 41 ++++++++++++++++++ ...onOfSerializedObjectTest.unknownCollection | Bin 0 -> 1007 bytes 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.unknownCollection 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 efc98b2bab..43e94e9aee 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 @@ -248,9 +248,13 @@ open class SerializerFactory( logger.debug { "descriptor=${schemaAndDescriptor.typeDescriptor}, typeNotation=$name" } name to processSchemaEntry(notation) } catch (e: ClassNotFoundException) { - // class missing from the classpath, so load its interfaces and add it for carpenting (see method docs). - interfacesPerClass[name]!!.forEach { processSchemaEntry(it) } - metaSchema.buildFor(notation, classloader) + // Class missing from the classpath, so load its interfaces and add it for carpenting (see method docs). + // This carpenting should only be carried out for non-collections. These are detected by looking for + // types that are not composites (RestrictedTypes), and not enums (have no choices). + if (!(notation is RestrictedType && notation.choices.isEmpty())) { + interfacesPerClass[name]!!.forEach { processSchemaEntry(it) } + metaSchema.buildFor(notation, classloader) + } null } }.toMap() diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.kt index 64b83a7525..795f5c2f9d 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.kt @@ -90,6 +90,47 @@ class StaticInitialisationOfSerializedObjectTest { .doesNotContain(interfaceType.name) } +// enum class MyEnum { +// Foo, +// Bar, +// Baz +// } + + // This test checks that if a known class name is received with an additional property of type + // Collection, UnknownType is carpented correctly and a usable known type is produced. + // + // To regenerate the serialized data for this test: + // - Uncomment the enum above + // - Uncomment the definition of DummyClass with the additional set and comment out the current definition + // - Uncomment the generation code and comment out all code underneath it in the test + // - Run the test. This will print a path to a new file; copy this to the test resources directory + @Test + fun collectionsOfUnknownTypesAreHandledCorrectly() { +// data class DummyClass(val a: String, val b: List) + data class DummyClass(val a: String) + + val path = EvolvabilityTests::class.java.getResource("StaticInitialisationOfSerializedObjectTest.unknownCollection") + val f = File(path.toURI()) + +// val sf = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader()) +// val sc = SerializationOutput(sf).serialize(DummyClass("foo", listOf(MyEnum.Foo))) +// f.writeBytes(sc.bytes) +// println(path) + + class WL : ClassWhitelist { + override fun hasListed(type: Class<*>): Boolean = + type.name == "net.corda.nodeapi.internal.serialization.amqp" + + ".StaticInitialisationOfSerializedObjectTest\$collectionsOfUnknownTypesAreHandledCorrectly\$DummyClass" + || type.name == "net.corda.nodeapi.internal.serialization.amqp" + + ".StaticInitialisationOfSerializedObjectTest\$MyEnum" + } + + val sf2 = SerializerFactory(WL(), ClassLoader.getSystemClassLoader()) + val bytes = f.readBytes() + val output = DeserializationInput(sf2).deserialize(SerializedBytes(bytes)) + assertEquals(output.a, "foo") + } + @Test fun deserializeTest() { diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.unknownCollection b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.unknownCollection new file mode 100644 index 0000000000000000000000000000000000000000..2f3eaa103a4e454669fd42b9cb0ccd6e7ad1e147 GIT binary patch literal 1007 zcmchWPfi;#7{%=jOvuiBvd&w4mdb=VtZ)A29CfL zELL6g4ralgC99sL4lPpxLa?D5>+|=Xt)HLmn2Q8K0Kmmf&yE1VTL7FpTR7lqkzwV< zQ^)3dwJB@W?+I$OI+chw%LhTJ6l`so!<=^doX?oMK`~5M4+d)|@$Pzj{Hfk;k;c?q zgfqS#MWu~GI4+9rxYG&QMlj-yBkTKMH?4Bs{)d&DVwE5G3io^u$n#c%scp^c%Z%7# zJi%y0Ji-(f3{g*FK@cTpND