Merge pull request #18096 from Teufelchen1/feat/port_parsing

sys/uri_parser: Adding the port as uint16_t

pkg/wakaama/contrib/lwm2m_client_connection.c

@@ -252,7 +252,7 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id,
     lwm2m_client_connection_t *conn = NULL;
     char uri[MAX_URI_LENGTH];
     size_t uri_len = ARRAY_SIZE(uri);
-    const char *port;
+    uint16_t port;
     bool is_bootstrap;
 
     DEBUG("Creating connection\n");
@@ -287,19 +287,19 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id,
     }
 
     /* if no port specified, use the default server or BS-server ports */
-    if (!parsed_uri.port) {
+    if (parsed_uri.port == 0) {
         if (is_bootstrap) {
-            port = CONFIG_LWM2M_BSSERVER_PORT;
+            port = atoi(CONFIG_LWM2M_BSSERVER_PORT);
         }
         else {
-            port = CONFIG_LWM2M_STANDARD_PORT;
+            port = atoi(CONFIG_LWM2M_STANDARD_PORT);
         }
     }
     else {
         port = parsed_uri.port;
     }
 
-    DEBUG("[_connection_create] Creating connection to Host: %.*s, Port: %s\n",
+    DEBUG("[_connection_create] Creating connection to Host: %.*s, Port: %u\n",
           parsed_uri.ipv6addr_len, parsed_uri.ipv6addr, port);
 
     /* allocate new connection */
@@ -313,7 +313,7 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id,
     /* configure to any IPv6 */
     conn->remote.family = AF_INET6;
     conn->remote.netif = SOCK_ADDR_ANY_NETIF;
-    conn->remote.port = atoi(port);
+    conn->remote.port = port;
 
     if (!ipv6_addr_from_buf((ipv6_addr_t *)&conn->remote.addr.ipv6, parsed_uri.ipv6addr,
                             parsed_uri.ipv6addr_len)) {

sys/include/uri_parser.h

@@ -67,15 +67,16 @@ typedef struct {
      */
     const char *zoneid;
 
-    const char *port;               /**< port */
+    const char *port_str;           /**< port as str */
     const char *path;               /**< path */
     const char *query;              /**< query */
+    uint16_t port;                  /**< port as uint16_t */
     uint16_t scheme_len;            /**< length of @ref scheme */
     uint16_t userinfo_len;          /**< length of @ref userinfo */
     uint16_t host_len;              /**< length of @ref host */
     uint16_t ipv6addr_len;          /**< length of @ref ipv6addr */
     uint16_t zoneid_len;            /**< length of @ref zoneid */
-    uint16_t port_len;              /**< length of @ref port */
+    uint16_t port_str_len;          /**< length of @ref port_str */
     uint16_t path_len;              /**< length of @ref path */
     uint16_t query_len;             /**< length of @ref query */
 } uri_parser_result_t;

sys/net/application_layer/gcoap/dns.c

@@ -466,7 +466,7 @@ static int _set_remote(const uri_parser_result_t *uri_comp,
               _uri);
         return -EHOSTUNREACH;
     }
-    if (uri_comp->port == NULL) {
+    if (uri_comp->port == 0) {
         if (strncmp(_uri, "coap:", sizeof("coap:") - 1) == 0) {
             remote->port = CONFIG_GCOAP_PORT;
         }
@@ -475,15 +475,7 @@ static int _set_remote(const uri_parser_result_t *uri_comp,
         }
     }
     else {
-        char port_str[uri_comp->port_len + 1];
+        remote->port = uri_comp->port;
-
-        strncpy(port_str, uri_comp->port, uri_comp->port_len);
-        port_str[uri_comp->port_len] = '\0';
-        remote->port = atoi(port_str);
-        if (remote->port == 0U) {
-            DEBUG("gcoap_dns: invalid port %s\n", port_str);
-            return -EINVAL;
-        }
     }
     return 0;
 }

sys/net/application_layer/gcoap/forward_proxy.c

@@ -174,16 +174,8 @@ static bool _parse_endpoint(sock_udp_ep_t *remote,
     }
     memcpy(&remote->addr.ipv6[0], &addr.u8[0], sizeof(addr.u8));
 
-    if (urip->port_len) {
+    if (urip->port != 0) {
-        /* copy port string into scratch for atoi */
+        remote->port = urip->port;
-        memcpy(scratch, urip->port, urip->port_len);
-        scratch[urip->port_len] = '\0';
-
-        remote->port = atoi(scratch);
-
-        if (remote->port == 0) {
-          return false;
-        }
     }
     else {
         remote->port = COAP_PORT;

sys/uri_parser/uri_parser.c

@@ -19,9 +19,12 @@
  */
 
 #include <assert.h>
+#include <stdlib.h>
 
 #include "uri_parser.h"
 
+#define MAX_PORT_STR_LEN    (5)
+
 #define ENABLE_DEBUG 0
 #include "debug.h"
 
@@ -93,6 +96,8 @@ bool _consume_port(uri_parser_result_t *result, const char *ipv6_end,
 {
     /* check for port after host part */
     const char *port_begin = NULL;
+    uint16_t port_str_len = 0;
+
     /* repeat until last ':' in authority section */
     /* if ipv6 address, check after ipv6 end marker */
     const char *p = (ipv6_end ? ipv6_end : result->host);
@@ -103,22 +108,46 @@ bool _consume_port(uri_parser_result_t *result, const char *ipv6_end,
 
     /* check if match */
     if (port_begin && (port_begin[0] == ':')) {
-        /* port should be at least one character, => + 1 */
+        port_begin++;
-        if (port_begin + 1 == authority_end) {
-            return false;
-        }
-        result->port = port_begin + 1;
-        result->port_len = authority_end - result->port;
 
-        /* ports can't exceed 5 characters */
+        /* port should be at least one character long */
-        if (result->port_len > 5){
+        if (port_begin == authority_end) {
-            result->port = NULL;
-            result->port_len = 0;
             return false;
         }
 
+        port_str_len = authority_end - port_begin;
+
+        /* Verify that the port number is up to 5 (random) chars in size */
+        if (port_str_len > MAX_PORT_STR_LEN) {
+            return false;
+        }
+
+        /* Verify that all characters of the port are numerical */
+        for (unsigned int i = 0; i < port_str_len; ++i) {
+            if (!((port_begin[i] >= '0') && (port_begin[i] <= '9'))) {
+                return false;
+            }
+        }
+
+        /* Verify that the next character, after the port, is an invalid
+         * character for the atol function. Preventing it from reading out-
+         * side of the port section */
+        if ((authority_end[0] >= '0') && (authority_end[0] <= '9')) {
+            return false;
+        }
+
+        /* Verify that the port is smaller or equal to UINT16_MAX. */
+        uint32_t port = atol(port_begin);
+        if (port > UINT16_MAX) {
+            return false;
+        }
+
+        result->port = (uint16_t)port;
+        result->port_str = port_begin;
+        result->port_str_len = port_str_len;
+
         /* cut host part before port and ':' */
-        result->host_len -= result->port_len + 1;
+        result->host_len -= result->port_str_len + 1;
     }
 
     return true;
@@ -300,10 +329,16 @@ int uri_parser_process(uri_parser_result_t *result, const char *uri,
     memset(result, 0, sizeof(*result));
 
     if (uri_parser_is_absolute(uri, uri_len)) {
-        return _parse_absolute(result, uri, uri + uri_len);
+        if (_parse_absolute(result, uri, uri + uri_len) != 0) {
+            memset(result, 0, sizeof(*result));
+            return -1;
+        }
     }
     else {
-        return _parse_relative(result, uri, uri + uri_len);
+        if (_parse_relative(result, uri, uri + uri_len) != 0) {
+            memset(result, 0, sizeof(*result));
+            return -1;
+        }
     }
 
     return 0;

tests/unittests/tests-uri_parser/tests-uri_parser.c

@@ -21,12 +21,23 @@
 #include "unittests-constants.h"
 #include "tests-uri_parser.h"
 
-#define VEC(u, f, s, us, h, v6a, z, po, pa, q, e)                           \
+#define VEC(u, f, s, us, h, v6a, z, po_str, po, pa, q, e)                   \
     { .uri = u, .full_uri = f, .scheme = s, .userinfo = us, .host = h,      \
-      .ipv6addr = v6a, .zoneid = z, .port = po, .path = pa,                 \
+      .ipv6addr = v6a, .zoneid = z, .port_str = po_str, .port = po,         \
-      .query = q, .expected = e}
+      .path = pa, .query = q, .expected = e }
 
-#define VEC_CHECK(comp, i, vec_msg)                                         \
+#define VEC_CHECK_INT(comp, i, vec_msg)                                     \
+    do {                                                                    \
+        vec_msg[0] = '\0';                                                  \
+        stdimpl_strcat(vec_msg, "Unexpected " # comp " member ");           \
+        stdimpl_lltoa(validate_uris[i].comp, &vec_msg[strlen(vec_msg)], 10);\
+        stdimpl_strcat(vec_msg, " for \"");                                 \
+        stdimpl_strcat(vec_msg, validate_uris[i].uri);                      \
+        stdimpl_strcat(vec_msg, "\"");                                      \
+        TEST_ASSERT_MESSAGE(validate_uris[i].comp == ures.comp, vec_msg);   \
+    } while (0)
+
+#define VEC_CHECK_STR(comp, i, vec_msg)                                     \
     do {                                                                    \
         if (ures.comp == NULL) {                                            \
             TEST_ASSERT(validate_uris[i].comp[0] == '\0');                  \
@@ -58,7 +69,8 @@ typedef struct {
     char host[24];
     char ipv6addr[16];
     char zoneid[8];
-    char port[32];
+    char port_str[32];
+    uint16_t port;
     char path[48];
     char query[32];
     int expected;
@@ -66,7 +78,7 @@ typedef struct {
 
 /*
   VEC(uri_to_parse,
-      scheme, userinfo, host, port,
+      scheme, userinfo, host, port_str, port,
       path, query, expected return value)
 */
 static const validate_t validate_uris[] = {
@@ -84,8 +96,10 @@ static const validate_t validate_uris[] = {
         "fe80:db8::1",
         /* parsed zoneid */
         "tap0",
-        /* parsed port */
+        /* parsed port_str */
         "5683",
+        /* parsed port */
+        5683,
         /* parsed path */
         "/.well-known/core",
         /* parsed query */
@@ -100,6 +114,7 @@ static const validate_t validate_uris[] = {
         "fe80:db8::1",
         "",
         "5683",
+        5683,
         "/.well-known/core",
         "v=1",
         -1),
@@ -111,6 +126,7 @@ static const validate_t validate_uris[] = {
         "fe80::1",
         "",
         "",
+        0,
         "/foo%20bar",
         "",
         0),
@@ -122,9 +138,58 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/.well-known/core",
         "v=1",
         0),
+    VEC("coap://R@[2001:db8::1]:5/v=1",
+        true,
+        "coap",
+        "R",
+        "[2001:db8::1]",
+        "2001:db8::1",
+        "",
+        "5",
+        5,
+        "/v=1",
+        "",
+        0),
+    VEC("coap://R@[2001:db8::1]:5/:v=1",
+        true,
+        "coap",
+        "R",
+        "[2001:db8::1]",
+        "2001:db8::1",
+        "",
+        "5",
+        5,
+        "/:v=1",
+        "",
+        0),
+    VEC("cap://R@[2001:db8::1]:5/?v=1",
+        true,
+        "cap",
+        "R",
+        "[2001:db8::1]",
+        "2001:db8::1",
+        "",
+        "5",
+        5,
+        "/",
+        "v=1",
+        0),
+    VEC("oap://Y2001:db8::1]:5/av=1",
+        true,
+        "oap",
+        "",
+        "Y2001:db8::1]",
+        "",
+        "",
+        "5",
+        5,
+        "/av=1",
+        "",
+        0),
     VEC("coap://R@[2001:db8::1]:5own/v=1",
         true,
         "coap",
@@ -133,9 +198,10 @@ static const validate_t validate_uris[] = {
         "2001:db8::1",
         "",
         "5own",
+        5,
         "/v=1",
         "",
-        0),
+        -1),
     VEC("coap://R@[2001:db8::1]:5own/:v=1",
         true,
         "coap",
@@ -144,9 +210,10 @@ static const validate_t validate_uris[] = {
         "2001:db8::1",
         "",
         "5own",
+        5,
         "/:v=1",
         "",
-        0),
+        -1),
     VEC("cap://R@[2001:db8::1]:5own/?v=1",
         true,
         "cap",
@@ -155,9 +222,10 @@ static const validate_t validate_uris[] = {
         "2001:db8::1",
         "",
         "5own",
+        5,
         "/",
         "v=1",
-        0),
+        -1),
     VEC("oap://Y2001:db8::1]:5own/av=1",
         true,
         "oap",
@@ -166,9 +234,10 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "5own",
+        5,
         "/av=1",
         "",
-        0),
+        -1),
     VEC("//Rb[ʰ00J:d/5v=0",
         false,
         "",
@@ -177,6 +246,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "//Rb[ʰ00J:d/5v=0",
         "",
         0),
@@ -188,6 +258,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "",
         "",
         -1),
@@ -199,6 +270,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/R@[2008::1]:5own//R@[2008::1]:5own/",
         "v=1",
         0),
@@ -210,6 +282,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/RZ[2001[8:01[8::1]:5o:1]:5oTMv=1",
         "",
         0),
@@ -221,6 +294,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "////////////////7///v=1",
         "",
         0),
@@ -232,6 +306,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "coa[:////[2001:db5ow:5own/Ov=1",
         "",
         0),
@@ -243,6 +318,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "+1-816-555-1212",
         "",
         0),
@@ -254,6 +330,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "+15105550101,+15105550102",
         "body=hello%20there",
         0),
@@ -265,6 +342,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "a",
         "",
         0),
@@ -276,6 +354,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "test@example.com",
         "",
         0),
@@ -287,6 +366,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/rfc/rfc1808.txt",
         "",
         0),
@@ -298,6 +378,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/rfc/rfc2396.txt",
         "",
         0),
@@ -309,6 +390,7 @@ static const validate_t validate_uris[] = {
         "2001:db8::7",
         "",
         "",
+        0,
         "/c=GB",
         "objectClass?one",
         0),
@@ -320,6 +402,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "John.Doe@example.com",
         "",
         0),
@@ -331,6 +414,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "comp.infosystems.www.servers.unix",
         "",
         0),
@@ -342,6 +426,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "+1-816-555-1212",
         "",
         0),
