From 8cc091b3e15abc9dd72dbe92a961480d7944ae22 Mon Sep 17 00:00:00 2001 From: Chris Rankin Date: Tue, 26 Sep 2017 08:33:30 +0100 Subject: [PATCH] Transform Kotlin's EmptyList, EmptySet and EmptyMap into Java classes (#1550) * Transform Kotlin's EmptyList, EmptySet and EmptyMap into Java classes before serialising them. * Transform Kotlin's EmptyList, EmptySet and EmptyMap to their unmodifiable Java equivalents. --- build.gradle | 2 +- .../serialization/CordaClassResolver.kt | 37 +++++--- .../serialization/DefaultWhitelist.kt | 4 +- .../serialization/CordaClassResolverTests.kt | 85 +++++++++++++++++-- .../internal/serialization/KryoTests.kt | 26 +++++- .../serialization/ListsSerializationTest.kt | 41 +++++++-- .../serialization/MapsSerializationTest.kt | 27 +++++- .../serialization/SetsSerializationTest.kt | 54 ++++++++++++ 8 files changed, 240 insertions(+), 36 deletions(-) create mode 100644 node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/SetsSerializationTest.kt diff --git a/build.gradle b/build.gradle index ac62af2c8b..f32dfbe9f7 100644 --- a/build.gradle +++ b/build.gradle @@ -36,7 +36,7 @@ buildscript { ext.typesafe_config_version = constants.getProperty("typesafeConfigVersion") ext.fileupload_version = '1.3.2' ext.junit_version = '4.12' - ext.mockito_version = '1.10.19' + ext.mockito_version = '2.10.0' ext.jopt_simple_version = '5.0.2' ext.jansi_version = '1.14' ext.hibernate_version = '5.2.6.Final' diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt index 68b468dc77..09983c8c68 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt @@ -25,9 +25,22 @@ import java.util.* class CordaClassResolver(serializationContext: SerializationContext) : DefaultClassResolver() { val whitelist: ClassWhitelist = TransientClassWhiteList(serializationContext.whitelist) + /* + * These classes are assignment-compatible Java equivalents of Kotlin classes. + * The point is that we do not want to send Kotlin types "over the wire" via RPC. + */ + private val javaAliases: Map, Class<*>> = mapOf( + listOf().javaClass to Collections.emptyList().javaClass, + setOf().javaClass to Collections.emptySet().javaClass, + mapOf().javaClass to Collections.emptyMap().javaClass + ) + + private fun typeForSerializationOf(type: Class<*>): Class<*> = javaAliases[type] ?: type + /** Returns the registration for the specified class, or null if the class is not registered. */ override fun getRegistration(type: Class<*>): Registration? { - return super.getRegistration(type) ?: checkClass(type) + val targetType = typeForSerializationOf(type) + return super.getRegistration(targetType) ?: checkClass(targetType) } private var whitelistEnabled = true @@ -61,9 +74,9 @@ class CordaClassResolver(serializationContext: SerializationContext) : DefaultCl } override fun registerImplicit(type: Class<*>): Registration { - + val targetType = typeForSerializationOf(type) val objectInstance = try { - type.kotlin.objectInstance + targetType.kotlin.objectInstance } catch (t: Throwable) { null // objectInstance will throw if the type is something like a lambda } @@ -74,17 +87,21 @@ class CordaClassResolver(serializationContext: SerializationContext) : DefaultCl kryo.references = true val serializer = when { objectInstance != null -> KotlinObjectSerializer(objectInstance) - kotlin.jvm.internal.Lambda::class.java.isAssignableFrom(type) -> // Kotlin lambdas extend this class and any captured variables are stored in synthetic fields - FieldSerializer(kryo, type).apply { setIgnoreSyntheticFields(false) } - Throwable::class.java.isAssignableFrom(type) -> ThrowableSerializer(kryo, type) - else -> kryo.getDefaultSerializer(type) + kotlin.jvm.internal.Lambda::class.java.isAssignableFrom(targetType) -> // Kotlin lambdas extend this class and any captured variables are stored in synthetic fields + FieldSerializer(kryo, targetType).apply { setIgnoreSyntheticFields(false) } + Throwable::class.java.isAssignableFrom(targetType) -> ThrowableSerializer(kryo, targetType) + else -> kryo.getDefaultSerializer(targetType) } - return register(Registration(type, serializer, NAME.toInt())) + return register(Registration(targetType, serializer, NAME.toInt())) } finally { kryo.references = references } } + override fun writeName(output: Output, type: Class<*>, registration: Registration) { + super.writeName(output, registration.type ?: type, registration) + } + // Trivial Serializer which simply returns the given instance, which we already know is a Kotlin object private class KotlinObjectSerializer(private val objectInstance: Any) : Serializer() { override fun read(kryo: Kryo, input: Input, type: Class): Any = objectInstance @@ -128,10 +145,6 @@ interface MutableClassWhitelist : ClassWhitelist { fun add(entry: Class<*>) } -object EmptyWhitelist : ClassWhitelist { - override fun hasListed(type: Class<*>): Boolean = false -} - class BuiltInExceptionsWhitelist : ClassWhitelist { companion object { private val packageName = "^(?:java|kotlin)(?:[.]|$)".toRegex() diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/DefaultWhitelist.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/DefaultWhitelist.kt index 0053e3d2aa..c602698ade 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/DefaultWhitelist.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/DefaultWhitelist.kt @@ -19,16 +19,14 @@ class DefaultWhitelist : CordaPluginRegistry() { Notification::class.java, Notification.Kind::class.java, ArrayList::class.java, - listOf().javaClass, // EmptyList Pair::class.java, ByteArray::class.java, UUID::class.java, LinkedHashSet::class.java, - setOf().javaClass, // EmptySet Currency::class.java, listOf(Unit).javaClass, // SingletonList setOf(Unit).javaClass, // SingletonSet - mapOf(Unit to Unit).javaClass, // SingletonSet + mapOf(Unit to Unit).javaClass, // SingletonMap NetworkHostAndPort::class.java, SimpleString::class.java, KryoException::class.java, // TODO: Will be removed when we migrate away from Kryo diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt index 36641f911e..3a67501191 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt @@ -3,12 +3,13 @@ package net.corda.nodeapi.internal.serialization import com.esotericsoftware.kryo.* import com.esotericsoftware.kryo.io.Input import com.esotericsoftware.kryo.io.Output +import com.esotericsoftware.kryo.util.DefaultClassResolver import com.esotericsoftware.kryo.util.MapReferenceResolver +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.whenever import net.corda.core.node.services.AttachmentStorage -import net.corda.core.serialization.CordaSerializable -import net.corda.core.serialization.SerializationContext -import net.corda.core.serialization.SerializationFactory -import net.corda.core.serialization.SerializedBytes +import net.corda.core.serialization.* import net.corda.core.utilities.ByteSequence import net.corda.nodeapi.internal.AttachmentsClassLoader import net.corda.nodeapi.internal.AttachmentsClassLoaderTests @@ -19,6 +20,7 @@ import org.junit.rules.ExpectedException import java.lang.IllegalStateException import java.sql.Connection import java.util.* +import kotlin.test.* @CordaSerializable enum class Foo { @@ -56,7 +58,6 @@ open class SerializableViaSubInterface : SerializableSubInterface class SerializableViaSuperSubInterface : SerializableViaSubInterface() - @CordaSerializable class CustomSerializable : KryoSerializable { override fun read(kryo: Kryo?, input: Input?) { @@ -79,7 +80,17 @@ class DefaultSerializableSerializer : Serializer() { } } +object EmptyWhitelist : ClassWhitelist { + override fun hasListed(type: Class<*>): Boolean = false +} + class CordaClassResolverTests { + private companion object { + val emptyListClass = listOf().javaClass + val emptySetClass = setOf().javaClass + val emptyMapClass = mapOf().javaClass + } + val factory: SerializationFactory = object : SerializationFactory() { override fun deserialize(byteSequence: ByteSequence, clazz: Class, context: SerializationContext): T { TODO("not implemented") @@ -88,7 +99,6 @@ class CordaClassResolverTests { override fun serialize(obj: T, context: SerializationContext): SerializedBytes { TODO("not implemented") } - } private val emptyWhitelistContext: SerializationContext = SerializationContextImpl(KryoHeaderV0_1, this.javaClass.classLoader, EmptyWhitelist, emptyMap(), true, SerializationContext.UseCase.P2P) @@ -197,6 +207,69 @@ class CordaClassResolverTests { resolver.getRegistration(HashSet::class.java) } + @Test + fun `Kotlin EmptyList not registered`() { + val resolver = CordaClassResolver(allButBlacklistedContext) + assertNull(resolver.getRegistration(emptyListClass)) + } + + @Test + fun `Kotlin EmptyList registers as Java emptyList`() { + val javaEmptyListClass = Collections.emptyList().javaClass + val kryo = mock() + val resolver = CordaClassResolver(allButBlacklistedContext).apply { setKryo(kryo) } + whenever(kryo.getDefaultSerializer(javaEmptyListClass)).thenReturn(DefaultSerializableSerializer()) + + val registration = resolver.registerImplicit(emptyListClass) + assertNotNull(registration) + assertEquals(javaEmptyListClass, registration.type) + assertEquals(DefaultClassResolver.NAME.toInt(), registration.id) + verify(kryo).getDefaultSerializer(javaEmptyListClass) + assertEquals(registration, resolver.getRegistration(emptyListClass)) + } + + @Test + fun `Kotlin EmptySet not registered`() { + val resolver = CordaClassResolver(allButBlacklistedContext) + assertNull(resolver.getRegistration(emptySetClass)) + } + + @Test + fun `Kotlin EmptySet registers as Java emptySet`() { + val javaEmptySetClass = Collections.emptySet().javaClass + val kryo = mock() + val resolver = CordaClassResolver(allButBlacklistedContext).apply { setKryo(kryo) } + whenever(kryo.getDefaultSerializer(javaEmptySetClass)).thenReturn(DefaultSerializableSerializer()) + + val registration = resolver.registerImplicit(emptySetClass) + assertNotNull(registration) + assertEquals(javaEmptySetClass, registration.type) + assertEquals(DefaultClassResolver.NAME.toInt(), registration.id) + verify(kryo).getDefaultSerializer(javaEmptySetClass) + assertEquals(registration, resolver.getRegistration(emptySetClass)) + } + + @Test + fun `Kotlin EmptyMap not registered`() { + val resolver = CordaClassResolver(allButBlacklistedContext) + assertNull(resolver.getRegistration(emptyMapClass)) + } + + @Test + fun `Kotlin EmptyMap registers as Java emptyMap`() { + val javaEmptyMapClass = Collections.emptyMap().javaClass + val kryo = mock() + val resolver = CordaClassResolver(allButBlacklistedContext).apply { setKryo(kryo) } + whenever(kryo.getDefaultSerializer(javaEmptyMapClass)).thenReturn(DefaultSerializableSerializer()) + + val registration = resolver.registerImplicit(emptyMapClass) + assertNotNull(registration) + assertEquals(javaEmptyMapClass, registration.type) + assertEquals(DefaultClassResolver.NAME.toInt(), registration.id) + verify(kryo).getDefaultSerializer(javaEmptyMapClass) + assertEquals(registration, resolver.getRegistration(emptyMapClass)) + } + open class SubHashSet : HashSet() @Test fun `Check blacklisted subclass`() { 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/KryoTests.kt index 24ed6f19cb..1855a4c8c7 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/KryoTests.kt @@ -25,9 +25,8 @@ import org.slf4j.LoggerFactory import java.io.ByteArrayInputStream import java.io.InputStream import java.time.Instant -import kotlin.test.assertEquals -import kotlin.test.assertNotNull -import kotlin.test.assertTrue +import java.util.Collections +import kotlin.test.* class KryoTests : TestDependencyInjectionBase() { private lateinit var factory: SerializationFactory @@ -113,6 +112,27 @@ class KryoTests : TestDependencyInjectionBase() { assertThat(deserialised).isSameAs(TestSingleton) } + @Test + fun `check Kotlin EmptyList can be serialised`() { + val deserialisedList: List = emptyList().serialize(factory, context).deserialize(factory, context) + assertEquals(0, deserialisedList.size) + assertEquals(Collections.emptyList().javaClass, deserialisedList.javaClass) + } + + @Test + fun `check Kotlin EmptySet can be serialised`() { + val deserialisedSet: Set = emptySet().serialize(factory, context).deserialize(factory, context) + assertEquals(0, deserialisedSet.size) + assertEquals(Collections.emptySet().javaClass, deserialisedSet.javaClass) + } + + @Test + fun `check Kotlin EmptyMap can be serialised`() { + val deserialisedMap: Map = emptyMap().serialize(factory, context).deserialize(factory, context) + assertEquals(0, deserialisedMap.size) + assertEquals(Collections.emptyMap().javaClass, deserialisedMap.javaClass) + } + @Test fun `InputStream serialisation`() { val rubbish = ByteArray(12345, { (it * it * 0.12345).toByte() }) diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt index 2d01f3466c..07780c9fd1 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt @@ -1,18 +1,23 @@ package net.corda.nodeapi.internal.serialization -import net.corda.core.serialization.CordaSerializable -import net.corda.core.serialization.SerializedBytes -import net.corda.core.serialization.deserialize -import net.corda.core.serialization.serialize +import com.esotericsoftware.kryo.Kryo +import com.esotericsoftware.kryo.util.DefaultClassResolver +import net.corda.core.serialization.* import net.corda.node.services.statemachine.SessionData import net.corda.testing.TestDependencyInjectionBase import net.corda.testing.amqpSpecific import org.assertj.core.api.Assertions +import org.junit.Assert.* import org.junit.Test +import java.io.ByteArrayOutputStream import java.io.NotSerializableException -import kotlin.test.assertEquals +import java.nio.charset.StandardCharsets.* +import java.util.* class ListsSerializationTest : TestDependencyInjectionBase() { + private companion object { + val javaEmptyListClass = Collections.emptyList().javaClass + } @Test fun `check list can be serialized as root of serialization graph`() { @@ -23,16 +28,32 @@ class ListsSerializationTest : TestDependencyInjectionBase() { @Test fun `check list can be serialized as part of SessionData`() { - run { val sessionData = SessionData(123, listOf(1)) assertEqualAfterRoundTripSerialization(sessionData) } - run { val sessionData = SessionData(123, listOf(1, 2)) assertEqualAfterRoundTripSerialization(sessionData) } + run { + val sessionData = SessionData(123, emptyList()) + assertEqualAfterRoundTripSerialization(sessionData) + } + } + + @Test + fun `check empty list serialises as Java emptyList`() { + val nameID = 0 + val serializedForm = emptyList().serialize() + val output = ByteArrayOutputStream().apply { + write(KryoHeaderV0_1.bytes) + write(DefaultClassResolver.NAME + 2) + write(nameID) + write(javaEmptyListClass.name.toAscii()) + write(Kryo.NOT_NULL.toInt()) + } + assertArrayEquals(output.toByteArray(), serializedForm.bytes) } @CordaSerializable @@ -55,4 +76,8 @@ internal inline fun assertEqualAfterRoundTripSerialization(obj: val deserializedInstance = serializedForm.deserialize() assertEquals(obj, deserializedInstance) -} \ No newline at end of file +} + +internal fun String.toAscii(): ByteArray = toByteArray(US_ASCII).apply { + this[lastIndex] = (this[lastIndex] + 0x80).toByte() +} diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/MapsSerializationTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/MapsSerializationTest.kt index 78e0e10c31..24bf16dc8a 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/MapsSerializationTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/MapsSerializationTest.kt @@ -1,18 +1,25 @@ package net.corda.nodeapi.internal.serialization +import com.esotericsoftware.kryo.Kryo +import com.esotericsoftware.kryo.util.DefaultClassResolver import net.corda.core.serialization.CordaSerializable import net.corda.core.serialization.serialize import net.corda.node.services.statemachine.SessionData import net.corda.testing.TestDependencyInjectionBase import net.corda.testing.amqpSpecific import org.assertj.core.api.Assertions +import org.junit.Assert.assertArrayEquals import org.junit.Test import org.bouncycastle.asn1.x500.X500Name +import java.io.ByteArrayOutputStream import java.io.NotSerializableException +import java.util.* class MapsSerializationTest : TestDependencyInjectionBase() { - - private val smallMap = mapOf("foo" to "bar", "buzz" to "bull") + private companion object { + val javaEmptyMapClass = Collections.emptyMap().javaClass + val smallMap = mapOf("foo" to "bar", "buzz" to "bull") + } @Test fun `check EmptyMap serialization`() = amqpSpecific("kotlin.collections.EmptyMap is not enabled for Kryo serialization") { @@ -54,4 +61,18 @@ class MapsSerializationTest : TestDependencyInjectionBase() { MyKey(10.0) to MyValue(X500Name("CN=ten"))) assertEqualAfterRoundTripSerialization(myMap) } -} \ No newline at end of file + + @Test + fun `check empty map serialises as Java emptytMap`() { + val nameID = 0 + val serializedForm = emptyMap().serialize() + val output = ByteArrayOutputStream().apply { + write(KryoHeaderV0_1.bytes) + write(DefaultClassResolver.NAME + 2) + write(nameID) + write(javaEmptyMapClass.name.toAscii()) + write(Kryo.NOT_NULL.toInt()) + } + assertArrayEquals(output.toByteArray(), serializedForm.bytes) + } +} diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/SetsSerializationTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/SetsSerializationTest.kt new file mode 100644 index 0000000000..c1c5ea05dd --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/SetsSerializationTest.kt @@ -0,0 +1,54 @@ +package net.corda.nodeapi.internal.serialization + +import com.esotericsoftware.kryo.Kryo +import com.esotericsoftware.kryo.util.DefaultClassResolver +import net.corda.core.serialization.serialize +import net.corda.node.services.statemachine.SessionData +import net.corda.testing.TestDependencyInjectionBase +import org.junit.Assert.* +import org.junit.Test +import java.io.ByteArrayOutputStream +import java.util.* + +class SetsSerializationTest : TestDependencyInjectionBase() { + private companion object { + val javaEmptySetClass = Collections.emptySet().javaClass + } + + @Test + fun `check set can be serialized as root of serialization graph`() { + assertEqualAfterRoundTripSerialization(emptySet()) + assertEqualAfterRoundTripSerialization(setOf(1)) + assertEqualAfterRoundTripSerialization(setOf(1, 2)) + } + + @Test + fun `check set can be serialized as part of SessionData`() { + run { + val sessionData = SessionData(123, setOf(1)) + assertEqualAfterRoundTripSerialization(sessionData) + } + run { + val sessionData = SessionData(123, setOf(1, 2)) + assertEqualAfterRoundTripSerialization(sessionData) + } + run { + val sessionData = SessionData(123, emptySet()) + assertEqualAfterRoundTripSerialization(sessionData) + } + } + + @Test + fun `check empty set serialises as Java emptySet`() { + val nameID = 0 + val serializedForm = emptySet().serialize() + val output = ByteArrayOutputStream().apply { + write(KryoHeaderV0_1.bytes) + write(DefaultClassResolver.NAME + 2) + write(nameID) + write(javaEmptySetClass.name.toAscii()) + write(Kryo.NOT_NULL.toInt()) + } + assertArrayEquals(output.toByteArray(), serializedForm.bytes) + } +}