From 74e68ad41876675f0b8e4b92fb017834abe3ca4d Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Fri, 20 Sep 2024 11:10:37 +0300 Subject: [PATCH] fix: Allow string IDs in JSON RPC requests (#10759) * fix: Allow string IDs in JSON RPC requests * Remove duplicated line * Do not try convert hex to string - return it as is * Fix sanitize_error else option for Poison --- .../block_scout_web/views/api/eth_rpc/view.ex | 62 ++++++++++++------- apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex | 18 ++++++ .../lib/ethereum_jsonrpc/http.ex | 4 +- 3 files changed, 59 insertions(+), 25 deletions(-) diff --git a/apps/block_scout_web/lib/block_scout_web/views/api/eth_rpc/view.ex b/apps/block_scout_web/lib/block_scout_web/views/api/eth_rpc/view.ex index d2b29c0117..0e22ee5ba6 100644 --- a/apps/block_scout_web/lib/block_scout_web/views/api/eth_rpc/view.ex +++ b/apps/block_scout_web/lib/block_scout_web/views/api/eth_rpc/view.ex @@ -4,6 +4,8 @@ defmodule BlockScoutWeb.API.EthRPC.View do """ use BlockScoutWeb, :view + @jsonrpc_2_0 ~s("jsonrpc":"2.0") + defstruct [:result, :id, :error] def render("show.json", %{result: result, id: id}) do @@ -50,50 +52,64 @@ defmodule BlockScoutWeb.API.EthRPC.View do end) end - defimpl Poison.Encoder, for: BlockScoutWeb.API.EthRPC.View do - def encode(%BlockScoutWeb.API.EthRPC.View{result: result, id: id, error: error}, _options) when is_nil(error) do - result = Poison.encode!(result) + @doc """ + Encodes id into JSON string + """ + @spec sanitize_id(any()) :: non_neg_integer() | String.t() + def sanitize_id(id) do + if is_integer(id), do: id, else: "\"#{id}\"" + end - """ - {"jsonrpc":"2.0","result":#{result},"id":#{id}} - """ + @doc """ + Encodes error into JSON string + """ + @spec sanitize_error(any(), :jason | :poison) :: String.t() + def sanitize_error(error, json_encoder) do + case json_encoder do + :jason -> if is_map(error), do: Jason.encode!(error), else: "\"#{error}\"" + :poison -> if is_map(error), do: Poison.encode!(error), else: "\"#{error}\"" end + end + + @doc """ + Pass "jsonrpc":"2.0" to use in Poison.Encoder and Jason.Encoder below + """ + @spec jsonrpc_2_0() :: String.t() + def jsonrpc_2_0, do: @jsonrpc_2_0 - def encode(%BlockScoutWeb.API.EthRPC.View{id: id, error: error}, _options) when is_map(error) do - error = Poison.encode!(error) + defimpl Poison.Encoder, for: BlockScoutWeb.API.EthRPC.View do + alias BlockScoutWeb.API.EthRPC.View + + def encode(%View{result: result, id: id, error: error}, _options) when is_nil(error) do + result = Poison.encode!(result) """ - {"jsonrpc":"2.0","error": #{error},"id": #{id}} + {#{View.jsonrpc_2_0()},"result": #{result},"id": #{View.sanitize_id(id)}} """ end - def encode(%BlockScoutWeb.API.EthRPC.View{id: id, error: error}, _options) do + def encode(%View{id: id, error: error}, _options) do """ - {"jsonrpc":"2.0","error": "#{error}","id": #{id}} + {#{View.jsonrpc_2_0()},"error": #{View.sanitize_error(error, :poison)},"id": #{View.sanitize_id(id)}} """ end end defimpl Jason.Encoder, for: BlockScoutWeb.API.EthRPC.View do - def encode(%BlockScoutWeb.API.EthRPC.View{result: result, id: id, error: error}, _options) when is_nil(error) do - result = Jason.encode!(result) + # credo:disable-for-next-line + alias BlockScoutWeb.API.EthRPC.View - """ - {"jsonrpc":"2.0","result":#{result},"id":#{id}} - """ - end - - def encode(%BlockScoutWeb.API.EthRPC.View{id: id, error: error}, _options) when is_map(error) do - error = Jason.encode!(error) + def encode(%View{result: result, id: id, error: error}, _options) when is_nil(error) do + result = Jason.encode!(result) """ - {"jsonrpc":"2.0","error": #{error},"id": #{id}} + {#{View.jsonrpc_2_0()},"result": #{result},"id": #{View.sanitize_id(id)}} """ end - def encode(%BlockScoutWeb.API.EthRPC.View{id: id, error: error}, _options) do + def encode(%View{id: id, error: error}, _options) do """ - {"jsonrpc":"2.0","error": "#{error}","id": #{id}} + {#{View.jsonrpc_2_0()},"error": #{View.sanitize_error(error, :jason)},"id": #{View.sanitize_id(id)}} """ end end diff --git a/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex b/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex index 954036218d..c2ac95cb87 100644 --- a/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex +++ b/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex @@ -496,6 +496,24 @@ defmodule EthereumJSONRPC do def quantity_to_integer(_), do: nil + @doc """ + Sanitizes ID in JSON RPC request following JSON RPC [spec](https://www.jsonrpc.org/specification#request_object:~:text=An%20identifier%20established%20by%20the%20Client%20that%20MUST%20contain%20a%20String%2C%20Number%2C%20or%20NULL%20value%20if%20included.%20If%20it%20is%20not%20included%20it%20is%20assumed%20to%20be%20a%20notification.%20The%20value%20SHOULD%20normally%20not%20be%20Null%20%5B1%5D%20and%20Numbers%20SHOULD%20NOT%20contain%20fractional%20parts%20%5B2%5D). + """ + @spec sanitize_id(quantity) :: non_neg_integer() | String.t() | nil + + def sanitize_id(integer) when is_integer(integer), do: integer + + def sanitize_id(string) when is_binary(string) do + # match ID string and ID string without non-ASCII characters + if string == for(<>, c < 128, into: "", do: <>) do + string + else + nil + end + end + + def sanitize_id(_), do: nil + @doc """ Converts `t:non_neg_integer/0` to `t:quantity/0` """ diff --git a/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/http.ex b/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/http.ex index 4335a29e77..a411ab921b 100644 --- a/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/http.ex +++ b/apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/http.ex @@ -7,7 +7,7 @@ defmodule EthereumJSONRPC.HTTP do require Logger - import EthereumJSONRPC, only: [quantity_to_integer: 1] + import EthereumJSONRPC, only: [sanitize_id: 1] @behaviour Transport @@ -190,7 +190,7 @@ defmodule EthereumJSONRPC.HTTP do # argument matching. # Nethermind return string ids - id = quantity_to_integer(unstandardized["id"]) + id = sanitize_id(unstandardized["id"]) standardized = %{jsonrpc: jsonrpc, id: id}