@@ -353,6 +438,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "80",
+        80,
         "/",
         "",
         0),
@@ -364,6 +450,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "oasis:names:specification:docbook:dtd:xml:4.1.2",
         "",
         0),
@@ -375,6 +462,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "",
         "",
         -1),
@@ -386,6 +474,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "/",
         "",
         0),
@@ -397,6 +486,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "./this:that",
         "",
         0),
@@ -408,6 +498,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "",
         "",
         0),
@@ -419,6 +510,7 @@ static const validate_t validate_uris[] = {
         "",
         "",
         "",
+        0,
         "",
         "",
         0),
@@ -435,14 +527,40 @@ static const validate_t validate_uris[] = {
         "",
         /* parsed zoneid */
         "",
-        /* parsed port */
+        /* parsed port_str */
         "",
+        /* parsed port */
+        0,
         /* parsed path */
         "",
         /* parsed query */
         "",
         /* expected return value */
         -1),
+    VEC("coap://example.com: 12/foo",
+        true,
+        "",
+        "",
+        "example.com",
+        "",
+        "",
+        "",
+        0,
+        "",
+        "",
+        -1),
+    VEC("coap://example.com:0x12/foo",
+        true,
+        "",
+        "",
+        "example.com",
+        "",
+        "",
+        "",
+        0,
+        "",
+        "",
+        -1),
 };
 
 static char _failure_msg[VEC_MSG_LEN];
