agent: Accept duplicated ports if no other option

Commit a04fa4d492 introduced a new feature to try to have a different port for every
local candidate, even if they are on different interfaces. This breaks setups where the
application really wants a specific port and sets a range of exactly 1 port or a very small range.
In that case, if we can't find non-duplicated ports, then we just go around again and skip that check,
but only if both ports are in the same stream and component. Otherwise, we fail the whole component!

diff --git a/agent/agent.c b/agent/agent.c
index 16d8689..2c1a20c 100644 (file)
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -3164,6 +3164,7 @@ nice_agent_gather_candidates (
         NiceCandidateTransport transport;
         guint current_port;
         guint start_port;
+        gboolean accept_duplicate = FALSE;
         HostCandidateResult res = HOST_CANDIDATE_CANT_CREATE_SOCKET;
 
         if ((agent->use_ice_udp == FALSE && add_type == ADD_HOST_UDP) ||
@@ -3194,14 +3195,17 @@ nice_agent_gather_candidates (
             res == HOST_CANDIDATE_DUPLICATE_PORT) {
           nice_debug ("Agent %p: Trying to create host candidate on port %d", agent, current_port);
           nice_address_set_port (addr, current_port);
-          res =  discovery_add_local_host_candidate (agent, stream->id, cid,
-              addr, transport, &host_candidate);
+          res = discovery_add_local_host_candidate (agent, stream->id, cid,
+              addr, transport, accept_duplicate, &host_candidate);
           if (current_port > 0)
             current_port++;
           if (current_port > component->max_port)
             current_port = component->min_port;
-          if (current_port == start_port && res != HOST_CANDIDATE_DUPLICATE_PORT)
-            break;
+          if (current_port == start_port) {
+            if (accept_duplicate)
+              break;
+            accept_duplicate = TRUE;
+          }
           if (current_port == 0 && res != HOST_CANDIDATE_DUPLICATE_PORT)
             break;
         }
@@ -3217,16 +3221,26 @@ nice_agent_gather_candidates (
         } else if (res == HOST_CANDIDATE_CANT_CREATE_SOCKET) {
           if (nice_debug_is_enabled ()) {
             gchar ip[NICE_ADDRESS_STRING_LEN];
+
             nice_address_to_string (addr, ip);
-            nice_debug ("Agent %p: Unable to add local host candidate %s for"
-                " s%d:%d. Invalid interface?", agent, ip, stream->id,
-                component->id);
+            nice_debug ("Agent %p: Unable to add local host %s candidate %s"
+                " for s%d:%d. Invalid interface?", agent,
+                nice_candidate_transport_to_string (transport), ip,
+                stream->id, component->id);
           }
           continue;
         } else if (res == HOST_CANDIDATE_DUPLICATE_PORT) {
-          nice_debug ("Agent %p: Ignoring local candidate, duplicate port",
-              agent);
-          continue;
+           if (nice_debug_is_enabled ()) {
+            gchar ip[NICE_ADDRESS_STRING_LEN];
+
+            nice_address_to_string (addr, ip);
+            nice_debug ("Agent %p: Unable to add local host %s candidate %s"
+                " for"
+                " s%d:%d. Every port is duplicated", agent, ip,
+                nice_candidate_transport_to_string (transport), stream->id,
+                component->id);
+           }
+           goto error;
         }
 
         found_local_address = TRUE;

diff --git a/agent/discovery.c b/agent/discovery.c
index 0abc7ec..05efe54 100644 (file)
--- a/agent/discovery.c
+++ b/agent/discovery.c
@@ -614,7 +614,7 @@ void priv_generate_candidate_credentials (NiceAgent *agent,
 
 static gboolean
 priv_local_host_candidate_duplicate_port (NiceAgent *agent,
-  NiceCandidate *candidate)
+  NiceCandidate *candidate, gboolean accept_duplicate)
 {
   GSList *i, *j, *k;
 
@@ -634,8 +634,26 @@ priv_local_host_candidate_duplicate_port (NiceAgent *agent,
             nice_address_ip_version (&candidate->addr) ==
             nice_address_ip_version (&c->addr) &&
             nice_address_get_port (&candidate->addr) ==
-            nice_address_get_port (&c->addr))
+            nice_address_get_port (&c->addr)) {
+
+          if (accept_duplicate && candidate->stream_id == stream->id &&
+              candidate->component_id == component->id) {
+            /* We accept it anyway, but with a warning! */
+            gchar ip[NICE_ADDRESS_STRING_LEN];
+            gchar ip2[NICE_ADDRESS_STRING_LEN];
+
+            nice_address_to_string (&candidate->addr, ip);
+            nice_address_to_string (&c->addr, ip2);
+            nice_debug ("Agent %p: Local host %s candidate %s"
+                " for s%d:%d will use the same port %d as %s .", agent, ip,
+                nice_candidate_transport_to_string (c->transport),
+                stream->id, component->id, nice_address_get_port (&c->addr),
+                ip2);
+            return FALSE;
+          }
+
           return TRUE;
+        }
       }
     }
   }
