From 3ba9224c5e07afbbb9654017b8605ea1748f7064 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 20 Mar 2023 12:25:55 +1000 Subject: [PATCH 1/4] add data field to message in case of invalid params Signed-off-by: Sally MacFarlane --- .../api/jsonrpc/execution/BaseJsonRpcProcessor.java | 4 +++- .../api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java index 8beba6af9b4..13fb6a8fb66 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java @@ -41,8 +41,10 @@ public JsonRpcResponse process( try { return method.response(request); } catch (final InvalidJsonRpcParameters e) { + JsonRpcError invalidParamsError = JsonRpcError.INVALID_PARAMS; + invalidParamsError.setData(e.getMessage()); LOG.debug("Invalid Params for method: {}", method.getName(), e); - return new JsonRpcErrorResponse(id, JsonRpcError.INVALID_PARAMS); + return new JsonRpcErrorResponse(id, invalidParamsError); } catch (final MultiTenancyValidationException e) { return new JsonRpcUnauthorizedResponse(id, JsonRpcError.UNAUTHORIZED); } catch (final RuntimeException e) { diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java index 4bb9fb11479..5efd4359d4e 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java @@ -44,6 +44,7 @@ import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -261,9 +262,13 @@ private void checkResponse( // Check error if (expectedResponse.has("error")) { assertThat(responseBody.has("error")).isTrue(); - final String expectedError = expectedResponse.get("error").toString(); - final String actualError = responseBody.get("error").toString(); - assertThat(actualError).isEqualToIgnoringWhitespace(expectedError); + + final JsonNode expectedErrorJson = expectedResponse.get("error"); + final JsonNode actualErrorJson = responseBody.get("error"); + + // ignore data field, it's only used to provide extra error info for INVALID_PARAMS + assertThat(actualErrorJson.get("code")).isEqualTo(expectedErrorJson.get("code")); + assertThat(actualErrorJson.get("message")).isEqualTo(expectedErrorJson.get("message")); } } From c9ce8a8f6059f75f7bc46532523ce42cc29d3b21 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 20 Mar 2023 13:13:08 +1000 Subject: [PATCH 2/4] unused import Signed-off-by: Sally MacFarlane --- .../besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java index 5efd4359d4e..43161ae3698 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java @@ -44,7 +44,6 @@ import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; -import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; From 30ba90887dfa3478f30af653e23c43820e1f43bd Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 20 Mar 2023 14:03:03 +1000 Subject: [PATCH 3/4] added changelog entry Signed-off-by: Sally MacFarlane --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e608495f2..ab695816d9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Breaking Changes ### Additions and Improvements +- Add data field to JsonRpcError in case of `INVALID_PARAMS` error [#5241](https://github.com/hyperledger/besu/pull/5241) ### Bug Fixes @@ -351,7 +352,7 @@ https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/22.10.0-RC1/besu-22. ### Bug Fixes - Corrects treating a block as bad on internal error [#4512](https://github.com/hyperledger/besu/issues/4512) -- update appache-commons-text to 1.10.0 to address CVE-2022-42889 [#4542](https://github.com/hyperledger/besu/pull/4542) +- update apache-commons-text to 1.10.0 to address CVE-2022-42889 [#4542](https://github.com/hyperledger/besu/pull/4542) - In GraphQL update scalar parsing to be variable friendly [#4522](https://github.com/hyperledger/besu/pull/4522) ### Download Links From 66fab85073a94ec6f1086bf61a97e182c3c34567 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 20 Mar 2023 14:52:35 +1000 Subject: [PATCH 4/4] compare data field if provided in expected response Signed-off-by: Sally MacFarlane --- .../AbstractJsonRpcHttpBySpecTest.java | 6 ++- .../api/jsonrpc/JsonRpcHttpServiceTest.java | 54 +++++++++++++++---- .../api/jsonrpc/JsonRpcTestHelper.java | 20 ++++++- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java index 43161ae3698..a3c247b642f 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java @@ -261,11 +261,13 @@ private void checkResponse( // Check error if (expectedResponse.has("error")) { assertThat(responseBody.has("error")).isTrue(); - final JsonNode expectedErrorJson = expectedResponse.get("error"); final JsonNode actualErrorJson = responseBody.get("error"); - // ignore data field, it's only used to provide extra error info for INVALID_PARAMS + // compare data field if explicitly included in expected response + if (expectedErrorJson.has("data")) { + assertThat(actualErrorJson.get("data")).isEqualTo(expectedErrorJson.get("data")); + } assertThat(actualErrorJson.get("code")).isEqualTo(expectedErrorJson.get("code")); assertThat(actualErrorJson.get("message")).isEqualTo(expectedErrorJson.get("message")); } 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 1d258021448..341556e0a90 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 @@ -764,7 +764,11 @@ public void getBlockByHashWithMissingHashParameter() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 0. Supplied value was: 'true' of type: 'java.lang.Boolean' - expected type: 'org.hyperledger.besu.datatypes.Hash'"); } } @@ -788,7 +792,11 @@ public void getBlockByHashWithMissingBooleanParameter() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Missing required json rpc parameter at index 1"); } } @@ -811,7 +819,11 @@ public void getBlockByHashWithInvalidHashParameterWithOddLength() throws Excepti final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 0. Supplied value was: '0xe' of type: 'java.lang.String' - expected type: 'org.hyperledger.besu.datatypes.Hash'"); } } @@ -834,7 +846,11 @@ public void getBlockByHashWithInvalidHashParameterThatIsTooShort() throws Except final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 0. Supplied value was: '0xe670' of type: 'java.lang.String' - expected type: 'org.hyperledger.besu.datatypes.Hash'"); } } @@ -858,7 +874,11 @@ public void getBlockByHashWithInvalidBooleanParameter() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 1. Supplied value was: '{}' of type: 'java.util.LinkedHashMap' - expected type: 'java.lang.Boolean'"); } } @@ -878,7 +898,11 @@ public void getBlockByHashWithAllParametersMissing() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Missing required json rpc parameter at index 0"); } } @@ -898,7 +922,11 @@ public void getBlockByHashWithNoParameters() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Missing required json rpc parameter at index 0"); } } @@ -984,7 +1012,11 @@ public void getBlockByNumberForInvalidBlockParameter() throws Exception { final JsonObject json = new JsonObject(respBody); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 0. Supplied value was: 'bla' of type: 'java.lang.String' - expected type: 'org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.BlockParameter'"); } } @@ -2040,7 +2072,11 @@ public void ethGetStorageAtInvalidParameterStorageIndex() throws Exception { final JsonObject json = new JsonObject(resp.body().string()); final JsonRpcError expectedError = JsonRpcError.INVALID_PARAMS; testHelper.assertValidJsonRpcError( - json, id, expectedError.getCode(), expectedError.getMessage()); + json, + id, + expectedError.getCode(), + expectedError.getMessage(), + "Invalid json rpc parameter at index 1. Supplied value was: 'blah' of type: 'java.lang.String' - expected type: 'org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.UInt256Parameter'"); } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestHelper.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestHelper.java index 7e5762d3336..b236e606d79 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestHelper.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestHelper.java @@ -37,6 +37,17 @@ protected void assertValidJsonRpcResult(final JsonObject json, final Object id) } protected void assertValidJsonRpcError( + final JsonObject json, + final Object id, + final int errorCode, + final String errorMessage, + final String data) + throws Exception { + + assertThat(data).isEqualTo(assertValidJsonRpcError(json, id, errorCode, errorMessage)); + } + + protected String assertValidJsonRpcError( final JsonObject json, final Object id, final int errorCode, final String errorMessage) throws Exception { // Check all expected fieldnames are set @@ -53,13 +64,20 @@ protected void assertValidJsonRpcError( // Check error format final JsonObject error = json.getJsonObject("error"); final Set errorFieldNames = error.fieldNames(); - assertThat(errorFieldNames.size()).isEqualTo(2); + assertThat(errorFieldNames.size()).isGreaterThanOrEqualTo(2); + // 3 if data field is present ie INVALID_PARAMS + assertThat(errorFieldNames.size()).isLessThanOrEqualTo(3); + assertThat(errorFieldNames.contains("code")).isTrue(); assertThat(errorFieldNames.contains("message")).isTrue(); // Check error field values assertThat(error.getInteger("code")).isEqualTo(errorCode); assertThat(error.getString("message")).isEqualTo(errorMessage); + if (errorFieldNames.contains("data")) { + return error.getString("data"); + } + return null; } protected void assertIdMatches(final JsonObject json, final Object expectedId) {