From afd123a33eaf6d8a4b81c3938e5497b22ee2545e Mon Sep 17 00:00:00 2001 From: CJ Hare Date: Fri, 4 Oct 2019 10:13:08 +1000 Subject: [PATCH] JsonRpc method disabled error condition rewrite and unit test (#80) Signed-off-by: Christopher Hare Signed-off-by: Gregory Markou --- .../api/jsonrpc/JsonRpcHttpService.java | 37 +++++++++++------ .../besu/ethereum/api/jsonrpc/RpcMethods.java | 40 ------------------- .../api/jsonrpc/JsonRpcHttpServiceTest.java | 40 +++++++++++++++++-- 3 files changed, 61 insertions(+), 56 deletions(-) delete mode 100644 ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethods.java diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java index 889314d5199..4886379ab40 100755 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java +++ b/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 rpcMethods; private final Optional natManager; private final Path dataDir; private final LabelledMetric 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 {}", 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 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 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 diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethods.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethods.java deleted file mode 100644 index 6a22549ea0b..00000000000 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethods.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.ethereum.api.jsonrpc; - -import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; - -import java.util.Map; - -public class RpcMethods { - - private Map enabledMethods; - - public RpcMethods(final Map enabledMethods) { - this.enabledMethods = enabledMethods; - } - - public JsonRpcMethod getMethod(final String methodName) { - return enabledMethods.get(methodName); - } - - public boolean isEnabled(final String methodName) { - return enabledMethods.containsKey(methodName); - } - - public boolean isDefined(final String methodName) { - return RpcMethod.rpcMethodExists(methodName) || isEnabled(methodName); - } -} diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java index d80427f00ae..4c5bac65130 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java +++ b/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 =