Added exception if same attachment uploaded. Added test

core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt

@@ -45,6 +45,13 @@ interface AttachmentStorage {
     @Throws(FileAlreadyExistsException::class, IOException::class)
     fun importAttachment(jar: InputStream, uploader: String, filename: String): AttachmentId
 
+    /**
+     * Inserts or returns Attachment Id of attachment. Does not throw an exception if already uploaded.
+     * @param jar [InputStream] of Jar file
+     * @return [AttachmentId] of uploaded attachment
+     */
+    fun importOrGetAttachment(jar: InputStream): AttachmentId
+
     /**
      * Searches attachment using given criteria and optional sort rules
      * @param criteria Query criteria to use as a filter
@@ -53,5 +60,12 @@ interface AttachmentStorage {
      * @return List of AttachmentId of attachment matching criteria, sorted according to given sorting parameter
      */
     fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort? = null): List<AttachmentId>
+
+    /**
+     * Searches for an attachment already in the store
+     * @param attachmentId The attachment Id
+     * @return true if it's in there
+     */
+    fun hasAttachment(attachmentId: AttachmentId): Boolean
 }
 

node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt

@@ -2,18 +2,24 @@ package net.corda.node.internal.cordapp
 
 import com.google.common.collect.HashBiMap
 import net.corda.core.contracts.ContractClassName
-import net.corda.core.crypto.SecureHash
-import net.corda.core.node.services.AttachmentStorage
 import net.corda.core.cordapp.Cordapp
 import net.corda.core.cordapp.CordappContext
+import net.corda.core.crypto.SecureHash
 import net.corda.core.node.services.AttachmentId
+import net.corda.core.node.services.AttachmentStorage
 import net.corda.core.serialization.SingletonSerializeAsToken
