ENT-1226 Improve Network client Error Handling (#2344)

* Improve Network client Error Handling

* Reformatted NetworkMapServer

* Removed line that is now redundant

node/src/main/kotlin/net/corda/node/services/network/NetworkMapClient.kt

@@ -18,9 +18,11 @@ import net.corda.nodeapi.internal.network.SignedNetworkMap
 import net.corda.nodeapi.internal.SignedNodeInfo
 import okhttp3.CacheControl
 import okhttp3.Headers
+import org.apache.commons.io.IOUtils
 import rx.Subscription
 import java.io.BufferedReader
 import java.io.Closeable
+import java.io.IOException
 import java.net.HttpURLConnection
 import java.net.URL
 import java.security.cert.X509Certificate
@@ -33,15 +35,15 @@ class NetworkMapClient(compatibilityZoneURL: URL, private val trustedRoot: X509C
 
     fun publish(signedNodeInfo: SignedNodeInfo) {
         val publishURL = URL("$networkMapUrl/publish")
-        val conn = publishURL.openHttpConnection()
+        publishURL.openHttpConnection().apply {
-        conn.doOutput = true
+            doOutput = true
-        conn.requestMethod = "POST"
+            requestMethod = "POST"
-        conn.setRequestProperty("Content-Type", "application/octet-stream")
+            setRequestProperty("Content-Type", "application/octet-stream")
-        conn.outputStream.use { signedNodeInfo.serialize().open().copyTo(it) }
+            outputStream.use { signedNodeInfo.serialize().open().copyTo(it) }
-
+            if (responseCode != 200) {
-        // This will throw IOException if the response code is not HTTP 200.
+                throw IOException("Response Code $responseCode: ${IOUtils.toString(errorStream)}")
-        // This gives a much better exception then reading the error stream.
+            }
-        conn.inputStream.close()
+        }
     }
 
     fun getNetworkMap(): NetworkMapResponse {

node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt

@@ -9,13 +9,17 @@ import net.corda.testing.BOB_NAME
 import net.corda.testing.DEV_ROOT_CA
 import net.corda.testing.SerializationEnvironmentRule
 import net.corda.testing.driver.PortAllocation
+import net.corda.testing.internal.TestNodeInfoBuilder
 import net.corda.testing.internal.createNodeInfoAndSigned
+import net.corda.testing.internal.signWith
 import net.corda.testing.node.internal.network.NetworkMapServer
 import org.assertj.core.api.Assertions.assertThat
+import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.junit.After
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
+import java.io.IOException
 import java.net.URL
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
@@ -63,6 +67,18 @@ class NetworkMapClientTest {
         assertEquals(nodeInfo2, networkMapClient.getNodeInfo(nodeInfoHash2))
     }
 
+    @Test
+    fun `errors return a meaningful error message`() {
+        val nodeInfoBuilder = TestNodeInfoBuilder()
+        val (_, aliceKey) = nodeInfoBuilder.addIdentity(ALICE_NAME)
+        nodeInfoBuilder.addIdentity(BOB_NAME)
+        val nodeInfo3 = nodeInfoBuilder.build()
+        val signedNodeInfo3 = nodeInfo3.signWith(listOf(aliceKey))
+
+        assertThatThrownBy { networkMapClient.publish(signedNodeInfo3) }
+                .isInstanceOf(IOException::class.java)
+                .hasMessage("Response Code 403: Missing signatures. Found 1 expected 2")
+    }
 
     @Test
     fun `download NetworkParameter correctly`() {

testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt

@@ -25,6 +25,7 @@ import org.glassfish.jersey.servlet.ServletContainer
 import java.io.Closeable
 import java.io.InputStream
 import java.net.InetSocketAddress
+import java.security.SignatureException
 import java.time.Duration
 import java.time.Instant
 import javax.security.auth.x500.X500Principal
@@ -32,6 +33,7 @@ import javax.ws.rs.*
 import javax.ws.rs.core.MediaType
 import javax.ws.rs.core.Response
 import javax.ws.rs.core.Response.ok
+import javax.ws.rs.core.Response.status
 
 class NetworkMapServer(cacheTimeout: Duration,
                        hostAndPort: NetworkHostAndPort,
@@ -58,10 +60,10 @@ class NetworkMapServer(cacheTimeout: Duration,
 
     private val server: Server
     var networkParameters: NetworkParameters = stubNetworkParameters
-      set(networkParameters) {
+        set(networkParameters) {
-          check(field == stubNetworkParameters) { "Network parameters can be set only once" }
+            check(field == stubNetworkParameters) { "Network parameters can be set only once" }
-          field = networkParameters
+            field = networkParameters
-      }
+        }
     private val serializedParameters get() = networkParameters.serialize()
     private val service = InMemoryNetworkMapService(cacheTimeout, networkMapKeyAndCert(rootCa))
 
@@ -108,19 +110,28 @@ class NetworkMapServer(cacheTimeout: Duration,
                                           private val networkMapKeyAndCert: CertificateAndKeyPair) {
         private val nodeInfoMap = mutableMapOf<SecureHash, SignedNodeInfo>()
         private val parametersHash by lazy { serializedParameters.hash }
-        private val signedParameters by lazy { SignedData(
+        private val signedParameters by lazy {
-                serializedParameters,
+            SignedData(
-                DigitalSignature.WithKey(networkMapKeyAndCert.keyPair.public, Crypto.doSign(networkMapKeyAndCert.keyPair.private, serializedParameters.bytes))) }
+                    serializedParameters,
+                    DigitalSignature.WithKey(networkMapKeyAndCert.keyPair.public, Crypto.doSign(networkMapKeyAndCert.keyPair.private, serializedParameters.bytes)))
+        }
 
         @POST
         @Path("publish")
         @Consumes(MediaType.APPLICATION_OCTET_STREAM)
         fun publishNodeInfo(input: InputStream): Response {
-            val registrationData = input.readBytes().deserialize<SignedNodeInfo>()
+            return try {
-            val nodeInfo = registrationData.verified()
+                val registrationData = input.readBytes().deserialize<SignedNodeInfo>()
-            val nodeInfoHash = nodeInfo.serialize().sha256()
+                val nodeInfo = registrationData.verified()
-            nodeInfoMap.put(nodeInfoHash, registrationData)
+                val nodeInfoHash = nodeInfo.serialize().sha256()
-            return ok().build()
+                nodeInfoMap.put(nodeInfoHash, registrationData)
+                ok()
+            } catch (e: Exception) {
+                when (e) {
+                    is SignatureException -> status(Response.Status.FORBIDDEN).entity(e.message)
+                    else -> status(Response.Status.INTERNAL_SERVER_ERROR).entity(e.message)
+                }
+            }.build()
         }
 
         @GET