Fix RC between sending conn check and settting remote credentials (#68)

Author: mickel8

lib/ex_ice/priv/ice_agent.ex

@@ -242,8 +242,10 @@ defmodule ExICE.Priv.ICEAgent do
     # This is very loosely based on RFC 8863, sec. 4.
     # We can start eoc timer after sending and receiving ICE credentials.
     # In our case, we do this just after receiving remote credentials.
-    ice_agent = start_eoc_timer(ice_agent)
     %__MODULE__{ice_agent | remote_ufrag: ufrag, remote_pwd: pwd}
+    |> start_eoc_timer()
+    # check if timer does not need to be started
+    |> update_ta_timer()
   end
 
   def set_remote_credentials(
@@ -495,6 +497,10 @@ defmodule ExICE.Priv.ICEAgent do
     find_next_transaction(ice_agent, :conn_check)
   end
 
+  defp find_next_transaction(%{remote_ufrag: nil, remote_pwd: nil} = ice_agent, :conn_check) do
+    find_next_transaction(ice_agent, :gathering)
+  end
+
   defp find_next_transaction(ice_agent, :conn_check) do
     case Checklist.get_next_pair(ice_agent.checklist) do
       nil -> find_next_transaction(ice_agent, :gathering)
@@ -2458,7 +2464,8 @@ defmodule ExICE.Priv.ICEAgent do
         t_state in [:waiting, :in_progress]
       end)
 
-    not Checklist.finished?(ice_agent.checklist) or gath_trans_in_progress?
+    (not Checklist.finished?(ice_agent.checklist) and ice_agent.remote_pwd != nil and
+       ice_agent.remote_ufrag != nil) or gath_trans_in_progress?
   end
 
   defp enable_timer(ice_agent) do

test/priv/ice_agent_test.exs

@@ -773,6 +773,55 @@ defmodule ExICE.Priv.ICEAgentTest do
       assert_unauthenticated_error_response(socket, request)
     end
 
+    test "before setting remote candidates", %{
+      ice_agent: ice_agent,
+      remote_cand: remote_cand
+    } do
+      # 1. Receive binding request from the remote side
+      # 2. This will create prflx candidate, pair it with our local candidates
+      # 3. Timer should not be started as we don't have remote credentials
+      # 4. Set remote credentials
+      # 5. Timer should be started as we have conn checks to execute and we also have remote credentials
+      [socket] = ice_agent.sockets
+
+      request =
+        Message.new(%Type{class: :request, method: :binding}, [
+          %Username{value: "#{ice_agent.local_ufrag}:someufrag"},
+          %Priority{priority: 1234},
+          %ICEControlled{tiebreaker: 1234}
+        ])
+        |> Message.with_integrity(ice_agent.local_pwd)
+        |> Message.with_fingerprint()
+
+      raw_request = Message.encode(request)
+
+      ice_agent =
+        ICEAgent.handle_udp(
+          ice_agent,
+          socket,
+          remote_cand.address,
+          remote_cand.port,
+          raw_request
+        )
+
+      # flush the response
+      Transport.Mock.recv(socket)
+
+      # assert timer was not started
+      refute_receive :ta_timeout
+
+      # make sure that even if timer was started, handle_ta_timeout wouldn't try to send conn check
+      ice_agent = ICEAgent.handle_ta_timeout(ice_agent)
+      assert nil == Transport.Mock.recv(socket)
+
+      # set remote credentials and assert timer was started
+      ice_agent = ICEAgent.set_remote_credentials(ice_agent, "remote_ufrag", "remote_pwd")
+      assert_receive :ta_timeout
+
+      # handle timer without errors
+      ICEAgent.handle_ta_timeout(ice_agent)
+    end
+
     defp assert_bad_request_error_response(socket, request) do
       assert packet = Transport.Mock.recv(socket)
       assert is_binary(packet)