@@ -654,6 +672,7 @@ HostCandidateResult discovery_add_local_host_candidate (
   guint component_id,
   NiceAddress *address,
   NiceCandidateTransport transport,
+  gboolean accept_duplicate,
   NiceCandidate **outcandidate)
 {
   NiceCandidate *candidate;
@@ -707,7 +726,7 @@ HostCandidateResult discovery_add_local_host_candidate (
   candidate->addr = nicesock->addr;
   candidate->base_addr = nicesock->addr;
 
-  if (priv_local_host_candidate_duplicate_port (agent, candidate)) {
+  if (priv_local_host_candidate_duplicate_port (agent, candidate, accept_duplicate)) {
     res = HOST_CANDIDATE_DUPLICATE_PORT;
     goto errors;
   }

diff --git a/agent/discovery.h b/agent/discovery.h
index fd1dfe4..72d947d 100644 (file)
--- a/agent/discovery.h
+++ b/agent/discovery.h
@@ -115,6 +115,7 @@ discovery_add_local_host_candidate (
   guint component_id,
   NiceAddress *address,
   NiceCandidateTransport transport,
+  gboolean accept_duplicate,
   NiceCandidate **candidate);
 
 NiceCandidate*

diff --git a/tests/meson.build b/tests/meson.build
index b6d5a03..65ac23a 100644 (file)
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,6 +28,7 @@ nice_tests = [
   'test-drop-invalid',
   'test-nomination',
   'test-interfaces',
+  'test-set-port-range'
 ]
 
 if cc.has_header('arpa/inet.h')

diff --git a/tests/test-set-port-range.c b/tests/test-set-port-range.c
new file mode 100644 (file)
index 0000000..4a19121
--- /dev/null
+++ b/tests/test-set-port-range.c
@@ -0,0 +1,64 @@
+/*
+ * This file is part of the Nice GLib ICE library.
+ *
+ * Unit test for ICE full-mode related features.
+ *
+ * (C)2020 Collabora Ltd
+ *  @author: Olivier Crete <olivier.crete@collabora.com>
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ *
+ *
+ * Alternatively, the contents of this file may be used under the terms of the
+ * the GNU Lesser General Public License Version 2.1 (the "LGPL"), in which
+ * case the provisions of LGPL are applicable instead of those above. If you
+ * wish to allow use of your version of this file only under the terms of the
+ * LGPL and not to allow others to use your version of this file under the
+ * MPL, indicate your decision by deleting the provisions above and replace
+ * them with the notice and other provisions required by the LGPL. If you do
+ * not delete the provisions above, a recipient may use your version of this
+ * file under either the MPL or the LGPL.
+ */
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "agent.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+int main (int argc, char **argv)
+{
+  NiceAgent *agent;
+  guint stream1;
+
+#ifdef G_OS_WIN32
+  WSADATA w;
+
+  WSAStartup(0x0202, &w);
+#endif
+
+  agent = nice_agent_new (NULL, NICE_COMPATIBILITY_RFC5245);
+
+  stream1 = nice_agent_add_stream (agent, 1);
+
+  nice_agent_set_port_range (agent, stream1, 1, 8888, 8888);
+  nice_agent_gather_candidates (agent, stream1);
+
+  g_object_unref (agent);
+
+#ifdef G_OS_WIN32
+  WSACleanup();
+#endif
+  return 0;
+}
\ No newline at end of file