+import net.corda.core.utilities.loggerFor
 import java.net.URL
 
 /**
  * Cordapp provider and store. For querying CorDapps for their attachment and vice versa.
  */
 open class CordappProviderImpl(private val cordappLoader: CordappLoader, attachmentStorage: AttachmentStorage) : SingletonSerializeAsToken(), CordappProviderInternal {
+
+    companion object {
+        private val log = loggerFor<CordappProviderImpl>()
+    }
+
     override fun getAppContext(): CordappContext {
         // TODO: Use better supported APIs in Java 9
         Exception().stackTrace.forEach { stackFrame ->
@@ -45,7 +51,7 @@ open class CordappProviderImpl(private val cordappLoader: CordappLoader, attachm
 
     private fun loadContractsIntoAttachmentStore(attachmentStorage: AttachmentStorage): Map<SecureHash, URL> {
         val cordappsWithAttachments = cordapps.filter { !it.contractClassNames.isEmpty() }.map { it.jarPath }
-        val attachmentIds = cordappsWithAttachments.map { it.openStream().use { attachmentStorage.importAttachment(it) } }
+        val attachmentIds = cordappsWithAttachments.map { it.openStream().use { attachmentStorage.importOrGetAttachment(it) }}
         return attachmentIds.zip(cordappsWithAttachments).toMap()
     }
 

node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt

@@ -1,18 +1,19 @@
 package net.corda.node.services.persistence
 
 import com.codahale.metrics.MetricRegistry
-import net.corda.core.internal.VisibleForTesting
 import com.google.common.hash.HashCode
 import com.google.common.hash.Hashing
 import com.google.common.hash.HashingInputStream
 import com.google.common.io.CountingInputStream
 import net.corda.core.CordaRuntimeException
-import net.corda.core.contracts.*
+import net.corda.core.contracts.Attachment
-import net.corda.core.internal.AbstractAttachment
 import net.corda.core.crypto.SecureHash
+import net.corda.core.internal.AbstractAttachment
+import net.corda.core.internal.VisibleForTesting
 import net.corda.core.node.services.AttachmentId
 import net.corda.core.node.services.AttachmentStorage
-import net.corda.core.node.services.vault.*
+import net.corda.core.node.services.vault.AttachmentQueryCriteria
+import net.corda.core.node.services.vault.AttachmentSort
 import net.corda.core.serialization.*
 import net.corda.core.utilities.contextLogger
 import net.corda.node.services.vault.HibernateAttachmentQueryCriteriaParser
@@ -25,7 +26,6 @@ import java.time.Instant
 import java.util.jar.JarInputStream
 import javax.annotation.concurrent.ThreadSafe
 import javax.persistence.*
-import javax.persistence.Column
 
 /**
  * Stores attachments using Hibernate to database.
@@ -33,6 +33,29 @@ import javax.persistence.Column
 @ThreadSafe
 class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, SingletonSerializeAsToken() {
 
+    companion object {
+        private val log = contextLogger()
+
+        // Just iterate over the entries with verification enabled: should be good enough to catch mistakes.
+        // Note that JarInputStream won't throw any kind of error at all if the file stream is in fact not
+        // a ZIP! It'll just pretend it's an empty archive, which is kind of stupid but that's how it works.
+        // So we have to check to ensure we found at least one item.
+        private fun checkIsAValidJAR(stream: InputStream) {
+            val jar = JarInputStream(stream, true)
+            var count = 0
+            while (true) {
+                val cursor = jar.nextJarEntry ?: break
+                val entryPath = Paths.get(cursor.name)
+                // Security check to stop zips trying to escape their rightful place.
+                require(!entryPath.isAbsolute) { "Path $entryPath is absolute" }
+                require(entryPath.normalize() == entryPath) { "Path $entryPath is not normalised" }
+                require(!('\\' in cursor.name || cursor.name == "." || cursor.name == "..")) { "Bad character in $entryPath" }
+                count++
+            }
+            require(count > 0) { "Stream is either empty or not a JAR/ZIP" }
+        }
+    }
+
     @Entity
     @Table(name = "${NODE_DATABASE_PREFIX}attachments",
             indexes = arrayOf(Index(name = "att_id_idx", columnList = "att_id")))
@@ -55,10 +78,6 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single
             var filename: String? = null
     ) : Serializable
 
-    companion object {
-        private val log = contextLogger()
-    }
-
     @VisibleForTesting
     var checkAttachmentsOnLoad = true
 
@@ -170,6 +189,24 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single
         return import(jar, uploader, filename)
     }
 
+    fun getAttachmentIdAndBytes(jar: InputStream): Pair<AttachmentId, ByteArray> {
+        val hs = HashingInputStream(Hashing.sha256(), jar)
+        val bytes = hs.readBytes()
+        checkIsAValidJAR(ByteArrayInputStream(bytes))
+        val id = SecureHash.SHA256(hs.hash().asBytes())
+        return Pair(id, bytes)
+    }
+
+    override fun hasAttachment(attachmentId: AttachmentId): Boolean {
+        val session = currentDBSession()
+        val criteriaBuilder = session.criteriaBuilder
+        val criteriaQuery = criteriaBuilder.createQuery(Long::class.java)
+        val attachments = criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java)
+        criteriaQuery.select(criteriaBuilder.count(criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java)))
+        criteriaQuery.where(criteriaBuilder.equal(attachments.get<String>(DBAttachment::attId.name), attachmentId.toString()))
+        return (session.createQuery(criteriaQuery).singleResult > 0)
+    }
+
     // TODO: PLT-147: The attachment should be randomised to prevent brute force guessing and thus privacy leaks.
     private fun import(jar: InputStream, uploader: String?, filename: String?): AttachmentId {
         require(jar !is JarInputStream)
@@ -179,27 +216,28 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single
         // To do this we must pipe stream into the database without knowing its hash, which we will learn only once
         // the insert/upload is complete. We can then query to see if it's a duplicate and if so, erase, and if not
         // set the hash field of the new attachment record.
-        val hs = HashingInputStream(Hashing.sha256(), jar)
-        val bytes = hs.readBytes()
-        checkIsAValidJAR(ByteArrayInputStream(bytes))
-        val id = SecureHash.SHA256(hs.hash().asBytes())
 
+        val (id, bytes) = getAttachmentIdAndBytes(jar)
+        if (!hasAttachment(id)) {
+            checkIsAValidJAR(ByteArrayInputStream(bytes))
             val session = currentDBSession()
-        val criteriaBuilder = session.criteriaBuilder
-        val criteriaQuery = criteriaBuilder.createQuery(Long::class.java)
-        val attachments = criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java)
-        criteriaQuery.select(criteriaBuilder.count(criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java)))
-        criteriaQuery.where(criteriaBuilder.equal(attachments.get<String>(DBAttachment::attId.name), id.toString()))
-        val count = session.createQuery(criteriaQuery).singleResult
-        if (count == 0L) {
             val attachment = NodeAttachmentService.DBAttachment(attId = id.toString(), content = bytes, uploader = uploader, filename = filename)
             session.save(attachment)
-
             attachmentCount.inc()
             log.info("Stored new attachment $id")
+            return id
+        } else {
+            throw java.nio.file.FileAlreadyExistsException(id.toString())
+        }
     }
 
-        return id
+    override fun importOrGetAttachment(jar: InputStream): AttachmentId {
+        try {
+            return importAttachment(jar)
+        }
+        catch (faee: java.nio.file.FileAlreadyExistsException) {
+            return AttachmentId.parse(faee.message!!)
+        }
     }
 
     override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List<AttachmentId> {
@@ -226,22 +264,4 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single
     }
 
 
-    private fun checkIsAValidJAR(stream: InputStream) {
-        // Just iterate over the entries with verification enabled: should be good enough to catch mistakes.
-        // Note that JarInputStream won't throw any kind of error at all if the file stream is in fact not
-        // a ZIP! It'll just pretend it's an empty archive, which is kind of stupid but that's how it works.
-        // So we have to check to ensure we found at least one item.
-        val jar = JarInputStream(stream, true)
-        var count = 0
-        while (true) {
-            val cursor = jar.nextJarEntry ?: break
-            val entryPath = Paths.get(cursor.name)
-            // Security check to stop zips trying to escape their rightful place.
-            require(!entryPath.isAbsolute) { "Path $entryPath is absolute" }
-            require(entryPath.normalize() == entryPath) { "Path $entryPath is not normalised" }
-            require(!('\\' in cursor.name || cursor.name == "." || cursor.name == "..")) { "Bad character in $entryPath" }
-            count++
-        }
-        require(count > 0) { "Stream is either empty or not a JAR/ZIP" }
-    }
 }

node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt

@@ -241,6 +241,18 @@ class CordaRPCOpsImplTest {
         }
     }
 
+    @Test
+    fun `can't upload the same attachment`() {
+        withPermissions(invokeRpc(CordaRPCOps::uploadAttachment), invokeRpc(CordaRPCOps::attachmentExists)) {
+            assertThatExceptionOfType(java.nio.file.FileAlreadyExistsException::class.java).isThrownBy {
+                val inputJar1 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar)
+                val inputJar2 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar)
+                val secureHash1 = rpc.uploadAttachment(inputJar1)
+                val secureHash2 = rpc.uploadAttachment(inputJar2)
+            }
+        }
+    }
+
     @Test
     fun `can download an uploaded attachment`() {
         withPermissions(invokeRpc(CordaRPCOps::uploadAttachment), invokeRpc(CordaRPCOps::openAttachment)) {

testing/test-utils/src/main/kotlin/net/corda/testing/node/MockAttachmentStorage.kt

@@ -11,21 +11,25 @@ import net.corda.core.node.services.vault.AttachmentSort
 import net.corda.core.serialization.SingletonSerializeAsToken
 import java.io.ByteArrayOutputStream
 import java.io.InputStream
-import java.util.HashMap
+import java.util.*
 import java.util.jar.JarInputStream
 
 class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() {
-
+    companion object {
-    override fun importAttachment(jar: InputStream): AttachmentId {
+        fun getBytes(jar: InputStream) = run {
-        // JIS makes read()/readBytes() return bytes of the current file, but we want to hash the entire container here.
-        require(jar !is JarInputStream)
-
-        val bytes = run {
             val s = ByteArrayOutputStream()
             jar.copyTo(s)
             s.close()
             s.toByteArray()
         }
+    }
+
+    override fun importAttachment(jar: InputStream): AttachmentId {
+        // JIS makes read()/readBytes() return bytes of the current file, but we want to hash the entire container here.
+        require(jar !is JarInputStream)
+
+        val bytes = getBytes(jar)
+
         val sha256 = bytes.sha256()
         if (!files.containsKey(sha256)) {
             files[sha256] = bytes
@@ -49,4 +53,21 @@ class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() {
     override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List<AttachmentId> {
         throw NotImplementedError("Querying for attachments not implemented")
     }
+
+    override fun hasAttachment(attachmentId: AttachmentId) = files.containsKey(attachmentId)
+
+    fun getAttachmentIdAndBytes(jar: InputStream): Pair<AttachmentId, ByteArray> {
+        val bytes = getBytes(jar)
+        return Pair(bytes.sha256(), bytes)
+    }
+
+    override fun importOrGetAttachment(jar: InputStream): AttachmentId {
+        try {
+            return importAttachment(jar)
+        }
+        catch (faee: java.nio.file.FileAlreadyExistsException) {
+            return AttachmentId.parse(faee.message!!)
+        }
+    }
+
 }