Handling decoding errors

config/config.exs

@@ -1,3 +1,3 @@
-use Mix.Config
+import Mix.Config
 
 import_config "#{Mix.env()}.exs"

config/test.exs

@@ -1,3 +1,3 @@
-use Mix.Config
+import Mix.Config
 
 config :logger, level: :info

lib/coap/block.ex

@@ -58,6 +58,10 @@ defmodule CoAP.Block do
   def decode(<<number::size(28), more::size(1), size_exponent::size(3)>>),
     do: decode(number, more, size_exponent)
 
+  def decode(number) do
+    decode(number, 0, 0)
+  end
+
   @spec decode(integer, 0 | 1, integer) :: t()
   def decode(number, more, size_exponent) do
     %__MODULE__{

lib/coap/message_options.ex

@@ -56,7 +56,9 @@ defmodule CoAP.MessageOptions do
           decode(rest, key, append_option(CoAP.MessageOption.decode(key, value), option_list))
 
         <<>> ->
-          decode(<<>>, key, append_option(CoAP.MessageOption.decode(key, <<>>), option_list))
+          option = CoAP.MessageOption.decode(key, <<>>)
+          option_list = append_option(option, option_list)
+          decode(<<>>, key, option_list)
       end
     end
 
@@ -71,8 +73,13 @@ defmodule CoAP.MessageOptions do
             {new_tail1, delta_sum + key + 13}
 
           delta == 14 ->
-            <<key::size(16), new_tail1::binary>> = tail
-            {new_tail1, delta_sum + key + 269}
+            case tail do
+              <<key::size(16), new_tail1::binary>> ->
+                {new_tail1, delta_sum + key + 269}
+
+              _ ->
+                throw({:error, :invalid_option})
+            end
         end
 
       {tail2, option_length} =

lib/coap/socket_server.ex

@@ -91,23 +91,38 @@ defmodule CoAP.SocketServer do
   def handle_info({:udp, _socket, peer_ip, peer_port, data}, state) do
     debug("CoAP socket received raw data #{to_hex(data)} from #{inspect({peer_ip, peer_port})}")
 
-    message = Message.decode(data)
+    case decode_message(data) do
+      {:ok, message} ->
+        connection_id = {peer_ip, peer_port, message.token}
 
-    {connection, new_state} =
-      connection_for(message.request, {peer_ip, peer_port, message.token}, state)
+        {connection, new_state} = connection_for(message.request, connection_id, state)
+
+        if connection do
+          send(connection, {:receive, message})
+
+          {:noreply, new_state}
+        else
+          # If we can't find a connection, we can't deliver the message to the client
+
+          warn(
+            "CoAP socket received message for lost connection from " <>
+              "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  Message: #{inspect(message)}"
+          )
+
+          {:stop, :normal, state}
+        end
+
+      {:error, reason} ->
+        # If we can't decode the message, we can't construct the connection_id which is necessary to lookup
+        # the connection process, which is required to return an error message to the client.
 
-    case connection do
-      nil ->
         warn(
-          "CoAP socket received message for lost connection from " <>
-            "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  Message: #{inspect(message)}"
+          "CoAP socket failed to decode udp packets because #{inspect(reason)} from " <>
+            "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  data: #{inspect(data, limit: :infinity, print_limit: :infinity)}"
         )
 
-      c ->
-        send(c, {:receive, message})
+        {:stop, :normal, state}
     end
-
-    {:noreply, new_state}
   end
 
   # Deliver messages to be sent to a peer
@@ -148,9 +163,7 @@ defmodule CoAP.SocketServer do
     connection = Map.get(state[:connections], connection_id)
 
     debug(
-      "CoAP socket SERVER received DOWN:#{reason} in CoAP.SocketServer from:#{
-        inspect(connection_id)
-      }:#{inspect(connection)}:#{inspect(ref)}"
+      "CoAP socket SERVER received DOWN:#{reason} in CoAP.SocketServer from:#{inspect(connection_id)}:#{inspect(connection)}:#{inspect(ref)}"
     )
 
     {:noreply,
@@ -166,9 +179,7 @@ defmodule CoAP.SocketServer do
     connection = Map.get(state[:connections], connection_id)
 
     debug(
-      "CoAP socket CLIENT received DOWN:#{reason} in CoAP.SocketServer from: #{
-        inspect(connection_id)
-      }:#{inspect(connection)}:#{inspect(ref)}"
+      "CoAP socket CLIENT received DOWN:#{reason} in CoAP.SocketServer from: #{inspect(connection_id)}:#{inspect(connection)}:#{inspect(ref)}"
     )
 
     {:stop, :normal, state}
