Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/net/nanocoap: improve coap_build_reply() #21155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
sys/net/nanocoap: improve coap_build_reply
- The responsibility for handling matching CoAP No-Response Options
  has been split:
    - `coap_build_reply()` only needs to report this and return
      `-ECANCLED`
    - `coap_handle_req()` does generate the empty ACK is needed.
  ==> As a result, writing CoAP request handlers correctly becomes a
      lost easier. Correct error handling to be present is now
      sufficient for correct handling of No-Response options.
  ==> This change is backward compatible with existing code.
- The API doc has been cleaned up and straightened

Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
  • Loading branch information
maribu and mguetschow committed Jan 24, 2025
commit d1da109e4475bc59e6db0664ed4afdfe1c879e1e
73 changes: 52 additions & 21 deletions sys/include/net/nanocoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -1980,7 +1980,7 @@
*
* @returns amount of bytes written to @p buf
*/
size_t coap_put_option(uint8_t *buf, uint16_t lastonum, uint16_t onum, const void *odata, size_t olen);

Check warning on line 1983 in sys/include/net/nanocoap.h

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters

/**
* @brief Insert block1 option into buffer
Expand Down Expand Up @@ -2077,29 +2077,60 @@
* will create the reply packet header based on parameters from the request
* (e.g., id, token).
*
* Passing a non-zero @p payload_len will ensure the payload fits into the
* buffer along with the header. For this validation, payload_len must include
* any options, the payload marker, as well as the payload proper.
*
* @param[in] pkt packet to reply to
* @param[in] code reply code (e.g., COAP_CODE_204)
* @param[out] rbuf buffer to write reply to
* @param[in] rlen size of @p rbuf
* @param[in] payload_len length of payload
*
* @returns size of reply packet on success
*
* Note that this size can be severely shortened if due to a No-Response option there
mguetschow marked this conversation as resolved.
Show resolved Hide resolved
* is only an empty ACK to be sent back. The caller may just continue populating the
* payload (the space was checked to suffice), but may also skip that needless step
* if the returned length is less than the requested payload length.
*
* @returns 0 if no response should be sent due to a No-Response option in the request
* @returns <0 on error
* @returns -ENOSPC if @p rbuf too small
* Passing a non-zero @p max_data_len will ensure the remaining data fits into
* the buffer along with the header. For this validation, @p max_data_len must
* include any CoAP Options, the payload marker, as well as the payload proper.
*
* @param[in] pkt packet to reply to
* @param[in] code reply code (e.g., COAP_CODE_204)
* @param[out] rbuf buffer to write reply to
* @param[in] rlen size of @p rbuf
* @param[in] max_data_len Length of additional CoAP options, the payload marker and payload
*
* @warning CoAP request handlers *must* check the return value for being
* negative. If it is, they must stop further processing of the
* request and pass on the return value unmodified.
*
* @return @p max_data_len + size of the header written in bytes
*
* @retval -ECANCELED No-Response Option present and matching
* @retval -ENOSPC @p rbuf too small
* @retval <0 other error
*
* Usage:
*
* ```C
* static ssize_t _foo_handler(coap_pkt_t *pkt, uint8_t *buf, size_t len,
* coap_request_ctx_t *context)
* {
* static const char *payload = "Hello World";
* const payload_len = strlen(payload);
* // Worst case estimation of the data to add:
* size_t max_data_len = COAP_OPT_FOO_MAX_LEN + COAP_OPT_BAR_MAX_LEN
* + COAP_PAYLOAD_MARKER_SIZE + payload_len;
* ssize_t hdr_len = coap_build_reply(pkt, COAP_CODE_CONTENT, buf, len, max_data_len);
*
* if (hdr_len < 0) {
* return hdr_len; // pass through error
* }
*
* // This step is needed due to an API design flaw
* hdr_len -= max_data_len;
*
* uint8_t *pos = buf + hdr_len;
* uint16_t lastonum = 0;
* pos += coap_opt_put_uint(buf, lastonum, COAP_OPT_FOO, 42);
* lastonum = COAP_OPT_FOO;
* pos += coap_opt_put_uint(buf, lastonum, COAP_OPT_BAR, 1337);
* *pos++ = COAP_PAYLOAD_MARKER;
* memcpy(pos, payload, payload_len);
mguetschow marked this conversation as resolved.
Show resolved Hide resolved
* pos += payload_len;
* return (uintptr_t)pos - (uintptr_t)buf;
* }
* ```
*/
ssize_t coap_build_reply(coap_pkt_t *pkt, unsigned code,
uint8_t *rbuf, unsigned rlen, unsigned payload_len);
uint8_t *rbuf, unsigned rlen, unsigned max_data_len);

/**
* @brief Build empty reply to CoAP request
Expand Down
16 changes: 14 additions & 2 deletions sys/net/application_layer/nanocoap/nanocoap.c
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,20 @@ ssize_t coap_handle_req(coap_pkt_t *pkt, uint8_t *resp_buf, unsigned resp_buf_le
if (pkt->hdr->code == 0) {
return coap_build_reply(pkt, COAP_CODE_EMPTY, resp_buf, resp_buf_len, 0);
}
return coap_tree_handler(pkt, resp_buf, resp_buf_len, ctx,
coap_resources, coap_resources_numof);

ssize_t retval = coap_tree_handler(pkt, resp_buf, resp_buf_len, ctx,
coap_resources, coap_resources_numof);

if (retval == -ECANCELED) {
DEBUG_PUTS("nanocoap: No-Reponse Option present and matching");
/* ==> reply with empty ACK, if needed */
if (coap_get_type(pkt) == COAP_TYPE_CON) {
return coap_build_empty_ack(pkt, (void *)resp_buf);
}
return 0;
}

return retval;
}

ssize_t coap_subtree_handler(coap_pkt_t *pkt, uint8_t *buf, size_t len,
Expand Down
Loading