From 979d7f2c6387ea6a79190acb51e3ed2bd1b988f5 Mon Sep 17 00:00:00 2001 From: Anthony Keenan <34482776+anthonykr3@users.noreply.github.com> Date: Tue, 9 Jan 2018 16:55:16 +0000 Subject: [PATCH] ENT-1226 Improve Network client Error Handling (#2344) * Improve Network client Error Handling * Reformatted NetworkMapServer * Removed line that is now redundant --- .../node/services/network/NetworkMapClient.kt | 20 ++++++----- .../services/network/NetworkMapClientTest.kt | 16 +++++++++ .../node/internal/network/NetworkMapServer.kt | 35 ++++++++++++------- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/node/src/main/kotlin/net/corda/node/services/network/NetworkMapClient.kt b/node/src/main/kotlin/net/corda/node/services/network/NetworkMapClient.kt index db7bea9480..5064361c7b 100644 --- a/node/src/main/kotlin/net/corda/node/services/network/NetworkMapClient.kt +++ b/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() - conn.doOutput = true - conn.requestMethod = "POST" - conn.setRequestProperty("Content-Type", "application/octet-stream") - conn.outputStream.use { signedNodeInfo.serialize().open().copyTo(it) } - - // This will throw IOException if the response code is not HTTP 200. - // This gives a much better exception then reading the error stream. - conn.inputStream.close() + publishURL.openHttpConnection().apply { + doOutput = true + requestMethod = "POST" + setRequestProperty("Content-Type", "application/octet-stream") + outputStream.use { signedNodeInfo.serialize().open().copyTo(it) } + if (responseCode != 200) { + throw IOException("Response Code $responseCode: ${IOUtils.toString(errorStream)}") + } + } } fun getNetworkMap(): NetworkMapResponse { diff --git a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt b/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt index 9f342b2e0b..fa0a67fe15 100644 --- a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt +++ b/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`() { diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt index be1a11c293..810fcedf77 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt +++ b/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) { - check(field == stubNetworkParameters) { "Network parameters can be set only once" } - field = networkParameters - } + set(networkParameters) { + check(field == stubNetworkParameters) { "Network parameters can be set only once" } + 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() private val parametersHash by lazy { serializedParameters.hash } - private val signedParameters by lazy { SignedData( - serializedParameters, - DigitalSignature.WithKey(networkMapKeyAndCert.keyPair.public, Crypto.doSign(networkMapKeyAndCert.keyPair.private, serializedParameters.bytes))) } + private val signedParameters by lazy { + SignedData( + 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() - val nodeInfo = registrationData.verified() - val nodeInfoHash = nodeInfo.serialize().sha256() - nodeInfoMap.put(nodeInfoHash, registrationData) - return ok().build() + return try { + val registrationData = input.readBytes().deserialize() + val nodeInfo = registrationData.verified() + val nodeInfoHash = nodeInfo.serialize().sha256() + 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