From fa5ab4eeb65373c171730c032e9ea81c01855761 Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Fri, 18 May 2018 01:36:38 -0400 Subject: [PATCH 1/6] net/gcoap: Add format option to nanocoap --- sys/include/net/nanocoap.h | 2 +- sys/net/application_layer/gcoap/gcoap.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sys/include/net/nanocoap.h b/sys/include/net/nanocoap.h index ab2b3502403e..5e3ffb4b9a78 100644 --- a/sys/include/net/nanocoap.h +++ b/sys/include/net/nanocoap.h @@ -175,7 +175,7 @@ extern "C" { /** * @brief Nanocoap-specific value to indicate no format specified */ -#define COAP_FORMAT_NONE (65535) +#define COAP_FORMAT_NONE (UINT16_MAX) /** * @name Nanocoap specific maximum values diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index ac0f77033787..fe7236e6f443 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -38,6 +38,12 @@ #define GCOAP_RESOURCE_WRONG_METHOD -1 #define GCOAP_RESOURCE_NO_PATH -2 +/* + * gcoap internal Content-Format option value. Not intended for use in a + * transmitted packet. Must be a 3-byte unsigned value. + */ +#define COAP_FORMAT_NO_PAYLOAD (UINT16_MAX + 1) + /* Internal functions */ static void *_event_loop(void *arg); static void _listen(sock_udp_t *sock); From f0b966d65daff7982d758bd0f727836a40f8125f Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Wed, 16 May 2018 13:09:40 -0400 Subject: [PATCH 2/6] net/gcoap: Use nanocoap options API Observe still uses coap_pkt_t attribute. --- sys/include/net/gcoap.h | 37 +++-- sys/net/application_layer/gcoap/gcoap.c | 179 +++++++----------------- 2 files changed, 76 insertions(+), 140 deletions(-) diff --git a/sys/include/net/gcoap.h b/sys/include/net/gcoap.h index 8ec4c3b7a04a..ffa1938e795c 100644 --- a/sys/include/net/gcoap.h +++ b/sys/include/net/gcoap.h @@ -253,26 +253,28 @@ extern "C" { #endif /** - * @brief Size of the buffer used to write options, other than Uri-Path, in a - * request + * @brief Reduce payload length by this value for a request created with + * gcoap_req_init() * - * Accommodates Content-Format and Uri-Queries + * Accommodates writing Content-Format option in gcoap_finish() */ -#define GCOAP_REQ_OPTIONS_BUF (40) +#define GCOAP_REQ_OPTIONS_BUF (4) /** - * @brief Size of the buffer used to write options in a response + * @brief Reduce payload length by this value for a respons created with + * gcoap_resp_init() * - * Accommodates Content-Format. + * Accommodates writing Content-Format option in gcoap_finish() */ -#define GCOAP_RESP_OPTIONS_BUF (8) +#define GCOAP_RESP_OPTIONS_BUF (4) /** - * @brief Size of the buffer used to write options in an Observe notification + * @brief Reduce payload length by this value for an observe notification + * created with gcoap_obs_init() * - * Accommodates Content-Format and Observe. + * Accommodates writing Content-Format option in gcoap_finish() */ -#define GCOAP_OBS_OPTIONS_BUF (8) +#define GCOAP_OBS_OPTIONS_BUF (4) /** * @brief Maximum number of requests awaiting a response @@ -506,6 +508,11 @@ void gcoap_register_listener(gcoap_listener_t *listener); /** * @brief Initializes a CoAP request PDU on a buffer. + + * @warning After you use this function, you may not add Options with option + * number less than COAP_OPT_URI_PATH. Otherwise, use the struct-based API + * described with @link net_nanocoap nanocoap @endlink to initialize the + * message. See the implementation of gcoap_req_init() itself as an example. * * @param[out] pdu Request metadata * @param[out] buf Buffer containing the PDU @@ -525,8 +532,14 @@ int gcoap_req_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, /** * @brief Finishes formatting a CoAP PDU after the payload has been written * - * Assumes the PDU has been initialized with gcoap_req_init() or - * gcoap_resp_init(). + * Assumes the PDU has been initialized with a gcoap_xxx_init() function, like + * gcoap_req_init(). + * + * @warning To use this function, you only may have added an Option with + * option number less than COAP_OPT_CONTENT_FORMAT. Otherwise, use the + * struct-based API described with @link net_nanocoap nanocoap. @endlink With + * this API, you specify the format with coap_opt_add_uint(), prepare for the + * payload with coap_opt_finish(), and then write the payload. * * @param[in,out] pdu Request metadata * @param[in] payload_len Length of the payload, or 0 if none diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index fe7236e6f443..192ecb71c2b5 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -38,20 +38,12 @@ #define GCOAP_RESOURCE_WRONG_METHOD -1 #define GCOAP_RESOURCE_NO_PATH -2 -/* - * gcoap internal Content-Format option value. Not intended for use in a - * transmitted packet. Must be a 3-byte unsigned value. - */ -#define COAP_FORMAT_NO_PAYLOAD (UINT16_MAX + 1) - /* Internal functions */ static void *_event_loop(void *arg); static void _listen(sock_udp_t *sock); static ssize_t _well_known_core_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, void *ctx); -static ssize_t _write_options(coap_pkt_t *pdu, uint8_t *buf, size_t len); static size_t _handle_req(coap_pkt_t *pdu, uint8_t *buf, size_t len, sock_udp_ep_t *remote); -static ssize_t _finish_pdu(coap_pkt_t *pdu, uint8_t *buf, size_t len); static void _expire_request(gcoap_request_memo_t *memo); static void _find_req_memo(gcoap_request_memo_t **memo_ptr, coap_pkt_t *pdu, const sock_udp_ep_t *remote); @@ -343,9 +335,6 @@ static size_t _handle_req(coap_pkt_t *pdu, uint8_t *buf, size_t len, memcpy(&memo->token[0], pdu->token, memo->token_len); } DEBUG("gcoap: Registered observer for: %s\n", memo->resource->path); - /* generate initial notification value */ - uint32_t now = xtimer_now_usec(); - pdu->observe_value = (now >> GCOAP_OBS_TICK_EXPONENT) & 0xFFFFFF; } } else if (coap_get_observe(pdu) == COAP_OBS_DEREGISTER) { @@ -429,29 +418,6 @@ static int _find_resource(coap_pkt_t *pdu, const coap_resource_t **resource_ptr, return ret; } -/* - * Finishes handling a PDU -- write options and reposition payload. - * - * Returns the size of the PDU within the buffer, or < 0 on error. - */ -static ssize_t _finish_pdu(coap_pkt_t *pdu, uint8_t *buf, size_t len) -{ - ssize_t hdr_len = _write_options(pdu, buf, len); - DEBUG("gcoap: header length: %i\n", (int)hdr_len); - - if (hdr_len > 0) { - /* move payload over unused space after options */ - if (pdu->payload_len) { - memmove(buf + hdr_len, pdu->payload, pdu->payload_len); - } - - return hdr_len + pdu->payload_len; - } - else { - return -1; /* generic failure code */ - } -} - /* * Finds the memo for an outstanding request within the _coap_state.open_reqs * array. Matches on remote endpoint and token. @@ -535,69 +501,6 @@ static ssize_t _well_known_core_handler(coap_pkt_t* pdu, uint8_t *buf, size_t le return gcoap_finish(pdu, (size_t)plen, COAP_FORMAT_LINK); } -/* - * Creates CoAP options and sets payload marker, if any. - * - * Returns length of header + options, or -EINVAL on illegal path. - */ -static ssize_t _write_options(coap_pkt_t *pdu, uint8_t *buf, size_t len) -{ - uint8_t last_optnum = 0; - (void)len; - - uint8_t *bufpos = buf + coap_get_total_hdr_len(pdu); /* position for write */ - - /* Observe for notification or registration response */ - if (coap_get_code_class(pdu) == COAP_CLASS_SUCCESS && coap_has_observe(pdu)) { - uint32_t nval = htonl(pdu->observe_value); - uint8_t *nbyte = (uint8_t *)&nval; - unsigned i; - /* find address of non-zero MSB; max 3 bytes */ - for (i = 1; i < 4; i++) { - if (*(nbyte+i) > 0) { - break; - } - } - bufpos += coap_put_option(bufpos, last_optnum, COAP_OPT_OBSERVE, - nbyte+i, 4-i); - last_optnum = COAP_OPT_OBSERVE; - } - - /* Uri-Path for request */ - if (coap_get_code_class(pdu) == COAP_CLASS_REQ) { - size_t url_len = strlen((char *)pdu->url); - if (url_len) { - if (pdu->url[0] != '/') { - DEBUG("gcoap: _write_options: path does not start with '/'\n"); - return -EINVAL; - } - bufpos += coap_opt_put_uri_path(bufpos, last_optnum, - (char *)pdu->url); - last_optnum = COAP_OPT_URI_PATH; - } - } - - /* Content-Format */ - if (pdu->content_type != COAP_FORMAT_NONE) { - bufpos += coap_put_option_ct(bufpos, last_optnum, pdu->content_type); - last_optnum = COAP_OPT_CONTENT_FORMAT; - } - - /* Uri-query for requests */ - if (coap_get_code_class(pdu) == COAP_CLASS_REQ) { - bufpos += coap_opt_put_uri_query(bufpos, last_optnum, - (char *)pdu->qs); - /* uncomment when further options are added below ... */ - /* last_optnum = COAP_OPT_URI_QUERY; */ - } - - /* write payload marker */ - if (pdu->payload_len) { - *bufpos++ = GCOAP_PAYLOAD_MARKER; - } - return bufpos - buf; -} - /* * Find registered observer for a remote address and port. * @@ -729,11 +632,7 @@ int gcoap_req_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, { assert((path != NULL) && (path[0] == '/')); - (void)len; - pdu->hdr = (coap_hdr_t *)buf; - memset(pdu->url, 0, NANOCOAP_URL_MAX); - memset(pdu->qs, 0, NANOCOAP_QS_MAX); /* generate token */ #if GCOAP_TOKENLEN @@ -754,15 +653,8 @@ int gcoap_req_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, #endif if (hdrlen > 0) { - /* Reserve some space between the header and payload to write options later */ - pdu->payload = buf + coap_get_total_hdr_len(pdu) + strlen(path) - + GCOAP_REQ_OPTIONS_BUF; - /* Payload length really zero at this point, but we set this to the available - * length in the buffer. Allows us to reconstruct buffer length later. */ - pdu->payload_len = len - (pdu->payload - buf); - pdu->content_type = COAP_FORMAT_NONE; - - memcpy(&pdu->url[0], path, strlen(path)); + coap_pkt_init(pdu, buf, len - GCOAP_REQ_OPTIONS_BUF, hdrlen); + coap_opt_add_string(pdu, COAP_OPT_URI_PATH, path, '/'); return 0; } else { @@ -771,14 +663,43 @@ int gcoap_req_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, } } +/* + * Assumes pdu.payload_len attribute was reduced in gcoap_xxx_init() to + * ensure enough space in PDU buffer to write Content-Format option and + * payload marker here. + */ ssize_t gcoap_finish(coap_pkt_t *pdu, size_t payload_len, unsigned format) { - /* reconstruct full PDU buffer length */ - size_t len = pdu->payload_len + (pdu->payload - (uint8_t *)pdu->hdr); + assert( !(pdu->options_len) || + !(payload_len) || + (format == COAP_FORMAT_NONE) || + (pdu->options[pdu->options_len-1].opt_num < COAP_OPT_CONTENT_FORMAT)); - pdu->content_type = format; - pdu->payload_len = payload_len; - return _finish_pdu(pdu, (uint8_t *)pdu->hdr, len); + if (payload_len) { + /* determine Content-Format option length */ + unsigned format_optlen = 1; + if (format == COAP_FORMAT_NONE) { + format_optlen = 0; + } + else if (format > 255) { + format_optlen = 3; + } + else if (format > 0) { + format_optlen = 2; + } + + /* move payload to accommodate option and payload marker */ + memmove(pdu->payload+format_optlen+1, pdu->payload, payload_len); + + if (format_optlen) { + coap_opt_add_uint(pdu, COAP_OPT_CONTENT_FORMAT, format); + } + *pdu->payload++ = 0xFF; + } + /* must write option before updating PDU with actual length */ + pdu->payload_len = payload_len; + + return pdu->payload_len + (pdu->payload - (uint8_t *)pdu->hdr); } size_t gcoap_req_send(const uint8_t *buf, size_t len, const ipv6_addr_t *addr, @@ -912,12 +833,18 @@ int gcoap_resp_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, unsigned code) } coap_hdr_set_code(pdu->hdr, code); - /* Reserve some space between the header and payload to write options later */ - pdu->payload = buf + coap_get_total_hdr_len(pdu) + GCOAP_RESP_OPTIONS_BUF; - /* Payload length really zero at this point, but we set this to the available - * length in the buffer. Allows us to reconstruct buffer length later. */ - pdu->payload_len = len - (pdu->payload - buf); - pdu->content_type = COAP_FORMAT_NONE; + unsigned header_len = coap_get_total_hdr_len(pdu); + + pdu->options_len = 0; + pdu->payload = buf + header_len; + pdu->payload_len = len - header_len - GCOAP_RESP_OPTIONS_BUF; + + if (coap_get_observe(pdu) == COAP_OBS_REGISTER) { + /* generate initial notification value */ + uint32_t now = xtimer_now_usec(); + pdu->observe_value = (now >> GCOAP_OBS_TICK_EXPONENT) & 0xFFFFFF; + coap_opt_add_uint(pdu, COAP_OPT_OBSERVE, pdu->observe_value); + } return 0; } @@ -939,15 +866,11 @@ int gcoap_obs_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, memo->token_len, COAP_CODE_CONTENT, msgid); if (hdrlen > 0) { + coap_pkt_init(pdu, buf, len - GCOAP_OBS_OPTIONS_BUF, hdrlen); + uint32_t now = xtimer_now_usec(); pdu->observe_value = (now >> GCOAP_OBS_TICK_EXPONENT) & 0xFFFFFF; - - /* Reserve some space between the header and payload to write options later */ - pdu->payload = buf + coap_get_total_hdr_len(pdu) + GCOAP_OBS_OPTIONS_BUF; - /* Payload length really zero at this point, but we set this to the available - * length in the buffer. Allows us to reconstruct buffer length later. */ - pdu->payload_len = len - (pdu->payload - buf); - pdu->content_type = COAP_FORMAT_NONE; + coap_opt_add_uint(pdu, COAP_OPT_OBSERVE, pdu->observe_value); return GCOAP_OBS_INIT_OK; } From 034c78d51cb06d9d9815f2af95be442f5b2898af Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Thu, 22 Nov 2018 18:08:50 -0500 Subject: [PATCH 3/6] net/gcoap: move macros to implementation file --- sys/include/net/gcoap.h | 24 ------------------------ sys/net/application_layer/gcoap/gcoap.c | 9 +++++++++ 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/sys/include/net/gcoap.h b/sys/include/net/gcoap.h index ffa1938e795c..92b221fdec15 100644 --- a/sys/include/net/gcoap.h +++ b/sys/include/net/gcoap.h @@ -252,30 +252,6 @@ extern "C" { #define GCOAP_PDU_BUF_SIZE (128) #endif -/** - * @brief Reduce payload length by this value for a request created with - * gcoap_req_init() - * - * Accommodates writing Content-Format option in gcoap_finish() - */ -#define GCOAP_REQ_OPTIONS_BUF (4) - -/** - * @brief Reduce payload length by this value for a respons created with - * gcoap_resp_init() - * - * Accommodates writing Content-Format option in gcoap_finish() - */ -#define GCOAP_RESP_OPTIONS_BUF (4) - -/** - * @brief Reduce payload length by this value for an observe notification - * created with gcoap_obs_init() - * - * Accommodates writing Content-Format option in gcoap_finish() - */ -#define GCOAP_OBS_OPTIONS_BUF (4) - /** * @brief Maximum number of requests awaiting a response */ diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index 192ecb71c2b5..c4d64acd60df 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -38,6 +38,15 @@ #define GCOAP_RESOURCE_WRONG_METHOD -1 #define GCOAP_RESOURCE_NO_PATH -2 +/* + * Reduce payload length by this value for a request created with + * gcoap_req_init(), gcoap_resp_init(), and gcoap_obs_init(), respectively. + * Accommodates writing Content-Format option in gcoap_finish(). + */ +#define GCOAP_REQ_OPTIONS_BUF (4) +#define GCOAP_RESP_OPTIONS_BUF (4) +#define GCOAP_OBS_OPTIONS_BUF (4) + /* Internal functions */ static void *_event_loop(void *arg); static void _listen(sock_udp_t *sock); From f8e5b3dee3a2774d2ce5a4e21cf256f3c19cc586 Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Mon, 21 May 2018 12:59:57 -0400 Subject: [PATCH 4/6] net/gcoap: update tests for nanocoap options API --- tests/unittests/tests-gcoap/tests-gcoap.c | 62 ++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/unittests/tests-gcoap/tests-gcoap.c b/tests/unittests/tests-gcoap/tests-gcoap.c index fd20ae4b9939..e76dc77a4def 100644 --- a/tests/unittests/tests-gcoap/tests-gcoap.c +++ b/tests/unittests/tests-gcoap/tests-gcoap.c @@ -83,7 +83,10 @@ static void test_gcoap__client_get_req(void) TEST_ASSERT_EQUAL_INT(GCOAP_TOKENLEN, coap_get_token_len(&pdu)); TEST_ASSERT_EQUAL_INT(hdr_fixed_len + GCOAP_TOKENLEN, coap_get_total_hdr_len(&pdu)); TEST_ASSERT_EQUAL_INT(COAP_TYPE_NON, coap_get_type(&pdu)); - TEST_ASSERT_EQUAL_STRING(&path[0], (char *)&pdu.url[0]); + + char uri[10] = {0}; + coap_get_uri(&pdu, (uint8_t *)&uri[0]); + TEST_ASSERT_EQUAL_STRING(&path[0], &uri[0]); TEST_ASSERT_EQUAL_INT(0, pdu.payload_len); TEST_ASSERT_EQUAL_INT(sizeof(pdu_data), len); } @@ -124,6 +127,61 @@ static void test_gcoap__client_get_resp(void) } } +/* + * Client PUT request success case. Test request generation. + * Set value of /riot/value resource to 1 from nanocoap server example. + */ +static void test_gcoap__client_put_req(void) +{ + uint8_t buf[GCOAP_PDU_BUF_SIZE]; + coap_pkt_t pdu; + size_t len; + char path[] = "/riot/value"; + char payload[] = "1"; + + gcoap_req_init(&pdu, buf, GCOAP_PDU_BUF_SIZE, COAP_METHOD_PUT, path); + memcpy(pdu.payload, payload, 1); + len = gcoap_finish(&pdu, 1, COAP_FORMAT_TEXT); + + coap_parse(&pdu, buf, len); + + TEST_ASSERT_EQUAL_INT(COAP_METHOD_PUT, coap_get_code(&pdu)); + TEST_ASSERT_EQUAL_INT(1, pdu.payload_len); + TEST_ASSERT_EQUAL_INT('1', (char)*pdu.payload); +} + +/* + * Builds on get_req test, to test gcoap_add_qstring() to add Uri-Query + * options. + */ +static void test_gcoap__client_get_query(void) +{ + uint8_t buf[GCOAP_PDU_BUF_SIZE]; + coap_pkt_t pdu; + char path[] = "/time"; + char key1[] = "ab"; + char val1[] = "cde"; + char key2[] = "f"; + char expected[] = "ab=cde&f"; + int optlen; + + gcoap_req_init(&pdu, buf, GCOAP_PDU_BUF_SIZE, COAP_METHOD_GET, path); + + optlen = gcoap_add_qstring(&pdu, key1, val1); + TEST_ASSERT_EQUAL_INT(7, optlen); + optlen = gcoap_add_qstring(&pdu, key2, NULL); + TEST_ASSERT_EQUAL_INT(2, optlen); + + size_t len = gcoap_finish(&pdu, 0, COAP_FORMAT_NONE); + + coap_parse(&pdu, buf, len); + + char query[20] = {0}; + coap_get_uri_query(&pdu, (uint8_t *)&query[0]); + /* skip initial '&' from coap_get_uri_query() */ + TEST_ASSERT_EQUAL_STRING(&expected[0], &query[1]); +} + /* * Helper for server_get tests below. * Request from libcoap example for gcoap_cli /cli/stats resource @@ -283,6 +341,8 @@ Test *tests_gcoap_tests(void) EMB_UNIT_TESTFIXTURES(fixtures) { new_TestFixture(test_gcoap__client_get_req), new_TestFixture(test_gcoap__client_get_resp), + new_TestFixture(test_gcoap__client_put_req), + new_TestFixture(test_gcoap__client_get_query), new_TestFixture(test_gcoap__server_get_req), new_TestFixture(test_gcoap__server_get_resp), new_TestFixture(test_gcoap__server_con_req), From 424a01ddc182609bfd2613f2440ffd3bbcd30524 Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Mon, 21 May 2018 13:01:06 -0400 Subject: [PATCH 5/6] net/gcoap: remove gcoap attributes from coap_pkt_t --- examples/gcoap/gcoap_cli.c | 5 +-- sys/include/net/nanocoap.h | 3 -- sys/net/application_layer/gcoap/gcoap.c | 31 +++++++++++-------- sys/net/application_layer/nanocoap/nanocoap.c | 7 ----- tests/unittests/tests-gcoap/tests-gcoap.c | 9 ++++-- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/examples/gcoap/gcoap_cli.c b/examples/gcoap/gcoap_cli.c index a6ef88bacec7..95c405e89ec4 100644 --- a/examples/gcoap/gcoap_cli.c +++ b/examples/gcoap/gcoap_cli.c @@ -72,8 +72,9 @@ static void _resp_handler(unsigned req_state, coap_pkt_t* pdu, coap_get_code_class(pdu), coap_get_code_detail(pdu)); if (pdu->payload_len) { - if (pdu->content_type == COAP_FORMAT_TEXT - || pdu->content_type == COAP_FORMAT_LINK + unsigned content_type = coap_get_content_type(pdu); + if (content_type == COAP_FORMAT_TEXT + || content_type == COAP_FORMAT_LINK || coap_get_code_class(pdu) == COAP_CLASS_CLIENT_FAILURE || coap_get_code_class(pdu) == COAP_CLASS_SERVER_FAILURE) { /* Expecting diagnostic payload in failure cases */ diff --git a/sys/include/net/nanocoap.h b/sys/include/net/nanocoap.h index 5e3ffb4b9a78..a6ed28efe5ac 100644 --- a/sys/include/net/nanocoap.h +++ b/sys/include/net/nanocoap.h @@ -232,9 +232,6 @@ typedef struct { uint16_t options_len; /**< length of options array */ coap_optpos_t options[NANOCOAP_NOPTS_MAX]; /**< option offset array */ #ifdef MODULE_GCOAP - uint8_t url[NANOCOAP_URI_MAX]; /**< parsed request URL */ - uint8_t qs[NANOCOAP_QS_MAX]; /**< parsed query string */ - uint16_t content_type; /**< content type */ uint32_t observe_value; /**< observe value */ #endif } coap_pkt_t; diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index c4d64acd60df..79c26dc87b48 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -395,6 +395,12 @@ static int _find_resource(coap_pkt_t *pdu, const coap_resource_t **resource_ptr, /* Find path for CoAP msg among listener resources and execute callback. */ gcoap_listener_t *listener = _coap_state.listeners; + + uint8_t uri[NANOCOAP_URI_MAX]; + if (coap_get_uri_path(pdu, uri) <= 0) { + return GCOAP_RESOURCE_NO_PATH; + } + while (listener) { const coap_resource_t *resource = listener->resources; for (size_t i = 0; i < listener->resources_len; i++) { @@ -402,7 +408,7 @@ static int _find_resource(coap_pkt_t *pdu, const coap_resource_t **resource_ptr, resource++; } - int res = strcmp((char *)&pdu->url[0], resource->path); + int res = strcmp((char *)&uri[0], resource->path); if (res > 0) { continue; } @@ -961,26 +967,25 @@ int gcoap_get_resource_list(void *buf, size_t maxlen, uint8_t cf) int gcoap_add_qstring(coap_pkt_t *pdu, const char *key, const char *val) { - size_t qs_len = strlen((char *)pdu->qs); - size_t key_len = strlen(key); + char qs[NANOCOAP_QS_MAX]; + size_t len = strlen(key); size_t val_len = (val) ? (strlen(val) + 1) : 0; - /* make sure if url_len + the new query string fit into the url buffer */ - if ((qs_len + key_len + val_len + 2) >= NANOCOAP_QS_MAX) { + /* test if the query string fits, account for the zero termination */ + if ((len + val_len + 1) >= NANOCOAP_QS_MAX) { return -1; } - pdu->qs[qs_len++] = '&'; - memcpy(&pdu->qs[qs_len], key, key_len); - qs_len += key_len; + memcpy(&qs[0], key, len); if (val) { - pdu->qs[qs_len++] = '='; - memcpy(&pdu->qs[qs_len], val, val_len); - qs_len += val_len; + qs[len] = '='; + /* the `=` character was already counted in `val_len`, so subtract it here */ + memcpy(&qs[len + 1], val, (val_len - 1)); + len += val_len; } - pdu->qs[qs_len] = '\0'; + qs[len] = '\0'; - return (int)qs_len; + return coap_opt_add_string(pdu, COAP_OPT_URI_QUERY, qs, '&'); } /** @} */ diff --git a/sys/net/application_layer/nanocoap/nanocoap.c b/sys/net/application_layer/nanocoap/nanocoap.c index cb887ca673bf..672d31b101bf 100644 --- a/sys/net/application_layer/nanocoap/nanocoap.c +++ b/sys/net/application_layer/nanocoap/nanocoap.c @@ -128,9 +128,6 @@ int coap_parse(coap_pkt_t *pkt, uint8_t *buf, size_t len) } #ifdef MODULE_GCOAP - coap_get_uri_path(pkt, pkt->url); - pkt->content_type = coap_get_content_type(pkt); - if (coap_get_option_uint(pkt, COAP_OPT_OBSERVE, &pkt->observe_value) != 0) { pkt->observe_value = UINT32_MAX; } @@ -309,14 +306,10 @@ ssize_t coap_handle_req(coap_pkt_t *pkt, uint8_t *resp_buf, unsigned resp_buf_le unsigned method_flag = coap_method2flag(coap_get_code_detail(pkt)); -#ifdef MODULE_GCOAP - uint8_t *uri = pkt->url; -#else uint8_t uri[NANOCOAP_URI_MAX]; if (coap_get_uri_path(pkt, uri) <= 0) { return -EBADMSG; } -#endif DEBUG("nanocoap: URI path: \"%s\"\n", uri); for (unsigned i = 0; i < coap_resources_numof; i++) { diff --git a/tests/unittests/tests-gcoap/tests-gcoap.c b/tests/unittests/tests-gcoap/tests-gcoap.c index e76dc77a4def..d24517d8aadb 100644 --- a/tests/unittests/tests-gcoap/tests-gcoap.c +++ b/tests/unittests/tests-gcoap/tests-gcoap.c @@ -84,8 +84,8 @@ static void test_gcoap__client_get_req(void) TEST_ASSERT_EQUAL_INT(hdr_fixed_len + GCOAP_TOKENLEN, coap_get_total_hdr_len(&pdu)); TEST_ASSERT_EQUAL_INT(COAP_TYPE_NON, coap_get_type(&pdu)); - char uri[10] = {0}; - coap_get_uri(&pdu, (uint8_t *)&uri[0]); + char uri[NANOCOAP_URI_MAX] = {0}; + coap_get_uri_path(&pdu, (uint8_t *)&uri[0]); TEST_ASSERT_EQUAL_STRING(&path[0], &uri[0]); TEST_ASSERT_EQUAL_INT(0, pdu.payload_len); TEST_ASSERT_EQUAL_INT(sizeof(pdu_data), len); @@ -216,7 +216,10 @@ static void test_gcoap__server_get_req(void) TEST_ASSERT_EQUAL_INT(4 + 2, coap_get_total_hdr_len(&pdu)); TEST_ASSERT_EQUAL_INT(COAP_TYPE_NON, coap_get_type(&pdu)); TEST_ASSERT_EQUAL_INT(0, pdu.payload_len); - TEST_ASSERT_EQUAL_STRING("/cli/stats", (char *) &pdu.url[0]); + + char uri[NANOCOAP_URI_MAX] = {0}; + coap_get_uri_path(&pdu, (uint8_t *)&uri[0]); + TEST_ASSERT_EQUAL_STRING("/cli/stats", &uri[0]); } /* From 8fa6f10f97d582e557bbaebe5fcad6b290546235 Mon Sep 17 00:00:00 2001 From: Ken Bannister Date: Tue, 23 Oct 2018 05:04:05 -0400 Subject: [PATCH 6/6] cord/ep: Update for gcoap API change --- sys/net/application_layer/cord/ep/cord_ep.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sys/net/application_layer/cord/ep/cord_ep.c b/sys/net/application_layer/cord/ep/cord_ep.c index a2ea5f28f257..19af6a173e63 100644 --- a/sys/net/application_layer/cord/ep/cord_ep.c +++ b/sys/net/application_layer/cord/ep/cord_ep.c @@ -273,8 +273,11 @@ int cord_ep_register(const sock_udp_ep_t *remote, const char *regif) } /* set some packet options and write query string */ coap_hdr_set_type(pkt.hdr, COAP_TYPE_CON); + coap_opt_add_uint(&pkt, COAP_OPT_CONTENT_FORMAT, COAP_FORMAT_LINK); cord_common_add_qstring(&pkt); + pkt_len = coap_opt_finish(&pkt, COAP_OPT_FINISH_PAYLOAD); + /* add the resource description as payload */ res = gcoap_get_resource_list(pkt.payload, pkt.payload_len, COAP_FORMAT_LINK); @@ -282,9 +285,7 @@ int cord_ep_register(const sock_udp_ep_t *remote, const char *regif) retval = CORD_EP_ERR; goto end; } - - /* finish up the packet */ - pkt_len = gcoap_finish(&pkt, res, COAP_FORMAT_LINK); + pkt_len += res; /* send out the request */ res = gcoap_req_send2(buf, pkt_len, remote, _on_register);