Merge pull request #10223 from kb2ma/nanocoap/string_opt_first_char

net/nanocoap: fix string option separator write handling

sys/net/application_layer/nanocoap/nanocoap.c

@@ -653,10 +653,15 @@ size_t coap_opt_put_string(uint8_t *buf, uint16_t lastonum, uint16_t optnum,
 
     while (len) {
         size_t part_len;
-        uripos++;
+        if (*uripos == separator) {
+            uripos++;
+        }
         uint8_t *part_start = (uint8_t *)uripos;
 
-        while (len--) {
+        while (len) {
+            /* must decrement separately from while loop test to ensure
+             * the value remains non-negative */
+            len--;
             if ((*uripos == separator) || (*uripos == '\0')) {
                 break;
             }
@@ -710,10 +715,15 @@ ssize_t coap_opt_add_string(coap_pkt_t *pkt, uint16_t optnum, const char *string
 
     while (unread_len) {
         size_t part_len;
-        uripos++;
+        if (*uripos == separator) {
+            uripos++;
+        }
         uint8_t *part_start = (uint8_t *)uripos;
 
-        while (unread_len--) {
+        while (unread_len) {
+            /* must decrement separately from while loop test to ensure
+             * the value remains non-negative */
+            unread_len--;
             if ((*uripos == separator) || (*uripos == '\0')) {
                 break;
             }

tests/unittests/tests-nanocoap/tests-nanocoap.c

@@ -254,6 +254,81 @@ static void test_nanocoap__get_path_too_long(void)
     TEST_ASSERT_EQUAL_INT(-ENOSPC, get_len);
 }
 
+/*
+ * Builds on get_req test, to test Uri-Query option.
+ */
+static void test_nanocoap__get_query(void)
+{
+    uint8_t buf[_BUF_SIZE];
+    coap_pkt_t pkt;
+    uint16_t msgid = 0xABCD;
+    uint8_t token[2] = {0xDA, 0xEC};
+    char path[] = "/time";
+    size_t path_opt_len = 5;
+    char qs[] = "ab=cde";
+    size_t query_opt_len = 7;
+
+    size_t len = coap_build_hdr((coap_hdr_t *)&buf[0], COAP_TYPE_NON,
+                                &token[0], 2, COAP_METHOD_GET, msgid);
+
+    coap_pkt_init(&pkt, &buf[0], sizeof(buf), len);
+
+    len = coap_opt_add_string(&pkt, COAP_OPT_URI_PATH, &path[0], '/');
+    TEST_ASSERT_EQUAL_INT(path_opt_len, len);
+
+    uint8_t *query_pos = &pkt.payload[0];
+    len = coap_opt_add_string(&pkt, COAP_OPT_URI_QUERY, &qs[0], '&');
+    TEST_ASSERT_EQUAL_INT(query_opt_len, len);
+
+    char uri[10] = {0};
+    coap_get_uri_path(&pkt, (uint8_t *)&uri[0]);
+    TEST_ASSERT_EQUAL_STRING((char *)path, (char *)uri);
+
+    char query[10] = {0};
+    coap_get_uri_query(&pkt, (uint8_t *)&query[0]);
+    /* skip initial '&' from coap_get_uri_query() */
+    TEST_ASSERT_EQUAL_STRING((char *)qs, &query[1]);
+
+    /* overwrite query to test buffer-based put */
+    coap_opt_put_uri_query(query_pos, COAP_OPT_URI_PATH, qs);
+    coap_get_uri_query(&pkt, (uint8_t *)&query[0]);
+    /* skip initial '&' from coap_get_uri_query() */
+    TEST_ASSERT_EQUAL_STRING((char *)qs, &query[1]);
+}
+
+/*
+ * Builds on get_query test, to test multiple Uri-Query options.
+ */
+static void test_nanocoap__get_multi_query(void)
+{
+    uint8_t buf[_BUF_SIZE];
+    coap_pkt_t pkt;
+    uint16_t msgid = 0xABCD;
+    uint8_t token[2] = {0xDA, 0xEC};
+    char qs[] = "ab=cde&f=gh";
+    size_t query_opt_len = 13;    /* first opt header is 2 bytes long */
+
+    size_t len = coap_build_hdr((coap_hdr_t *)&buf[0], COAP_TYPE_NON,
+                                &token[0], 2, COAP_METHOD_GET, msgid);
+
+    coap_pkt_init(&pkt, &buf[0], sizeof(buf), len);
+
+    uint8_t *query_pos = &pkt.payload[0];
+    len = coap_opt_add_string(&pkt, COAP_OPT_URI_QUERY, &qs[0], '&');
+    TEST_ASSERT_EQUAL_INT(query_opt_len, len);
+
+    char query[20] = {0};
+    coap_get_uri_query(&pkt, (uint8_t *)&query[0]);
+    /* skip initial '&' from coap_get_uri_query() */
+    TEST_ASSERT_EQUAL_STRING((char *)qs, &query[1]);
+
+    /* overwrite query to test buffer-based put */
+    coap_opt_put_uri_query(query_pos, COAP_OPT_URI_PATH, qs);
+    coap_get_uri_query(&pkt, (uint8_t *)&query[0]);
+    /* skip initial '&' from coap_get_uri_query() */
+    TEST_ASSERT_EQUAL_STRING((char *)qs, &query[1]);
+}
+
 /*
  * Helper for server_get tests below.
  * GET Request for nanocoap server example /riot/value resource.
@@ -369,6 +444,8 @@ Test *tests_nanocoap_tests(void)
         new_TestFixture(test_nanocoap__get_root_path),
         new_TestFixture(test_nanocoap__get_max_path),
         new_TestFixture(test_nanocoap__get_path_too_long),
+        new_TestFixture(test_nanocoap__get_query),
+        new_TestFixture(test_nanocoap__get_multi_query),
         new_TestFixture(test_nanocoap__server_get_req),
         new_TestFixture(test_nanocoap__server_reply_simple),
         new_TestFixture(test_nanocoap__server_get_req_con),