diff --git a/core-deterministic/build.gradle b/core-deterministic/build.gradle index 73209915d2..636ce86800 100644 --- a/core-deterministic/build.gradle +++ b/core-deterministic/build.gradle @@ -71,6 +71,7 @@ def patchCore = tasks.register('patchCore', Zip) { exclude 'net/corda/core/crypto/SHA256DigestSupplier.class' exclude 'net/corda/core/internal/*ToggleField*.class' exclude 'net/corda/core/serialization/*SerializationFactory*.class' + exclude 'net/corda/core/serialization/internal/AttachmentsHolderImpl.class' exclude 'net/corda/core/serialization/internal/CheckpointSerializationFactory*.class' exclude 'net/corda/core/internal/rules/*.class' } diff --git a/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt b/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt new file mode 100644 index 0000000000..a2f7b8ab30 --- /dev/null +++ b/core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt @@ -0,0 +1,23 @@ +package net.corda.core.serialization.internal + +import net.corda.core.contracts.Attachment +import java.net.URL + +@Suppress("unused") +private class AttachmentsHolderImpl : AttachmentsHolder { + private val attachments = LinkedHashMap>() + + override val size: Int get() = attachments.size + + override fun getKey(key: URL): URL? { + return attachments[key]?.first + } + + override fun get(key: URL): Attachment? { + return attachments[key]?.second + } + + override fun set(key: URL, value: Attachment) { + attachments[key] = key to value + } +} diff --git a/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt b/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt index 6a98b20172..fcc081efb6 100644 --- a/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt +++ b/core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt @@ -1,19 +1,37 @@ package net.corda.coretests.transactions +import net.corda.core.contracts.AlwaysAcceptAttachmentConstraint import net.corda.core.contracts.Attachment +import net.corda.core.contracts.CommandData +import net.corda.core.contracts.CommandWithParties import net.corda.core.contracts.Contract +import net.corda.core.contracts.ContractAttachment +import net.corda.core.contracts.PrivacySalt +import net.corda.core.contracts.StateAndRef +import net.corda.core.contracts.TimeWindow +import net.corda.core.contracts.TransactionState import net.corda.core.contracts.TransactionVerificationException import net.corda.core.crypto.Crypto import net.corda.core.crypto.SecureHash +import net.corda.core.identity.Party +import net.corda.core.internal.AbstractAttachment import net.corda.core.internal.AttachmentTrustCalculator +import net.corda.core.internal.createLedgerTransaction import net.corda.core.internal.declaredField import net.corda.core.internal.hash import net.corda.core.internal.inputStream import net.corda.core.node.NetworkParameters import net.corda.core.node.services.AttachmentId import net.corda.core.serialization.internal.AttachmentsClassLoader -import net.corda.node.services.attachments.NodeAttachmentTrustCalculator import net.corda.testing.common.internal.testNetworkParameters +import net.corda.node.services.attachments.NodeAttachmentTrustCalculator +import net.corda.testing.contracts.DummyContract +import net.corda.testing.core.ALICE_NAME +import net.corda.testing.core.BOB_NAME +import net.corda.testing.core.DUMMY_NOTARY_NAME +import net.corda.testing.core.SerializationEnvironmentRule +import net.corda.testing.core.TestIdentity +import net.corda.testing.core.internal.ContractJarTestUtils import net.corda.testing.core.internal.ContractJarTestUtils.signContractJar import net.corda.testing.internal.TestingNamedCacheFactory import net.corda.testing.internal.fakeAttachment @@ -27,10 +45,14 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder import java.io.ByteArrayOutputStream import java.io.InputStream import java.net.URL +import java.nio.file.Path +import java.security.PublicKey import kotlin.test.assertFailsWith import kotlin.test.fail @@ -47,8 +69,21 @@ class AttachmentsClassLoaderTests { it.toByteArray() } } + val ALICE = TestIdentity(ALICE_NAME, 70).party + val BOB = TestIdentity(BOB_NAME, 80).party + val dummyNotary = TestIdentity(DUMMY_NOTARY_NAME, 20) + val DUMMY_NOTARY get() = dummyNotary.party + val PROGRAM_ID: String = "net.corda.testing.contracts.MyDummyContract" } + @Rule + @JvmField + val tempFolder = TemporaryFolder() + + @Rule + @JvmField + val testSerialization = SerializationEnvironmentRule() + private lateinit var storage: MockAttachmentStorage private lateinit var internalStorage: InternalMockAttachmentStorage private lateinit var attachmentTrustCalculator: AttachmentTrustCalculator @@ -469,4 +504,93 @@ class AttachmentsClassLoaderTests { createClassloader(trustedAttachment).use {} } + + @Test(timeout=300_000) + fun `attachment still available in verify after forced gc in verify`() { + tempFolder.root.toPath().let { path -> + val baseOutState = TransactionState(DummyContract.SingleOwnerState(0, ALICE), PROGRAM_ID, DUMMY_NOTARY, constraint = AlwaysAcceptAttachmentConstraint) + val inputs = emptyList>() + val outputs = listOf(baseOutState, baseOutState.copy(notary = ALICE), baseOutState.copy(notary = BOB)) + val commands = emptyList>() + + val content = createContractString(PROGRAM_ID) + val contractJarPath = ContractJarTestUtils.makeTestContractJar(path, PROGRAM_ID, content = content) + + val attachments = createAttachments(contractJarPath) + + val id = SecureHash.randomSHA256() + val timeWindow: TimeWindow? = null + val privacySalt = PrivacySalt() + val transaction = createLedgerTransaction( + inputs, + outputs, + commands, + attachments, + id, + null, + timeWindow, + privacySalt, + testNetworkParameters(), + emptyList(), + isAttachmentTrusted = { true } + ) + transaction.verify() + } + } + + private fun createContractString(contractName: String, versionSeed: Int = 0): String { + val pkgs = contractName.split(".") + val className = pkgs.last() + val packages = pkgs.subList(0, pkgs.size - 1) + + val output = """package ${packages.joinToString(".")}; + import net.corda.core.contracts.*; + import net.corda.core.transactions.*; + import java.net.URL; + import java.io.InputStream; + + public class $className implements Contract { + private int seed = $versionSeed; + @Override + public void verify(LedgerTransaction tx) throws IllegalArgumentException { + System.gc(); + InputStream str = this.getClass().getClassLoader().getResourceAsStream("importantDoc.pdf"); + if (str == null) throw new IllegalStateException("Could not find importantDoc.pdf"); + } + } + """.trimIndent() + + System.out.println(output) + return output + } + + private fun createAttachments(contractJarPath: Path) : List { + + val attachment = object : AbstractAttachment({contractJarPath.inputStream().readBytes()}, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + } + val contractAttachment = ContractAttachment(attachment, PROGRAM_ID) + + return listOf( + object : AbstractAttachment({ISOLATED_CONTRACTS_JAR_PATH.openStream().readBytes()}, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + }, + object : AbstractAttachment({fakeAttachment("importantDoc.pdf", "I am a pdf!").inputStream().readBytes() + }, uploader = "app") { + @Suppress("OverridingDeprecatedMember") + override val signers: List = emptyList() + override val signerKeys: List = emptyList() + override val size: Int = 1234 + override val id: SecureHash = SecureHash.sha256(attachmentData) + }, + contractAttachment) + } } diff --git a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt index dfb4e95481..e4b00a6c8b 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt @@ -17,6 +17,7 @@ import net.corda.core.utilities.debug import java.io.ByteArrayOutputStream import java.io.IOException import java.io.InputStream +import java.lang.ref.WeakReference import java.net.* import java.security.Permission import java.util.* @@ -53,14 +54,6 @@ class AttachmentsClassLoader(attachments: List, private val ignoreDirectories = listOf("org/jolokia/", "org/json/simple/") private val ignorePackages = ignoreDirectories.map { it.replace("/", ".") } - @VisibleForTesting - private fun readAttachment(attachment: Attachment, filepath: String): ByteArray { - ByteArrayOutputStream().use { - attachment.extractFile(filepath, it) - return it.toByteArray() - } - } - /** * Apply our custom factory either directly, if `URL.setURLStreamHandlerFactory` has not been called yet, * or use a decorator and reflection to bypass the single-call-per-JVM restriction otherwise. @@ -359,8 +352,7 @@ object AttachmentsClassLoaderBuilder { object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory { internal const val attachmentScheme = "attachment" - // TODO - what happens if this grows too large? - private val loadedAttachments = mutableMapOf().toSynchronised() + private val loadedAttachments: AttachmentsHolder = AttachmentsHolderImpl() override fun createURLStreamHandler(protocol: String): URLStreamHandler? { return if (attachmentScheme == protocol) { @@ -368,34 +360,79 @@ object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory { } else null } + @Synchronized fun toUrl(attachment: Attachment): URL { - val id = attachment.id.toString() - loadedAttachments[id] = attachment - return URL(attachmentScheme, "", -1, id, AttachmentURLStreamHandler) + val proposedURL = URL(attachmentScheme, "", -1, attachment.id.toString(), AttachmentURLStreamHandler) + val existingURL = loadedAttachments.getKey(proposedURL) + return if (existingURL == null) { + loadedAttachments[proposedURL] = attachment + proposedURL + } else { + existingURL + } } + @VisibleForTesting + fun loadedAttachmentsSize(): Int = loadedAttachments.size + private object AttachmentURLStreamHandler : URLStreamHandler() { override fun openConnection(url: URL): URLConnection { if (url.protocol != attachmentScheme) throw IOException("Cannot handle protocol: ${url.protocol}") - val attachment = loadedAttachments[url.path] ?: throw IOException("Could not load url: $url .") + val attachment = loadedAttachments[url] ?: throw IOException("Could not load url: $url .") return AttachmentURLConnection(url, attachment) } - } - private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) { - override fun getContentLengthLong(): Long = attachment.size.toLong() - override fun getInputStream(): InputStream = attachment.open() - /** - * Define the permissions that [AttachmentsClassLoader] will need to - * use this [URL]. The attachment is stored in memory, and so we - * don't need any extra permissions here. But if we don't override - * [getPermission] then [AttachmentsClassLoader] will assign the - * default permission of ALL_PERMISSION to these classes' - * [java.security.ProtectionDomain]. This would be a security hole! - */ - override fun getPermission(): Permission? = null - override fun connect() { - connected = true + override fun equals(attachmentUrl: URL, otherURL: URL?): Boolean { + if (attachmentUrl.protocol != otherURL?.protocol) return false + if (attachmentUrl.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${attachmentUrl.protocol}") + return attachmentUrl.file == otherURL?.file + } + + override fun hashCode(url: URL): Int { + if (url.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${url.protocol}") + return url.file.hashCode() } } +} + +interface AttachmentsHolder { + val size: Int + fun getKey(key: URL): URL? + operator fun get(key: URL): Attachment? + operator fun set(key: URL, value: Attachment) +} + +private class AttachmentsHolderImpl : AttachmentsHolder { + private val attachments = WeakHashMap, Attachment>>().toSynchronised() + + override val size: Int get() = attachments.size + + override fun getKey(key: URL): URL? { + return attachments[key]?.first?.get() + } + + override fun get(key: URL): Attachment? { + return attachments[key]?.second + } + + override fun set(key: URL, value: Attachment) { + attachments[key] = WeakReference(key) to value + } +} + +private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) { + override fun getContentLengthLong(): Long = attachment.size.toLong() + override fun getInputStream(): InputStream = attachment.open() + /** + * Define the permissions that [AttachmentsClassLoader] will need to + * use this [URL]. The attachment is stored in memory, and so we + * don't need any extra permissions here. But if we don't override + * [getPermission] then [AttachmentsClassLoader] will assign the + * default permission of ALL_PERMISSION to these classes' + * [java.security.ProtectionDomain]. This would be a security hole! + */ + override fun getPermission(): Permission? = null + override fun connect() { + connected = true + } } \ No newline at end of file diff --git a/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt b/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt index 3e19491b94..68bd3ba625 100644 --- a/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt +++ b/core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt @@ -5,13 +5,20 @@ import net.corda.core.contracts.ContractAttachment import net.corda.core.contracts.ContractClassName import net.corda.core.crypto.SecureHash import net.corda.core.identity.Party +import net.corda.core.node.services.AttachmentId import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory import net.corda.core.serialization.internal.AttachmentsClassLoader import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Ignore import org.junit.Test import java.io.ByteArrayOutputStream +import java.lang.ref.ReferenceQueue +import java.lang.ref.WeakReference +import java.net.URL import java.net.URLClassLoader import java.security.PublicKey import java.util.jar.JarOutputStream @@ -120,9 +127,125 @@ class ClassLoadingUtilsTest { } } - private fun signedAttachment(data: ByteArray, vararg parties: Party) = ContractAttachment.create( + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test weak reference removed from map`() { + val jarData = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + val attachment = signedAttachment(jarData) + var url: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment) + + val referenceQueue: ReferenceQueue = ReferenceQueue() + val weakReference = WeakReference(url, referenceQueue) + + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + // Clear strong reference + url = null + System.gc() + val ref = referenceQueue.remove(100000) + assertSame(weakReference, ref) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test adding same attachment twice then removing`() { + val jarData = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + val attachment1 = signedAttachment(jarData) + val attachment2 = signedAttachment(jarData) + var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1) + var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2) + + val referenceQueue1: ReferenceQueue = ReferenceQueue() + val weakReference1 = WeakReference(url1, referenceQueue1) + + val referenceQueue2: ReferenceQueue = ReferenceQueue() + val weakReference2 = WeakReference(url2, referenceQueue2) + + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + url1 = null + System.gc() + val ref1 = referenceQueue1.remove(500) + assertNull(ref1) + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + + url2 = null + System.gc() + val ref2 = referenceQueue2.remove(100000) + assertSame(weakReference2, ref2) + assertSame(weakReference1, referenceQueue1.poll()) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + @Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.") + @Test(timeout=300_000) + @Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE") + fun `test adding two different attachments then removing`() { + val jarData1 = with(ByteArrayOutputStream()) { + val internalName = STANDALONE_CLASS_NAME.asInternalName + JarOutputStream(this, Manifest()).use { + it.setLevel(NO_COMPRESSION) + it.setMethod(DEFLATED) + it.putNextEntry(directoryEntry("com")) + it.putNextEntry(directoryEntry("com/example")) + it.putNextEntry(classEntry(internalName)) + it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName)) + } + toByteArray() + } + + val attachment1 = signedAttachment(jarData1) + val attachment2 = signedAttachment(jarData1, id = SecureHash.randomSHA256()) + var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1) + var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2) + + val referenceQueue1: ReferenceQueue = ReferenceQueue() + val weakReference1 = WeakReference(url1, referenceQueue1) + + val referenceQueue2: ReferenceQueue = ReferenceQueue() + val weakReference2 = WeakReference(url2, referenceQueue2) + + assertEquals(2, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + url1 = null + System.gc() + val ref1 = referenceQueue1.remove(100000) + assertSame(weakReference1, ref1) + assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + + url2 = null + System.gc() + val ref2 = referenceQueue2.remove(100000) + assertSame(weakReference2, ref2) + assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize()) + } + + private fun signedAttachment(data: ByteArray, id: AttachmentId = contractAttachmentId, + vararg parties: Party) = ContractAttachment.create( object : AbstractAttachment({ data }, "test") { - override val id: SecureHash get() = contractAttachmentId + override val id: SecureHash get() = id override val signerKeys: List get() = parties.map(Party::owningKey) }, PROGRAM_ID, signerKeys = parties.map(Party::owningKey) diff --git a/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt b/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt index cefc596c3a..413f2bf843 100644 --- a/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt +++ b/testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt @@ -59,12 +59,14 @@ object ContractJarTestUtils { return workingDir.resolve(jarName) to signer } + @Suppress("LongParameterList") @JvmOverloads - fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0): Path { + fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0, + content: String? = null): Path { val packages = contractName.split(".") val jarName = "attachment-${packages.last()}-$version-$versionSeed-${(if (signed) "signed" else "")}.jar" val className = packages.last() - createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed) + createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed, content) workingDir.createJar(jarName, "${contractName.replace(".", "/")}.class") workingDir.addManifest(jarName, Pair(Attributes.Name(CORDAPP_CONTRACT_VERSION), version.toString())) return workingDir.resolve(jarName) @@ -87,8 +89,8 @@ object ContractJarTestUtils { return workingDir.resolve(jarName) } - private fun createTestClass(workingDir: Path, className: String, packages: List, versionSeed: Int = 0): Path { - val newClass = """package ${packages.joinToString(".")}; + private fun createTestClass(workingDir: Path, className: String, packages: List, versionSeed: Int = 0, content: String? = null): Path { + val newClass = content ?: """package ${packages.joinToString(".")}; import net.corda.core.contracts.*; import net.corda.core.transactions.*;