@@ -456,14 +574,15 @@ static void test_uri_parser__validate(void)
                uri_parser_is_absolute_string(validate_uris[i].uri));
         TEST_ASSERT_EQUAL_INT(validate_uris[i].expected, res);
         if (res == 0) {
-            VEC_CHECK(scheme, i, _failure_msg);
+            VEC_CHECK_STR(scheme, i, _failure_msg);
-            VEC_CHECK(userinfo, i, _failure_msg);
+            VEC_CHECK_STR(userinfo, i, _failure_msg);
-            VEC_CHECK(host, i, _failure_msg);
+            VEC_CHECK_STR(host, i, _failure_msg);
-            VEC_CHECK(ipv6addr, i, _failure_msg);
+            VEC_CHECK_STR(ipv6addr, i, _failure_msg);
-            VEC_CHECK(zoneid, i, _failure_msg);
+            VEC_CHECK_STR(zoneid, i, _failure_msg);
-            VEC_CHECK(port, i, _failure_msg);
+            VEC_CHECK_STR(port_str, i, _failure_msg);
-            VEC_CHECK(path, i, _failure_msg);
+            VEC_CHECK_INT(port, i, _failure_msg);
-            VEC_CHECK(query, i, _failure_msg);
+            VEC_CHECK_STR(path, i, _failure_msg);
+            VEC_CHECK_STR(query, i, _failure_msg);
         }
     }
 }
