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

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
   """
-      {"jsonrpc":"2.0","result":#{result},"id":#{id}}
+  @spec sanitize_id(any()) :: non_neg_integer() | String.t()
+  def sanitize_id(id) do
+    if is_integer(id), do: id, else: "\"#{id}\""
+  end
+
+  @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
 
-    def encode(%BlockScoutWeb.API.EthRPC.View{id: id, error: error}, _options) when is_map(error) do
-      error = Poison.encode!(error)
+  @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
+
+  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

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 <- string>>, c < 128, into: "", do: <<c>>) do
+      string
+    else
+      nil
+    end
+  end
+
+  def sanitize_id(_), do: nil
+
   @doc """
   Converts `t:non_neg_integer/0` to `t:quantity/0`
   """

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}