Skip to content

Commit fd84d92

Browse files
committed
Add comments
1 parent 116c61e commit fd84d92

File tree

2 files changed

+51
-27
lines changed

2 files changed

+51
-27
lines changed

lib/ex_ice/priv/ice_agent.ex

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2220,9 +2220,24 @@ defmodule ExICE.Priv.ICEAgent do
22202220
{local_cand, ice_agent}
22212221

22222222
local_cand ->
2223-
# If selected local candidate uses different socket than received the response
2224-
# Take local candidate from connection check
2225-
# See https://github.com/elixir-webrtc/ex_ice/issues/77
2223+
# When we try to send UDP datagram from bridge interfaces, that can be used to create local candidates,
2224+
# our source IP address is translated from bridge one to our physical network interface card address.
2225+
2226+
# This behavior can cause specific scenarios to arise:
2227+
2228+
# L - local side
2229+
# R - remote side
2230+
# RC1 - remote candidate
2231+
2232+
# 1. L opens socket on interface 1 (I1), port 5000 - first local candidate (LC1)
2233+
# 2. L opens socket on interface 2 (I2), port 5000 - second local candidate (LC2)
2234+
# 3. L sends a connection check from LC1 to RC1. Given LC1 operates via I1, which is a bridge interface, its source address is rewritten to I2
2235+
# 4. R perceives the request from L as originating from I2, port 5000, and responds successfully to I2, port 5000
2236+
# 5. This response arrives to the I1 port 5000. L notices that R recognized it as coming from I2, port 5000
2237+
2238+
# Consequently, LC2 cannot be used to establish a discovered pair
2239+
# As this would lead to a mismatch between the successful and discovered pair sockets.
2240+
# In this case we choose to use LC1 as we are aware that this is bridge interface.
22262241
{conn_check_local_cand, ice_agent}
22272242

22282243
true ->

test/priv/ice_agent_test.exs

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,7 +1371,24 @@ defmodule ExICE.Priv.ICEAgentTest do
13711371
assert new_pair.responses_received == pair.responses_received + 1
13721372
end
13731373

1374-
test "Success response with the address of a local candidate from a different socket." do
1374+
test "success response with the xor address of a local candidate with a different socket." do
1375+
# This test checks a specific scenario where one of the local candidates uses a socket opened on a bridge interface:
1376+
1377+
# L - local side
1378+
# R - remote side
1379+
# RC1 - remote candidate
1380+
1381+
# 1. L opens socket on interface 1 (I1), port 5000 - first local candidate (LC1)
1382+
# 2. L opens socket on interface 2 (I2), port 5000 - second local candidate (LC2)
1383+
# 3. L sends a connection check from LC1 to RC1. Given LC1 operates via I1, which is a bridge interface, its source address is rewritten to I2
1384+
# 4. R perceives the request from L as originating from I2, port 5000, and responds successfully to I2, port 5000
1385+
# 5. This response arrives to the I1 port 5000. L notices that R recognized it as coming from I2, port 5000
1386+
# 6. L chooses to use LC1 and RC1 as the discovered pair because we know that I1 is a bridge interface.
1387+
1388+
# Note: If we were to use LC2 and RC1 as the discovered pair
1389+
# we would have different sockets between the succeeded and discovered pairs, which would cause a runtime error.
1390+
1391+
# Setup ice_agent to have two local candidates
13751392
ice_agent =
13761393
ICEAgent.new(
13771394
controlling_process: self(),
@@ -1384,25 +1401,29 @@ defmodule ExICE.Priv.ICEAgentTest do
13841401
|> ICEAgent.gather_candidates()
13851402
|> ICEAgent.add_remote_candidate(@remote_cand)
13861403

1387-
sockets = ice_agent.sockets
1388-
1404+
# send connectivity check
13891405
ice_agent = ICEAgent.handle_ta_timeout(ice_agent)
1390-
{req, req_socket} = find_binding_request(sockets, ice_agent.remote_pwd)
13911406

1407+
# find candidate pair on which connectivity check was sent
1408+
{_pair_id, pair} =
1409+
Enum.find(ice_agent.checklist, fn {_pair_id, pair} -> pair.state == :in_progress end)
1410+
1411+
local_cand = Map.fetch!(ice_agent.local_cands, pair.local_cand_id)
1412+
req = read_binding_request(local_cand.base.socket, ice_agent.remote_pwd)
1413+
1414+
# create a response that includes the address of the second local candidate and its corresponding socket
13921415
resp =
13931416
binding_response(
13941417
req.transaction_id,
13951418
ice_agent.transport_module,
1396-
# Send response with other socket address
1397-
# See: https://github.com/elixir-webrtc/ex_ice/issues/77
1398-
Enum.find(sockets, &(&1 != req_socket)),
1419+
Enum.find(ice_agent.sockets, &(&1 != local_cand.base.socket)),
13991420
ice_agent.remote_pwd
14001421
)
14011422

14021423
ice_agent =
14031424
ICEAgent.handle_udp(
14041425
ice_agent,
1405-
req_socket,
1426+
local_cand.base.socket,
14061427
@remote_cand.address,
14071428
@remote_cand.port,
14081429
resp
@@ -1413,10 +1434,14 @@ defmodule ExICE.Priv.ICEAgentTest do
14131434
|> Map.values()
14141435
|> Enum.sort(&(&1.priority > &2.priority))
14151436

1437+
# verify that discovered pair is the same as succeeded
14161438
assert pair_1.state == :succeeded
1439+
assert pair_1.id == pair_1.succeeded_pair_id
14171440
assert pair_1.succeeded_pair_id == pair_1.discovered_pair_id
14181441

14191442
assert pair_2.state == :waiting
1443+
assert pair_2.succeeded_pair_id == nil
1444+
assert pair_2.discovered_pair_id == nil
14201445
end
14211446

14221447
test "bad request error response", %{ice_agent: ice_agent, remote_cand: remote_cand} do
@@ -1793,22 +1818,6 @@ defmodule ExICE.Priv.ICEAgentTest do
17931818
end
17941819
end
17951820

1796-
defp find_binding_request(sockets, remote_pwd) do
1797-
{_req, _socket} =
1798-
Enum.find_value(sockets, fn socket ->
1799-
packet = Transport.Mock.recv(socket)
1800-
1801-
case ExSTUN.Message.decode(packet) do
1802-
{:ok, req} ->
1803-
:ok = ExSTUN.Message.authenticate(req, remote_pwd)
1804-
{req, socket}
1805-
1806-
_other ->
1807-
nil
1808-
end
1809-
end)
1810-
end
1811-
18121821
defp read_binding_request(socket, remote_pwd) do
18131822
packet = Transport.Mock.recv(socket)
18141823
{:ok, req} = ExSTUN.Message.decode(packet)

0 commit comments

Comments
 (0)