@@ -480,14 +599,15 @@ static void test_uri_parser__unterminated_string(void)
     int res = uri_parser_process(&ures, uri, strlen(validate_uris[0].uri));
 
     TEST_ASSERT_EQUAL_INT(0, res);
-    VEC_CHECK(scheme, 0, _failure_msg);
+    VEC_CHECK_STR(scheme, 0, _failure_msg);
-    VEC_CHECK(userinfo, 0, _failure_msg);
+    VEC_CHECK_STR(userinfo, 0, _failure_msg);
-    VEC_CHECK(host, 0, _failure_msg);
+    VEC_CHECK_STR(host, 0, _failure_msg);
-    VEC_CHECK(ipv6addr, 0, _failure_msg);
+    VEC_CHECK_STR(ipv6addr, 0, _failure_msg);
-    VEC_CHECK(zoneid, 0, _failure_msg);
+    VEC_CHECK_STR(zoneid, 0, _failure_msg);
-    VEC_CHECK(port, 0, _failure_msg);
+    VEC_CHECK_STR(port_str, 0, _failure_msg);
-    VEC_CHECK(path, 0, _failure_msg);
+    VEC_CHECK_INT(port, 0, _failure_msg);
-    VEC_CHECK(query, 0, _failure_msg);
+    VEC_CHECK_STR(path, 0, _failure_msg);
+    VEC_CHECK_STR(query, 0, _failure_msg);
 }
 
 Test *tests_uri_parser_tests(void)