Merge "fix(hicn-light): error handling for invalid connections + local bind/remote connect returning EINVAL in tests"

2 files changed, 36 insertions, 43 deletions

diff --git a/hicn-light/src/hicn/core/listener.c b/hicn-light/src/hicn/core/listener.c
index a3c5c8a12..58ba53fe1 100644
--- a/hicn-light/src/hicn/core/listener.c
+++ b/hicn-light/src/hicn/core/listener.c
@@ -189,40 +189,46 @@ unsigned listener_create_connection(listener_t *listener,
   assert(listener_has_valid_type(listener));
   assert(pair);
 
-  connection_table_t *table =
-      forwarder_get_connection_table(listener->forwarder);
-  connection_t *connection =
-      connection_table_allocate(table, pair, connection_name);
-  unsigned connection_id =
-      (unsigned int)connection_table_get_connection_id(table, connection);
+  /* initialized so that gcc-9 does not complain */
+  face_type_t connection_type = FACE_TYPE_UNDEFINED;
+  switch (listener->type) {
+    case FACE_TYPE_UDP_LISTENER:
+      connection_type = FACE_TYPE_UDP;
+      break;
+    case FACE_TYPE_TCP_LISTENER:
+      connection_type = FACE_TYPE_TCP;
+      break;
+    case FACE_TYPE_HICN:
+    case FACE_TYPE_HICN_LISTENER:
+    case FACE_TYPE_UDP:
+    case FACE_TYPE_TCP:
+    case FACE_TYPE_UNDEFINED:
+    case FACE_TYPE_N:
+      return CONNECTION_ID_UNDEFINED;
+  }
 
+#ifdef USE_CONNECTED_SOCKETS
   /*
    * We create a connected connection with its own fd, instead of returning
    * the fd of the listener. This will allow to avoid specifying the
    * destination address when sending packets, and will increase performance
    * by avoiding a FIB lookup for each packet.
    */
-#ifdef USE_CONNECTED_SOCKETS
   int fd = listener_get_socket(listener, address_pair_get_local(pair),
                                address_pair_get_remote(pair),
                                listener->interface_name);
+  if (fd < 0) return CONNECTION_ID_UNDEFINED;
 #else
   int fd = 0;  // means listener->fd;
 #endif
   bool local = address_is_local(address_pair_get_local(pair));
 
-  face_type_t connection_type;
-  switch (listener->type) {
-    case FACE_TYPE_UDP_LISTENER:
-      connection_type = FACE_TYPE_UDP;
-      break;
-    case FACE_TYPE_TCP_LISTENER:
-      connection_type = FACE_TYPE_TCP;
-      break;
-    default:
-      connection_table_remove_by_id(table, connection_id);
-      return CONNECTION_ID_UNDEFINED;
-  }
+  connection_table_t *table =
+      forwarder_get_connection_table(listener->forwarder);
+  connection_t *connection =
+      connection_table_allocate(table, pair, connection_name);
+  unsigned connection_id =
+      (unsigned int)connection_table_get_connection_id(table, connection);
 
   int rc = connection_initialize(connection, connection_type, connection_name,
                                  listener->interface_name, fd, pair, local,
diff --git a/hicn-light/src/hicn/test/test-strategy-local-remote.cc b/hicn-light/src/hicn/test/test-strategy-local-remote.cc
index b6e84d946..8230b9482 100644
--- a/hicn-light/src/hicn/test/test-strategy-local-remote.cc
+++ b/hicn-light/src/hicn/test/test-strategy-local-remote.cc
@@ -73,9 +73,9 @@ class StrategyLocalRemoteTest : public ::testing::Test {
 };
 
 TEST_F(StrategyLocalRemoteTest, InputLocalOutputLocal) {
-  address_t listener_addr = ADDRESS4_LOCALHOST(9596);
   address_t prod_addr = ADDRESS4_LOCALHOST(12345);
-  address_t cons_addr = ADDRESS4_LOCALHOST(54321);
+  address_t cons_addr = ADDRESS4_LOCALHOST(12346);
+  address_t listener_addr = ADDRESS4_LOCALHOST(9596);
 
   listener_t* listener = listener_create(FACE_TYPE_UDP_LISTENER, &listener_addr,
                                          "lo", "lo_udp4", fwd_);
@@ -105,15 +105,8 @@ TEST_F(StrategyLocalRemoteTest, InputLocalOutputLocal) {
 }
 
 TEST_F(StrategyLocalRemoteTest, InputRemoteOutputRemote) {
-  const char prod_addr_str[] = "192.168.1.1";
-  const char cons_addr_str[] = "192.168.1.2";
-  in_addr_t prod_addr_int;
-  in_addr_t cons_addr_int;
-  inet_pton(AF_INET, prod_addr_str, &prod_addr_int);
-  inet_pton(AF_INET, cons_addr_str, &cons_addr_int);
-
-  address_t prod_addr = ADDRESS4(prod_addr_int, 12345);
-  address_t cons_addr = ADDRESS4(cons_addr_int, 12345);
+  address_t prod_addr = ADDRESS4_LOCALHOST(12345);
+  address_t cons_addr = ADDRESS4_LOCALHOST(12346);
   address_t listener_addr = ADDRESS4_LOCALHOST(9596);
 
   listener_t* listener = listener_create(FACE_TYPE_UDP_LISTENER, &listener_addr,
@@ -153,12 +146,8 @@ TEST_F(StrategyLocalRemoteTest, InputRemoteOutputRemote) {
 }
 
 TEST_F(StrategyLocalRemoteTest, InputLocalOutputRemote) {
-  const char prod_addr_str[] = "192.168.1.1";
-  in_addr_t prod_addr_int;
-  inet_pton(AF_INET, prod_addr_str, &prod_addr_int);
-
-  address_t prod_addr = ADDRESS4(prod_addr_int, 12345);
-  address_t cons_addr = ADDRESS4_LOCALHOST(12345);
+  address_t prod_addr = ADDRESS4_LOCALHOST(12345);
+  address_t cons_addr = ADDRESS4_LOCALHOST(12346);
   address_t listener_addr = ADDRESS4_LOCALHOST(9596);
 
   listener_t* listener = listener_create(FACE_TYPE_UDP_LISTENER, &listener_addr,
@@ -197,13 +186,9 @@ TEST_F(StrategyLocalRemoteTest, InputLocalOutputRemote) {
 }
 
 TEST_F(StrategyLocalRemoteTest, InputRemoteOutputLocal) {
-  const char cons_addr_str[] = "192.168.1.2";
-  in_addr_t cons_addr_int;
-  inet_pton(AF_INET, cons_addr_str, &cons_addr_int);
-
-  address_t cons_addr = ADDRESS4(cons_addr_int, 12345);
-  address_t prod_addr = ADDRESS4_LOCALHOST(12345);
-  address_t listener_addr = ADDRESS4_LOCALHOST(9596);
+  address_t cons_addr = ADDRESS4_LOCALHOST(12345);
+  address_t prod_addr = ADDRESS4_LOCALHOST(12346);
+  address_t listener_addr = ADDRESS4_LOCALHOST(9695);
 
   listener_t* listener = listener_create(FACE_TYPE_UDP_LISTENER, &listener_addr,
                                          "lo", "lo_udp4", fwd_);
@@ -226,9 +211,11 @@ TEST_F(StrategyLocalRemoteTest, InputRemoteOutputLocal) {
 
   conn = connection_table_get_by_id(forwarder_get_connection_table(fwd_),
                                     prod_conn_id);
+  ASSERT_TRUE(conn != NULL);
   conn->local = true;
   conn = connection_table_get_by_id(forwarder_get_connection_table(fwd_),
                                     cons_conn_id);
+  ASSERT_TRUE(conn != NULL);
   conn->local = false;
 
   msgbuf_.connection_id = cons_conn_id;