Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invalid params data #5241

Closed
wants to merge 6 commits into from
Closed

Invalid params data #5241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3ba9224
add data field to message in case of invalid params
macfarla Mar 20, 2023
e47f25a
Merge branch 'main' of github.com:hyperledger/besu into invalid-param…
macfarla Mar 20, 2023
c9ce8a8
unused import
macfarla Mar 20, 2023
30ba908
added changelog entry
macfarla Mar 20, 2023
66fab85
compare data field if provided in expected response
macfarla Mar 20, 2023
91c8c7b
Merge branch 'main' of github.com:hyperledger/besu into invalid-param…
macfarla Mar 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion ...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't touch the Engine API does it?
I think error messages are part of the spec in some cases for that: https://github.com/ethereum/execution-apis/blob/main/src/engine/common.md#errors

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) {
Expand Down
12 changes: 9 additions & 3 deletions ...rc/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpBySpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,15 @@ 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");

// 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"));
}
}

Expand Down
54 changes: 45 additions & 9 deletions ...m/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'");
}
}

Expand All @@ -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");
}
}

Expand All @@ -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'");
}
}

Expand All @@ -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'");
}
}

Expand All @@ -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'");
}
}

Expand All @@ -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");
}
}

Expand All @@ -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");
}
}

Expand Down Expand Up @@ -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'");
}
}

Expand Down Expand Up @@ -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'");
}
}

Expand Down
20 changes: 19 additions & 1 deletion ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -53,13 +64,20 @@ protected void assertValidJsonRpcError(
// Check error format
final JsonObject error = json.getJsonObject("error");
final Set<String> 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) {
Expand Down