@@ -239,4 +250,14 @@ defmodule CoAP.SocketServer do
 
   defp type(%{port: 0}), do: :client
   defp type(_), do: :server
+
+  defp decode_message(data) do
+    {:ok, Message.decode(data)}
+  rescue
+    e ->
+      {:error, e}
+  catch
+    e ->
+      {:error, e}
+  end
 end

test/coap/socket_server_test.exs

@@ -0,0 +1,79 @@
+defmodule CoAP.SocketServerTest do
+  use ExUnit.Case
+
+  import ExUnit.CaptureLog
+
+  alias CoAP.SocketServer
+  alias CoAP.Adapters.GenericServer
+
+  defmodule FakeEndpoint do
+  end
+
+  describe "CoAP.SocketServer decoding errors" do
+    test "non repeatable options" do
+      peer_ip = "127.0.0.1"
+      peer_port = 5830
+
+      {:ok, server} = SocketServer.start_link([{GenericServer, FakeEndpoint}, peer_port])
+
+      data =
+        <<100, 69, 0, 1, 252, 1, 46, 56, 68, 80, 231, 168, 249, 100, 69, 0, 1, 252, 1, 46, 56, 68,
+          80, 231, 168, 249>>
+
+      log =
+        capture_log(fn ->
+          ref = Process.monitor(server)
+          send(server, {:udp, :socket, peer_ip, peer_port, data})
+
+          response =
+            receive do
+              {:DOWN, ^ref, :process, ^server, _reason} ->
+                :ok
+            after
+              100 -> {:error, {:timeout, :await_response}}
+            end
+
+          assert response == :ok
+        end)
+
+      assert log =~ "CoAP socket failed to decode udp packets"
+      assert log =~ "is not repeatable"
+      assert log =~ "from ip: \"#{peer_ip}\""
+      assert log =~ "port: #{peer_port}"
+      assert log =~ "data: #{inspect(data, binaries: :as_binary)}"
+    end
+
+    test "CaseClauseError" do
+      peer_ip = "127.0.0.1"
+      peer_port = 5830
+
+      {:ok, server} = SocketServer.start_link([{GenericServer, FakeEndpoint}, peer_port])
+
+      data =
+        <<100, 69, 0, 1, 83, 61, 239, 152, 68, 244, 81, 253, 46, 100, 69, 0, 1, 83, 61, 239, 152,
+          68, 244, 81, 253, 46>>
+
+      log =
+        capture_log(fn ->
+          ref = Process.monitor(server)
+          send(server, {:udp, :socket, peer_ip, peer_port, data})
+
+          response =
+            receive do
+              {:DOWN, ^ref, :process, ^server, _reason} ->
+                :ok
+            after
+              100 -> {:error, {:timeout, :await_response}}
+            end
+
+          assert response == :ok
+        end)
+
+      assert log =~ "CoAP socket failed to decode udp packets"
+      assert log =~ "CaseClauseError"
+      assert log =~ "from ip: \"#{peer_ip}\""
+      assert log =~ "port: #{peer_port}"
+      assert log =~ "data: #{inspect(data, binaries: :as_binary)}"
+    end
+  end
+end