You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by dm...@apache.org on 2022/06/28 08:49:03 UTC
[trafficserver] branch 10-Dev updated: JSONRPC: Deal with an empty id as it is an error. (#8923)
This is an automated email from the ASF dual-hosted git repository.
dmeden pushed a commit to branch 10-Dev
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/10-Dev by this push:
new bf5d2594a JSONRPC: Deal with an empty id as it is an error. (#8923)
bf5d2594a is described below
commit bf5d2594a0554f7c7ffaaf0fdf56e15bb0dfd30c
Author: Damian Meden <da...@gmail.com>
AuthorDate: Tue Jun 28 09:48:55 2022 +0100
JSONRPC: Deal with an empty id as it is an error. (#8923)
Co-authored-by: Damian Meden <dm...@apache.org>
---
.../jsonrpc/jsonrpc-handler-development.en.rst | 2 +-
.../jsonrpc/jsonrpc-node-errors.en.rst | 8 +++--
mgmt2/rpc/jsonrpc/error/RPCError.cc | 6 ++--
mgmt2/rpc/jsonrpc/error/RPCError.h | 35 ++++++++--------------
mgmt2/rpc/jsonrpc/json/YAMLCodec.h | 3 ++
.../rpc/jsonrpc/unit_tests/test_basic_protocol.cc | 20 +++++++++++--
6 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst
index f79b21a2c..f5b4cf58e 100644
--- a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst
+++ b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst
@@ -186,7 +186,7 @@ We recommend some ways to deal with this:
This can be set in case you would like to let the server to respond with an |RPC| error, ``ExecutionError`` will be used to catch all the
errors that are fired from within the function call, either by setting the proper errata or by throwing an exception.
-Please check the :ref:`jsonrpc-node-errors` and in particular ``ExecutionError = 9``. Also check :ref:`jsonrpc-handler-errors`
+Please check the :ref:`jsonrpc-node-errors` and in particular ``ExecutionError``. Also check :ref:`jsonrpc-handler-errors`
.. important::
diff --git a/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst
index 0d9253f08..079fe5480 100644
--- a/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst
+++ b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst
@@ -75,7 +75,7 @@ Standard errors
===============
=============== ========================================= =========================================================
-Field Message Description
+Id Message Description
=============== ========================================= =========================================================
-32700 Parse error Invalid JSON was received by the server.
An error occurred on the server while parsing the JSON text.
@@ -93,7 +93,7 @@ Custom errors
The following error list are defined by the server.
=============== ========================================= =========================================================
-Field Message Description
+Id Message Description
=============== ========================================= =========================================================
1 Invalid version, 2.0 only The server only accepts version field equal to `2.0`.
2 Invalid version type, should be a string Version field should be a literal string.
@@ -112,6 +112,8 @@ Field Message Description
10 Unauthorized action The rpc method will not be invoked because the action is not
permitted by some constraint or authorization issue.Check
:ref:`jsonrpc-node-errors-unauthorized-action` for mode details.
+11 Use of an empty string as id is An empty string "" as an id will not be accepted by the server.
+ discouraged
=============== ========================================= =========================================================
.. _jsonrpc-node-errors-unauthorized-action:
@@ -131,7 +133,7 @@ Under this error, the `data` field could be populated with the following errors,
]
=============== ========================================= =========================================================
-Field Message Description
+Id Message Description
=============== ========================================= =========================================================
1 Error getting peer credentials: {} Something happened while trying to get the peers credentials.
The error string will show the error code(`errno`) returned by the
diff --git a/mgmt2/rpc/jsonrpc/error/RPCError.cc b/mgmt2/rpc/jsonrpc/error/RPCError.cc
index 03b8f6a66..be02e5618 100644
--- a/mgmt2/rpc/jsonrpc/error/RPCError.cc
+++ b/mgmt2/rpc/jsonrpc/error/RPCError.cc
@@ -52,22 +52,18 @@ RPCErrorCategory::message(int ev) const
return {"Internal error"};
case RPCErrorCode::PARSE_ERROR:
return {"Parse error"};
- // version
case RPCErrorCode::InvalidVersion:
return {"Invalid version, 2.0 only"};
case RPCErrorCode::InvalidVersionType:
return {"Invalid version type, should be a string"};
case RPCErrorCode::MissingVersion:
return {"Missing version field"};
- // method
case RPCErrorCode::InvalidMethodType:
return {"Invalid method type, should be a string"};
case RPCErrorCode::MissingMethod:
return {"Missing method field"};
- // params
case RPCErrorCode::InvalidParamType:
return {"Invalid params type. A Structured value is expected"};
- // id
case RPCErrorCode::InvalidIdType:
return {"Invalid id type"};
case RPCErrorCode::NullId:
@@ -76,6 +72,8 @@ RPCErrorCategory::message(int ev) const
return {"Error during execution"};
case RPCErrorCode::Unauthorized:
return {"Unauthorized action"};
+ case RPCErrorCode::EmptyId:
+ return {"Use of an empty string as id is discouraged"};
default:
return "Rpc error " + std::to_string(ev);
}
diff --git a/mgmt2/rpc/jsonrpc/error/RPCError.h b/mgmt2/rpc/jsonrpc/error/RPCError.h
index f945c62bf..44f81f8dc 100644
--- a/mgmt2/rpc/jsonrpc/error/RPCError.h
+++ b/mgmt2/rpc/jsonrpc/error/RPCError.h
@@ -39,29 +39,18 @@ enum class RPCErrorCode {
INTERNAL_ERROR = -32603,
PARSE_ERROR = -32700,
- // Custom errors.
-
- // version
- InvalidVersion = 1,
- InvalidVersionType = 2,
- MissingVersion,
- // method
- InvalidMethodType,
- MissingMethod,
-
- // params
- InvalidParamType,
-
- // id
- InvalidIdType,
- NullId = 8,
-
- // execution errors
-
- // Internal rpc error when executing the method.
- //
- ExecutionError, //!< Handler's general error.
- Unauthorized //!< In case we want to block the call based on privileges, access permissions, etc.
+ // Custom errors. A more grained error codes than the main above.
+ InvalidVersion = 1, //!< Version should be equal to "2.0".
+ InvalidVersionType, //!< Invalid string conversion.
+ MissingVersion, //!< Missing version field.
+ InvalidMethodType, //!< Should be a string.
+ MissingMethod, //!< Method name missing.
+ InvalidParamType, //!< Not a valid structured type.
+ InvalidIdType, //!< Invalid string conversion.
+ NullId, //!< null id.
+ ExecutionError, //!< Handler's general error.
+ Unauthorized, //!< In case we want to block the call based on privileges, access permissions, etc.
+ EmptyId //!< Empty id("").
};
// TODO: force non 0 check
std::error_code make_error_code(rpc::error::RPCErrorCode e);
diff --git a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h
index b00919f79..1d7668d78 100644
--- a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h
+++ b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h
@@ -66,6 +66,9 @@ class yamlcpp_json_decoder
try {
request.id = id.as<std::string>();
+ if (request.id.empty()) {
+ return {request, error::RPCErrorCode::EmptyId};
+ }
} catch (YAML::Exception const &) {
return {request, error::RPCErrorCode::InvalidIdType};
}
diff --git a/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc b/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc
index 0732a252b..24aa1fbb1 100644
--- a/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc
+++ b/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc
@@ -533,8 +533,9 @@ TEST_CASE("Basic tests from the jsonrpc 2.0 doc.")
JsonRpcUnitTest rpc;
const auto resp = rpc.handle_call(R"([1,2,3])");
REQUIRE(resp);
- std::string_view expected =
- R"([{"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}}, {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}}, {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}}])";
+ std::string expected = R"([{"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}})"
+ R"(, {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}})"
+ R"(, {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}}])";
REQUIRE(*resp == expected);
}
@@ -591,3 +592,18 @@ TEST_CASE("Call registered notification with ID", "[notification_and_id]")
REQUIRE(*resp == expected);
}
}
+
+TEST_CASE("Call method with invalid ID", "[invalid_id]")
+{
+ JsonRpcUnitTest rpc;
+ SECTION("Basic test, invalid ids")
+ {
+ std::string req = R"([{"id": "", "jsonrpc": "2.0", "method": "will_not_pass_the_validation"},)"
+ R"({"id": {}, "jsonrpc": "2.0", "method": "will_not_pass_the_validation"}])";
+ auto resp = rpc.handle_call(req);
+ std::string expected =
+ R"([{"jsonrpc": "2.0", "error": {"code": 11, "message": "Use of an empty string as id is discouraged"}}, )"
+ R"({"jsonrpc": "2.0", "error": {"code": 7, "message": "Invalid id type"}}])";
+ REQUIRE(*resp == expected);
+ }
+}