From 7d3b585a9c884864f868d0228ce0912612fbdddd Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Fri, 2 Mar 2018 11:18:24 +0000 Subject: [PATCH 1/6] RELEASE: Bump platform version past V3 (#2701) --- constants.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/constants.properties b/constants.properties index afaa2b79c8..3aeb950c6c 100644 --- a/constants.properties +++ b/constants.properties @@ -1,6 +1,6 @@ gradlePluginsVersion=4.0.3 kotlinVersion=1.2.20 -platformVersion=2 +platformVersion=4 guavaVersion=21.0 bouncycastleVersion=1.57 typesafeConfigVersion=1.3.1 From 617838c108f237961c9ea477f13fe17c6748d0b0 Mon Sep 17 00:00:00 2001 From: Joel Dudley Date: Fri, 2 Mar 2018 11:20:29 +0000 Subject: [PATCH 2/6] Removes output that is no longer logged. --- docs/source/tutorial-cordapp.rst | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/docs/source/tutorial-cordapp.rst b/docs/source/tutorial-cordapp.rst index d64cc06718..503d799045 100644 --- a/docs/source/tutorial-cordapp.rst +++ b/docs/source/tutorial-cordapp.rst @@ -323,22 +323,7 @@ And click submit. Upon clicking submit, the modal dialogue will close, and the n Checking the output ^^^^^^^^^^^^^^^^^^^ -Assuming all went well, you should see some activity in PartyA's web-server terminal window: - -.. sourcecode:: none - - >> Signing transaction with our private key. - >> Gathering the counterparty's signature. - >> Collecting signatures from counter-parties. - >> Verifying collected signatures. - >> Done - >> Obtaining notary signature and recording transaction. - >> Requesting signature by notary service - >> Broadcasting transaction to participants - >> Done - >> Done - -You can view the newly-created IOU by accessing the vault of PartyA or PartyB: +Assuming all went well, you can view the newly-created IOU by accessing the vault of PartyA or PartyB: *Via the HTTP API:* @@ -505,4 +490,4 @@ Debugging is done via IntelliJ as follows: 5. Set your breakpoints and interact with the node you've connected to. When the node hits a breakpoint, execution will pause - * The node webserver runs in a separate process, and is not attached to by the debugger \ No newline at end of file + * The node webserver runs in a separate process, and is not attached to by the debugger From 41bdad5aa29b0d4d360321de0b79bc8307e73831 Mon Sep 17 00:00:00 2001 From: Joel Dudley Date: Fri, 2 Mar 2018 12:49:44 +0000 Subject: [PATCH 3/6] Makes it clear that monitoring must be turned on. --- docs/source/node-administration.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/source/node-administration.rst b/docs/source/node-administration.rst index 4132ac9385..9acaee8b6e 100644 --- a/docs/source/node-administration.rst +++ b/docs/source/node-administration.rst @@ -75,7 +75,7 @@ You will now be able to browse the tables and row data within them. Monitoring your node -------------------- -Like most Java servers, the node exports various useful metrics and management operations via the industry-standard +Like most Java servers, the node can be configured to export various useful metrics and management operations via the industry-standard `JMX infrastructure `_. JMX is a standard API for registering so-called *MBeans* ... objects whose properties and methods are intended for server management. It does not require any particular network protocol for export. So this data can be exported from the node in various ways: @@ -87,7 +87,7 @@ them out to a statistics collector over the network. For those systems, follow t agent `here `_. `Jolokia `_ allows you to access the raw data and operations without connecting to the JMX port -directly. The nodes export the data over HTTP on the ``/jolokia`` HTTP endpoint, Jolokia defines the JSON and REST +directly. Nodes can be configured to export the data over HTTP on the ``/jolokia`` HTTP endpoint, Jolokia defines the JSON and REST formats for accessing MBeans, and provides client libraries to work with that protocol as well. Here are a few ways to build dashboards and extract monitoring data for a node: @@ -145,4 +145,4 @@ node is running out of memory, you can give it more by running the node like thi The example command above would give a 1 gigabyte Java heap. -.. note:: Unfortunately the JVM does not let you limit the total memory usage of Java program, just the heap size. \ No newline at end of file +.. note:: Unfortunately the JVM does not let you limit the total memory usage of Java program, just the heap size. From 06a6eace67fa44205cc0b2d8aaf3bf7afcc63a3f Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Fri, 2 Mar 2018 13:13:00 +0000 Subject: [PATCH 4/6] CORDA-1115 - Cannot serialize private nested objects (#2665) * CORDA-1115 - Cannot serialize private nested objects Shown up by the simm-valuation-demo the problem was where a private object field of an object was being serialised within the outer objects context (see tests added for example) Fix is to switch from Kotlin reflection back to Java. Additional fix to the test where it was comparing two lists of state references in a flow and they weren't equal because they weren't in the same order... This I assume is just an oversight (in that them being in a different order but otherwise the same is actually fine) so converting to set comparison * Fix forward port issue where fingerprinting has moved * Review Comments * Review Comments * Review Comments * Gran -> Grab --- .../serialization/amqp/FingerPrinter.kt | 2 +- .../serialization/amqp/SerializationHelper.kt | 45 +++++++++++++++++++ .../serialization/amqp/SerializerFactory.kt | 13 +++--- .../amqp/SerializationOutputTests.kt | 32 ++++++++++++- .../kotlin/net/corda/vega/flows/SimmFlow.kt | 3 +- 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/FingerPrinter.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/FingerPrinter.kt index e5cc478acb..79ee00f5e5 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/FingerPrinter.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/FingerPrinter.kt @@ -153,7 +153,7 @@ class SerializerFingerPrinter : FingerPrinter { }.putUnencodedChars(type.name).putUnencodedChars(ENUM_HASH) } else { hasher.fingerprintWithCustomSerializerOrElse(factory!!, type, type) { - if (type.kotlin.objectInstance != null) { + if (type.objectInstance() != null) { // TODO: name collision is too likely for kotlin objects, we need to introduce some reference // to the CorDapp but maybe reference to the JAR in the short term. hasher.putUnencodedChars(type.name) 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 83e3486d5a..4c70dc0cd2 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 @@ -513,3 +513,48 @@ fun ClassWhitelist.hasAnnotationInHierarchy(type: Class<*>): Boolean { || type.interfaces.any { hasAnnotationInHierarchy(it) } || (type.superclass != null && hasAnnotationInHierarchy(type.superclass)) } + +/** + * By default use Kotlin reflection and grab the objectInstance. However, that doesn't play nicely with nested + * private objects. Even setting the accessibility override (setAccessible) still causes an + * [IllegalAccessException] when attempting to retrieve the value of the INSTANCE field. + * + * Whichever reference to the class Kotlin reflection uses, override (set from setAccessible) on that field + * isn't set even when it was explicitly set as accessible before calling into the kotlin reflection routines. + * + * For example + * + * clazz.getDeclaredField("INSTANCE")?.apply { + * isAccessible = true + * kotlin.objectInstance // This throws as the INSTANCE field isn't accessible + * } + * + * Therefore default back to good old java reflection and simply look for the INSTANCE field as we are never going + * to serialize a companion object. + * + * As such, if objectInstance fails access, revert to Java reflection and try that + */ +fun Class<*>.objectInstance() = + try { + this.kotlin.objectInstance + } catch (e: IllegalAccessException) { + // Check it really is an object (i.e. it has no constructor) + if (constructors.isNotEmpty()) null + else { + try { + this.getDeclaredField("INSTANCE")?.let { field -> + // and must be marked as both static and final (>0 means they're set) + if (modifiers and Modifier.STATIC == 0 || modifiers and Modifier.FINAL == 0) null + else { + val accessibility = field.isAccessible + field.isAccessible = true + val obj = field.get(null) + field.isAccessible = accessibility + obj + } + } + } catch (e: NoSuchFieldException) { + null + } + } + } 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 011b04dbf7..94f319f697 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 @@ -267,12 +267,15 @@ open class SerializerFactory( // Don't need to check the whitelist since each element will come back through the whitelisting process. if (clazz.componentType.isPrimitive) PrimArraySerializer.make(type, this) else ArraySerializer.make(type, this) - } else if (clazz.kotlin.objectInstance != null) { - whitelist.requireWhitelisted(clazz) - SingletonSerializer(clazz, clazz.kotlin.objectInstance!!, this) } else { - whitelist.requireWhitelisted(type) - ObjectSerializer(type, this) + val singleton = clazz.objectInstance() + if (singleton != null) { + whitelist.requireWhitelisted(clazz) + SingletonSerializer(clazz, singleton, this) + } else { + whitelist.requireWhitelisted(type) + ObjectSerializer(type, this) + } } } } 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 f8aa4fc768..e69a949dbd 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 @@ -47,6 +47,22 @@ import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertTrue +object AckWrapper { + object Ack + fun serialize() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(Ack) + } +} + +object PrivateAckWrapper { + private object Ack + fun serialize() { + val factory = testDefaultFactoryNoEvolution() + SerializationOutput(factory).serialize(Ack) + } +} + @RunWith(Parameterized::class) class SerializationOutputTests(private val compression: CordaSerializationEncoding?) { private companion object { @@ -1148,4 +1164,18 @@ class SerializationOutputTests(private val compression: CordaSerializationEncodi assertEquals(encodingNotPermittedFormat.format(compression), message) } } -} \ No newline at end of file + + @Test + fun nestedObjects() { + // The "test" is that this doesn't throw, anything else is a success + AckWrapper.serialize() + } + + @Test + fun privateNestedObjects() { + // The "test" is that this doesn't throw, anything else is a success + PrivateAckWrapper.serialize() + } + +} + diff --git a/samples/simm-valuation-demo/src/main/kotlin/net/corda/vega/flows/SimmFlow.kt b/samples/simm-valuation-demo/src/main/kotlin/net/corda/vega/flows/SimmFlow.kt index cfccafbc4d..2424101214 100644 --- a/samples/simm-valuation-demo/src/main/kotlin/net/corda/vega/flows/SimmFlow.kt +++ b/samples/simm-valuation-demo/src/main/kotlin/net/corda/vega/flows/SimmFlow.kt @@ -305,7 +305,8 @@ object SimmFlow { @Suspendable private fun agreePortfolio(portfolio: Portfolio): SignedTransaction { logger.info("Handshake finished, awaiting Simm offer") - require(offer.dealBeingOffered.portfolio == portfolio.refs) + + require(offer.dealBeingOffered.portfolio.toSet() == portfolio.refs.toSet()) val seller = TwoPartyDealFlow.Instigator( replyToSession, From 799d90b35030e000f928c621c580f346d22e5916 Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Fri, 2 Mar 2018 13:21:27 +0000 Subject: [PATCH 5/6] CORDA-1134 - Don't use private serializes for all caps public properties (#2692) * CORDA-1134 - Don't use private serializes for all caps public properties * Small fix * Review Comments --- .../serialization/amqp/SerializationHelper.kt | 74 ++++++++++++------- .../amqp/PrivatePropertyTests.kt | 28 +++++++ 2 files changed, 74 insertions(+), 28 deletions(-) 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 4c70dc0cd2..cc8591c612 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 @@ -136,7 +136,16 @@ fun Class.propertyDescriptors(): Map { this.field = property } } + clazz = clazz.superclass + } while (clazz != null) + // + // Running as two loops rather than one as we need to ensure we have captured all of the properties + // before looking for interacting methods and need to cope with the class hierarchy introducing + // new properties / methods + // + clazz = this + do { // Note: It is possible for a class to have multiple instances of a function where the types // differ. For example: // interface I { val a: T } @@ -148,14 +157,23 @@ fun Class.propertyDescriptors(): Map { // // In addition, only getters that take zero parameters and setters that take a single // parameter will be considered - clazz.declaredMethods?.map { func -> + clazz!!.declaredMethods?.map { func -> if (!Modifier.isPublic(func.modifiers)) return@map if (func.name == "getClass") return@map PropertyDescriptorsRegex.re.find(func.name)?.apply { - // take into account those constructor properties that don't directly map to a named - // property which are, by default, already added to the map - classProperties.computeIfAbsent(groups[2]!!.value.decapitalize()) { PropertyDescriptor() }.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 + // parameter that shadows a property + val properties = + classProperties[groups[2]!!.value] ?: + classProperties[groups[2]!!.value.decapitalize()] ?: + // take into account those constructor properties that don't directly map to a named + // property which are, by default, already added to the map + classProperties.computeIfAbsent(groups[2]!!.value) { PropertyDescriptor() } + + properties.apply { when (groups[1]!!.value) { "set" -> { if (func.parameterCount == 1) { @@ -221,38 +239,38 @@ internal fun propertiesForSerializationFromConstructor( // it so just ignore it as it'll be supplied at runtime anyway on invocation val name = param.value.name ?: return@forEach - val propertyReader = if (name in classProperties) { - if (classProperties[name]!!.getter != null) { - // it's a publicly accessible property - val matchingProperty = classProperties[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 + // but has a getter + val matchingProperty = classProperties[name] ?: classProperties[name.capitalize()] ?: + throw NotSerializableException( + "Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"") - // Check that the method has a getter in java. - val getter = matchingProperty.getter ?: throw NotSerializableException( - "Property has no getter method for - \"$name\" - of \"$clazz\". If using Java and the parameter name" - + "looks anonymous, check that you have the -parameters option specified in the " - + "Java compiler. Alternately, provide a proxy serializer " - + "(SerializationCustomSerializer) if recompiling isn't an option.") + // If the property has a getter we'll use that to retrieve it's value from the instance, if it doesn't + // *for *know* we switch to a reflection based method + val propertyReader = if (matchingProperty.getter != null) { + val getter = matchingProperty.getter ?: throw NotSerializableException( + "Property has no getter method for - \"$name\" - of \"$clazz\". If using Java and the parameter name" + + "looks anonymous, check that you have the -parameters option specified in the " + + "Java compiler. Alternately, provide a proxy serializer " + + "(SerializationCustomSerializer) if recompiling isn't an option.") - val returnType = resolveTypeVariables(getter.genericReturnType, type) - if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) { - throw NotSerializableException( - "Property - \"$name\" - has type \"$returnType\" on \"$clazz\" but differs from constructor " + - "parameter type \"${param.value.type.javaType}\"") - } + val returnType = resolveTypeVariables(getter.genericReturnType, type) + if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) { + throw NotSerializableException( + "Property - \"$name\" - has type \"$returnType\" on \"$clazz\" but differs from constructor " + + "parameter type \"${param.value.type.javaType}\"") + } - Pair(PublicPropertyReader(getter), returnType) - } else { - val field = classProperties[name]!!.field ?: + Pair(PublicPropertyReader(getter), returnType) + } else { + val field = classProperties[name]!!.field ?: throw NotSerializableException("No property matching constructor parameter named - \"$name\" - " + "of \"$clazz\". If using Java, check that you have the -parameters option specified " + "in the Java compiler. Alternately, provide a proxy serializer " + "(SerializationCustomSerializer) if recompiling isn't an option") - Pair(PrivatePropertyReader(field, type), field.genericType) - } - } else { - throw NotSerializableException( - "Constructor parameter - \"$name\" - doesn't refer to a property of \"$clazz\"") + Pair(PrivatePropertyReader(field, type), field.genericType) } this += PropertyAccessorConstructor( diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt index ae35d46286..3f8efa4407 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt @@ -196,4 +196,32 @@ class PrivatePropertyTests { assertEquals(c1, c2) } + + // + // Reproduces CORDA-1134 + // + @Suppress("UNCHECKED_CAST") + @Test + fun allCapsProprtyNotPrivate() { + data class C (val CCC: String) + + val output = SerializationOutput(factory).serializeAndReturnSchema(C("this is nice")) + + val serializersByDescriptor = fields["serializersByDesc"]!!.get(factory) as ConcurrentHashMap> + + val schemaDescriptor = output.schema.types.first().descriptor.name + serializersByDescriptor.filterKeys { (it as Symbol) == schemaDescriptor }.values.apply { + assertEquals(1, size) + + assertTrue(this.first() is ObjectSerializer) + val propertySerializers = (this.first() as ObjectSerializer).propertySerializers.serializationOrder.map { it.getter } + + // CCC is the only property to be serialised + assertEquals(1, propertySerializers.size) + + // and despite being all caps it should still be a public getter + assertTrue(propertySerializers[0].propertyReader is PublicPropertyReader) + } + } + } \ No newline at end of file From 80c00b920b56fc2e67abdd6bfc7ce160c4e04365 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Fri, 2 Mar 2018 15:10:54 +0100 Subject: [PATCH 6/6] Fix another non-serializable exception, add docs, fix a possible security issue. (#2707) * Fix another non-serializable exception, add docs, fix a possible security issue. * Update API definition to reflect methods added to make more exceptions serializable --- .ci/api-current.txt | 12 ++++++++ .../kotlin/net/corda/core/CordaInternal.kt | 2 +- .../net/corda/core/flows/FlowLogicRef.kt | 30 ++++++++++++++++--- .../statemachine/FlowLogicRefFactoryImpl.kt | 26 ++++++++++------ 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/.ci/api-current.txt b/.ci/api-current.txt index 34656174f5..bd17464368 100644 --- a/.ci/api-current.txt +++ b/.ci/api-current.txt @@ -563,15 +563,20 @@ public final class net.corda.core.contracts.TransactionStateKt extends java.lang ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ConflictingAttachmentsRejection extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractConstraintRejection extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractCreationError extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String, Throwable) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractRejection extends net.corda.core.contracts.TransactionVerificationException + public (net.corda.core.crypto.SecureHash, String, Throwable) public (net.corda.core.crypto.SecureHash, net.corda.core.contracts.Contract, Throwable) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$Direction extends java.lang.Enum protected (String, int) @@ -594,12 +599,17 @@ public final class net.corda.core.contracts.TransactionStateKt extends java.lang ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$NotaryChangeInWrongTransactionType extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, net.corda.core.identity.Party, net.corda.core.identity.Party) + @org.jetbrains.annotations.NotNull public final net.corda.core.identity.Party getOutputNotary() + @org.jetbrains.annotations.NotNull public final net.corda.core.identity.Party getTxNotary() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$SignersMissing extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, List) + @org.jetbrains.annotations.NotNull public final List getMissing() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$TransactionMissingEncumbranceException extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, int, net.corda.core.contracts.TransactionVerificationException$Direction) + @org.jetbrains.annotations.NotNull public final net.corda.core.contracts.TransactionVerificationException$Direction getInOut() + public final int getMissing() ## @net.corda.core.serialization.CordaSerializable public abstract class net.corda.core.contracts.TypeOnlyCommandData extends java.lang.Object implements net.corda.core.contracts.CommandData public () @@ -1327,6 +1337,8 @@ public static final class net.corda.core.flows.FlowStackSnapshot$Frame extends j ## @net.corda.core.serialization.CordaSerializable public final class net.corda.core.flows.IllegalFlowLogicException extends java.lang.IllegalArgumentException public (Class, String) + public (String, String) + @org.jetbrains.annotations.NotNull public final String getType() ## public @interface net.corda.core.flows.InitiatedBy public abstract Class value() diff --git a/core/src/main/kotlin/net/corda/core/CordaInternal.kt b/core/src/main/kotlin/net/corda/core/CordaInternal.kt index 40f145fdec..eb13b5b3c5 100644 --- a/core/src/main/kotlin/net/corda/core/CordaInternal.kt +++ b/core/src/main/kotlin/net/corda/core/CordaInternal.kt @@ -6,6 +6,6 @@ package net.corda.core * These fields are only meant to be used by Corda internally, and are not intended to be part of the public API. */ @Retention(AnnotationRetention.BINARY) -@Target(AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.PROPERTY_SETTER) +@Target(AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.PROPERTY_SETTER, AnnotationTarget.FUNCTION) @MustBeDocumented annotation class CordaInternal \ No newline at end of file diff --git a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt index 36b0971cb0..4558498114 100644 --- a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt +++ b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt @@ -1,10 +1,12 @@ package net.corda.core.flows +import net.corda.core.CordaInternal import net.corda.core.DoNotImplement import net.corda.core.serialization.CordaSerializable /** - * The public factory interface for creating validated FlowLogicRef instances as part of the scheduling framework. + * The public factory interface for creating validated [FlowLogicRef] instances as part of the scheduling framework. + * * Typically this would be used from within the nextScheduledActivity method of a QueryableState to specify * the flow to run at the scheduled time. */ @@ -14,20 +16,40 @@ interface FlowLogicRefFactory { * Construct a FlowLogicRef. This is intended for cases where the calling code has the relevant class already * and can provide it directly. */ - @Deprecated("This should be avoided, and the version which takes a class name used instead to avoid requiring the class on the classpath to deserialize calling code") fun create(flowClass: Class>, vararg args: Any?): FlowLogicRef + /** * Construct a FlowLogicRef. This is intended for cases where the calling code does not want to require the flow * class on the classpath for all cases where the calling code is loaded. */ fun create(flowClassName: String, vararg args: Any?): FlowLogicRef + + /** + * @suppress + * This is an internal method and should not be used: use [create] instead, which checks for the + * [SchedulableFlow] annotation. + */ + @CordaInternal fun createForRPC(flowClass: Class>, vararg args: Any?): FlowLogicRef + + /** + * Converts a [FlowLogicRef] object that was obtained from the calls above into a [FlowLogic], after doing some + * validation to ensure it points to a legitimate flow class. + */ fun toFlowLogic(ref: FlowLogicRef): FlowLogic<*> } +/** + * Thrown if the structure of a class implementing a flow is not correct. There can be several causes for this such as + * not inheriting from [FlowLogic], not having a valid constructor and so on. + * + * @property type the fully qualified name of the class that failed checks. + */ @CordaSerializable -class IllegalFlowLogicException(type: Class<*>, msg: String) : IllegalArgumentException( - "${FlowLogicRef::class.java.simpleName} cannot be constructed for ${FlowLogic::class.java.simpleName} of type ${type.name} $msg") +class IllegalFlowLogicException(val type: String, msg: String) : + IllegalArgumentException("A FlowLogicRef cannot be constructed for FlowLogic of type $type: $msg") { + constructor(type: Class<*>, msg: String) : this(type.name, msg) +} /** * A handle interface representing a [FlowLogic] instance which would be possible to safely pass out of the contract sandbox. diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt index 8d1aba86c6..9ed350c080 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt @@ -41,15 +41,21 @@ class FlowLogicRefFactoryImpl(private val classloader: ClassLoader) : SingletonS } override fun create(flowClassName: String, vararg args: Any?): FlowLogicRef { - val flowClass = Class.forName(flowClassName, true, classloader).asSubclass(FlowLogic::class.java) - if (flowClass == null) { - throw IllegalArgumentException("The class $flowClassName is not a subclass of FlowLogic.") - } else { - if (!flowClass.isAnnotationPresent(SchedulableFlow::class.java)) { - throw IllegalFlowLogicException(flowClass, "because it's not a schedulable flow") - } - return createForRPC(flowClass, *args) + val flowClass = validatedFlowClassFromName(flowClassName) + if (!flowClass.isAnnotationPresent(SchedulableFlow::class.java)) { + throw IllegalFlowLogicException(flowClass, "because it's not a schedulable flow") } + return createForRPC(flowClass, *args) + } + + private fun validatedFlowClassFromName(flowClassName: String): Class> { + val forName = try { + Class.forName(flowClassName, true, classloader) + } catch (e: ClassNotFoundException) { + throw IllegalFlowLogicException(flowClassName, "Flow not found: $flowClassName") + } + return forName.asSubclass(FlowLogic::class.java) ?: + throw IllegalFlowLogicException(flowClassName, "The class $flowClassName is not a subclass of FlowLogic.") } override fun createForRPC(flowClass: Class>, vararg args: Any?): FlowLogicRef { @@ -93,7 +99,9 @@ class FlowLogicRefFactoryImpl(private val classloader: ClassLoader) : SingletonS override fun toFlowLogic(ref: FlowLogicRef): FlowLogic<*> { if (ref !is FlowLogicRefImpl) throw IllegalFlowLogicException(ref.javaClass, "FlowLogicRef was not created via correct FlowLogicRefFactory interface") - val klass = Class.forName(ref.flowLogicClassName, true, classloader).asSubclass(FlowLogic::class.java) + // We re-validate here because a FlowLogicRefImpl could have arrived via deserialization and therefore the + // class name could point to anything at all. + val klass = validatedFlowClassFromName(ref.flowLogicClassName) return createConstructor(klass, ref.args)() }