JsonRpc method disabled error condition rewrite and unit test (hyperl…

…edger#80)

Signed-off-by: Christopher Hare <chris.hare@consensys.net>
Signed-off-by: edwardmack <ed@edwardmack.com>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java

@@ -82,7 +82,7 @@ public class JsonRpcHttpService {
 
   private final Vertx vertx;
   private final JsonRpcConfiguration config;
-  private final RpcMethods rpcMethods;
+  private final Map<String, JsonRpcMethod> rpcMethods;
   private final Optional<UpnpNatManager> natManager;
   private final Path dataDir;
   private final LabelledMetric<OperationTimer> requestTimer;
@@ -147,7 +147,7 @@ private JsonRpcHttpService(
     this.config = config;
     this.vertx = vertx;
     this.natManager = natManager;
-    this.rpcMethods = new RpcMethods(methods);
+    this.rpcMethods = methods;
     this.authenticationService = authenticationService;
     this.livenessService = livenessService;
     this.readinessService = readinessService;
@@ -479,18 +479,13 @@ private JsonRpcResponse process(final JsonObject requestJson, final Optional<Use
       return NO_RESPONSE;
     }
 
-    LOG.debug("JSON-RPC request -> {}", request.getMethod());
-    // Find method handler
-    final JsonRpcMethod method = rpcMethods.getMethod(request.getMethod());
-    if (method == null) {
-      if (!rpcMethods.isDefined(request.getMethod())) {
-        return errorResponse(id, JsonRpcError.METHOD_NOT_FOUND);
-      }
-      if (!rpcMethods.isEnabled(request.getMethod())) {
-        return errorResponse(id, JsonRpcError.METHOD_NOT_ENABLED);
-      }
+    final Optional<JsonRpcError> unavailableMethod = validateMethodAvailability(request);
+    if (unavailableMethod.isPresent()) {
+      return errorResponse(id, unavailableMethod.get());
     }
 
+    final JsonRpcMethod method = rpcMethods.get(request.getMethod());
+
     if (AuthenticationUtils.isPermitted(authenticationService, user, method)) {
       // Generate response
       try (final OperationTimer.TimingContext ignored =
@@ -508,6 +503,24 @@ private JsonRpcResponse process(final JsonObject requestJson, final Optional<Use
     }
   }
 
+  private Optional<JsonRpcError> validateMethodAvailability(final JsonRpcRequest request) {
+    final String name = request.getMethod();
+    LOG.debug("JSON-RPC request -> {}", name);
+
+    final JsonRpcMethod method = rpcMethods.get(name);
+
+    if (method == null) {
+      if (!RpcMethod.rpcMethodExists(name)) {
+        return Optional.of(JsonRpcError.METHOD_NOT_FOUND);
+      }
+      if (!rpcMethods.containsKey(name)) {
+        return Optional.of(JsonRpcError.METHOD_NOT_ENABLED);
+      }
+    }
+
+    return Optional.empty();
+  }
+
   private void handleJsonRpcError(
       final RoutingContext routingContext, final Object id, final JsonRpcError error) {
     routingContext

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java

@@ -15,10 +15,13 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.config.StubGenesisConfigOptions;
@@ -1472,12 +1475,42 @@ public void unknownMethod() throws Exception {
     }
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  public void disabledMethod() throws Exception {
+    final String methodName = RpcMethod.NET_SERVICES.getMethodName();
+    final String id = "234";
+    final RequestBody body =
+        RequestBody.create(
+            JSON,
+            "{\"jsonrpc\":\"2.0\",\"id\":"
+                + Json.encode(id)
+                + ",\"method\":\""
+                + methodName
+                + "\"}");
+
+    when(rpcMethods.get(any(String.class))).thenReturn(null);
+    when(rpcMethods.containsKey(any(String.class))).thenReturn(false);
+
+    try (final Response resp = client.newCall(buildPostRequest(body)).execute()) {
+      assertThat(resp.code()).isEqualTo(400);
+      final JsonObject json = new JsonObject(resp.body().string());
+      final JsonRpcError expectedError = JsonRpcError.METHOD_NOT_ENABLED;
+      testHelper.assertValidJsonRpcError(
+          json, id, expectedError.getCode(), expectedError.getMessage());
+    }
+
+    verify(rpcMethods).containsKey(methodName);
+    verify(rpcMethods).get(methodName);
+
+    reset(rpcMethods);
+  }
+
   @Test
   public void exceptionallyHandleJsonSingleRequest() throws Exception {
     final JsonRpcMethod jsonRpcMethod = mock(JsonRpcMethod.class);
     when(jsonRpcMethod.getName()).thenReturn("foo");
-    when(jsonRpcMethod.response(ArgumentMatchers.any()))
-        .thenThrow(new RuntimeException("test exception"));
+    when(jsonRpcMethod.response(any())).thenThrow(new RuntimeException("test exception"));
 
     doReturn(Optional.of(jsonRpcMethod)).when(rpcMethods).get("foo");
 
@@ -1493,8 +1526,7 @@ public void exceptionallyHandleJsonSingleRequest() throws Exception {
   public void exceptionallyHandleJsonBatchRequest() throws Exception {
     final JsonRpcMethod jsonRpcMethod = mock(JsonRpcMethod.class);
     when(jsonRpcMethod.getName()).thenReturn("foo");
-    when(jsonRpcMethod.response(ArgumentMatchers.any()))
-        .thenThrow(new RuntimeException("test exception"));
+    when(jsonRpcMethod.response(any())).thenThrow(new RuntimeException("test exception"));
     doReturn(Optional.of(jsonRpcMethod)).when(rpcMethods).get("foo");
 
     final RequestBody body =