From e2c8df147bb294741e2e3783a7529d11738dccf7 Mon Sep 17 00:00:00 2001 From: Jon Shallow Date: Thu, 4 Mar 2021 15:31:20 +0000 Subject: [PATCH] RFC7959: Handle both Block1 and Block2 for same request/response Correctly track the tokens so that the token that application sees in the response is the same token as used in the initial request. Update coap-server so that response data can be sent back to echo the data that was in the request. Fixes CID 1502431 CID 1502428 CID 1502422 --- examples/coap-server.c | 79 +++++++++++++++++++++++++++++++---------- man/coap-server.txt.in | 5 ++- man/coap_handler.txt.in | 24 ++++++++----- src/block.c | 27 +++++++++++--- src/net.c | 26 ++++++++++++-- 5 files changed, 127 insertions(+), 34 deletions(-) diff --git a/examples/coap-server.c b/examples/coap-server.c index dda5ae0f..848e7498 100644 --- a/examples/coap-server.c +++ b/examples/coap-server.c @@ -103,6 +103,7 @@ int key_defined = 0; static const char *hint = "CoAP"; static int support_dynamic = 0; static uint32_t block_mode = COAP_BLOCK_USE_LIBCOAP; +static int echo_back = 0; static coap_dtls_pki_t * setup_pki(coap_context_t *ctx, coap_dtls_role_t role, char *sni); @@ -215,6 +216,26 @@ release_resource_data(coap_session_t *session UNUSED_PARAM, coap_free(transient_value); } +/* + * Bump the reference count and return reference to data + */ +static coap_binary_t +reference_resource_data(transient_value_t *entry) { + coap_binary_t body; + if (entry) { + /* Bump reference so not removed elsewhere */ + entry->ref_cnt++; + assert(entry->value); + body.length = entry->value->length; + body.s = entry->value->s; + } + else { + body.length = 0; + body.s = NULL; + } + return body; +} + #define INDEX "This is a test server made with libcoap (see https://libcoap.net)\n" \ "Copyright (C) 2010--2021 Olaf Bergmann and others\n\n" @@ -447,6 +468,7 @@ hnd_get_example_data(coap_context_t *ctx UNUSED_PARAM, coap_string_t *query, coap_pdu_t *response ) { + coap_binary_t body; if (!example_data_value) { /* Initialise for the first time */ int i; @@ -465,10 +487,11 @@ hnd_get_example_data(coap_context_t *ctx UNUSED_PARAM, example_data_value = alloc_resource_data(value); } response->code = COAP_RESPONSE_CODE_CONTENT; + body = reference_resource_data(example_data_value); coap_add_data_large_response(resource, session, request, response, token, query, example_data_media_type, -1, 0, - example_data_value ? example_data_value->value->length : 0, - example_data_value ? example_data_value->value->s : NULL, + body.length, + body.s, release_resource_data, example_data_value); } @@ -599,6 +622,16 @@ hnd_put_example_data(coap_context_t *ctx UNUSED_PARAM, } coap_resource_notify_observers(resource, NULL); + if (echo_back) { + coap_binary_t body; + + body = reference_resource_data(example_data_value); + coap_add_data_large_response(resource, session, request, response, token, + query, example_data_media_type, -1, 0, + body.length, + body.s, + release_resource_data, example_data_value); + } } #if SERVER_CAN_PROXY @@ -1298,7 +1331,7 @@ hnd_get(coap_context_t *ctx UNUSED_PARAM, coap_str_const_t *uri_path; int i; dynamic_resource_t *resource_entry = NULL; - coap_binary_t value = { 0, NULL }; + coap_binary_t body; /* * request will be NULL if an Observe triggered request, so the uri_path, * if needed, must be abstracted from the resource. @@ -1323,17 +1356,12 @@ hnd_get(coap_context_t *ctx UNUSED_PARAM, resource_entry = &dynamic_entry[i]; - if (resource_entry->value) { - /* Bump reference so not removed elsewhere */ - resource_entry->value->ref_cnt++; - value.length = resource_entry->value->value->length; - value.s = resource_entry->value->value->s; - } response->code = COAP_RESPONSE_CODE_CONTENT; + body = reference_resource_data(resource_entry->value); coap_add_data_large_response(resource, session, request, response, token, query, resource_entry->media_type, -1, 0, - value.length, - value.s, + body.length, + body.s, release_resource_data, resource_entry->value); } @@ -1362,7 +1390,7 @@ hnd_put(coap_context_t *ctx UNUSED_PARAM, coap_opt_iterator_t opt_iter; coap_opt_t *option; coap_binary_t *data_so_far; - transient_value_t *value; + transient_value_t *transient_value; /* get the uri_path */ uri_path = coap_get_uri_path(request); @@ -1509,18 +1537,18 @@ hnd_put(coap_context_t *ctx UNUSED_PARAM, else { /* single body of data received */ data_so_far = coap_new_binary(size); - if (data_so_far) { + if (data_so_far && size) { memcpy(data_so_far->s, data, size); } } /* Need to de-reference as value may be in use elsewhere */ release_resource_data(session, resource_entry->value); - value = alloc_resource_data(data_so_far); - if (!value) { + transient_value = alloc_resource_data(data_so_far); + if (!transient_value) { response->code = COAP_RESPONSE_CODE_INTERNAL_ERROR; return; } - resource_entry->value = value; + resource_entry->value = transient_value; if (resource_entry->created) { resource_entry->created = 0; @@ -1530,6 +1558,17 @@ hnd_put(coap_context_t *ctx UNUSED_PARAM, response->code = COAP_RESPONSE_CODE_CHANGED; coap_resource_notify_observers(resource_entry->resource, NULL); } + + if (echo_back) { + coap_binary_t body; + + body = reference_resource_data(resource_entry->value); + coap_add_data_large_response(resource, session, request, response, token, + query, resource_entry->media_type, -1, 0, + body.length, + body.s, + release_resource_data, resource_entry->value); + } } /* @@ -2153,7 +2192,7 @@ usage( const char *program, const char *version) { break; } fprintf(stderr, "\n" - "Usage: %s [-d max] [-g group] [-l loss] [-p port] [-v num]\n" + "Usage: %s [-d max] [-e] [-g group] [-l loss] [-p port] [-v num]\n" "\t\t[-A address] [-L value] [-N]\n" "\t\t[-P scheme://address[:port],name1[,name2..]]\n" "\t\t[[-h hint] [-i match_identity_file] [-k key]\n" @@ -2165,6 +2204,7 @@ usage( const char *program, const char *version) { "\t-d max \t\tAllow dynamic creation of up to a total of max\n" "\t \t\tresources. If max is reached, a 4.06 code is returned\n" "\t \t\tuntil one of the dynamic resources has been deleted\n" + "\t-e \t\tEcho back the data sent with a PUT\n" "\t-g group\tJoin the given multicast group\n" "\t-l list\t\tFail to send some datagrams specified by a comma\n" "\t \t\tseparated list of numbers or number ranges\n" @@ -2618,7 +2658,7 @@ main(int argc, char **argv) { clock_offset = time(NULL); - while ((opt = getopt(argc, argv, "c:d:g:h:i:j:J:k:l:mnp:s:u:v:A:C:L:M:NP:R:S:")) != -1) { + while ((opt = getopt(argc, argv, "c:d:eg:h:i:j:J:k:l:mnp:s:u:v:A:C:L:M:NP:R:S:")) != -1) { switch (opt) { case 'A' : strncpy(addr_str, optarg, NI_MAXHOST-1); @@ -2633,6 +2673,9 @@ main(int argc, char **argv) { case 'd' : support_dynamic = atoi(optarg); break; + case 'e': + echo_back = 1; + break; case 'g' : group = optarg; break; diff --git a/man/coap-server.txt.in b/man/coap-server.txt.in index c7bc7c3f..cb424d76 100644 --- a/man/coap-server.txt.in +++ b/man/coap-server.txt.in @@ -14,7 +14,7 @@ coap-server - CoAP Server based on libcoap SYNOPSIS -------- -*coap-server* [*-d* max] [*-g* group] [*-l* loss] [*-p* port] [*-v* num] +*coap-server* [*-d* max] [*-e*] [*-g* group] [*-l* loss] [*-p* port] [*-v* num] [*-A* address] [*-L* value] [*-N*] [*-P* scheme://addr[:port],name1[,name2..]] [[*-h* hint] [*-i* match_identity_file] [*-k* key] @@ -35,6 +35,9 @@ OPTIONS - General limit of 'max'. If 'max' is reached, a 4.06 code is returned until one of the dynamic resources has been deleted. +*-e* :: + Echo back the data sent with a PUT. + *-g* group:: Join specified multicast 'group' on start up. diff --git a/man/coap_handler.txt.in b/man/coap_handler.txt.in index 2d1df4b9..82f59c20 100644 --- a/man/coap_handler.txt.in +++ b/man/coap_handler.txt.in @@ -62,7 +62,7 @@ COAP_REQUEST_PATCH COAP_REQUEST_IPATCH ---- -The handler function prototype is defined as: +The method handler function prototype is defined as: [source, c] ---- typedef void (*coap_method_handler_t)(coap_context_t *context, @@ -75,8 +75,14 @@ typedef void (*coap_method_handler_t)(coap_context_t *context, ---- *NOTE:* _incoming_pdu_ will be NULL for the GET Handler if this is an -internally generated Observe Response. *coap_find_observer()* can be used -to determine the subscription information in this case. +internally generated Observe Response. *coap_resource_get_uri_path*() +can be used to determine the URI in this case. + +*NOTE:* Any data associated with _incoming_pdu_ is no longer be available after +exiting this function as _incoming_pdu_ is deleted, unless *coap_add_data*() +was used to update _response_pdu_ where a copy of the data is taken. In +particular _incoming_pdu_'s data must not be used if calling +*coap_add_data_large_response*(). The *coap_register_response_handler*() function defines a request's response _handler_ for traffic associated with the _context_. The application can use @@ -84,7 +90,7 @@ this for handling any response packets, including sending a RST packet if this response was unexpected. If _handler_ is NULL, then the handler is de-registered. -The handler function prototype is defined as: +The response handler function prototype is defined as: [source, c] ---- typedef enum coap_response_t { @@ -112,7 +118,7 @@ The *coap_register_nack_handler*() function defines a request's negative response _handler_ for traffic associated with the _context_. If _handler_ is NULL, then the handler is de-registered. -The handler function prototype is defined as: +The nack handler function prototype is defined as: [source, c] ---- typedef void (*coap_nack_handler_t)(coap_context_t *context, @@ -134,7 +140,7 @@ The *coap_register_ping_handler*() function defines a _handler_ for tracking receipt of CoAP ping traffic associated with the _context_. If _handler_ is NULL, then the handler is de-registered. -The handler function prototype is defined as: +The ping handler function prototype is defined as: [source, c] ---- typedef void (*coap_ping_handler_t)(coap_context_t *context, @@ -147,7 +153,7 @@ The *coap_register_pong_handler*() function defines a _handler_ for tracking receipt of CoAP ping response traffic associated with the _context_. If _handler_ is NULL, then the handler is de-registered. -The handler function prototype is defined as: +The pong handler function prototype is defined as: [source, c] ---- typedef void (*coap_pong_handler_t)(coap_context_t *context, @@ -160,7 +166,7 @@ The *coap_register_event_handler*() function defines a _handler_ for tracking (D)TLS events associated with the _context_. If _handler_ is NULL, then the handler is de-registered. -The handler function prototype is defined as: +The event handler function prototype is defined as: [source, c] ---- typedef void (*coap_event_handler_t)(coap_context_t *context, @@ -303,7 +309,7 @@ coap_pdu_t *sent, coap_pdu_t *received, const coap_tid_t id) { SEE ALSO -------- -*coap_observe*(3) and *coap_resource*(3) +*coap_block*(3), *coap_observe*(3) and *coap_resource*(3) FURTHER INFORMATION ------------------- diff --git a/src/block.c b/src/block.c index 51eee989..327ba1c9 100644 --- a/src/block.c +++ b/src/block.c @@ -383,6 +383,7 @@ coap_add_data_large_internal(coap_session_t *session, /* Unfortunately need to free this off as potential size change */ LL_DELETE(session->lg_xmit, lg_xmit); coap_block_delete_lg_xmit(session, lg_xmit); + lg_xmit = NULL; break; } } @@ -403,6 +404,7 @@ coap_add_data_large_internal(coap_session_t *session, /* Unfortunately need to free this off as potential size change */ LL_DELETE(session->lg_xmit, lg_xmit); coap_block_delete_lg_xmit(session, lg_xmit); + lg_xmit = NULL; break; } } @@ -491,7 +493,10 @@ coap_add_data_large_internal(coap_session_t *session, memset(lg_xmit->b.b1.token, 0, sizeof(lg_xmit->b.b1.token)); lg_xmit->b.b1.token_length = coap_encode_var_safe8(lg_xmit->b.b1.token, sizeof(token), token); - coap_update_token(pdu, lg_xmit->b.b1.token_length, lg_xmit->b.b1.token); + /* + * Token will be updated in pdu later as original pdu may be needed in + * coap_send_large() + */ coap_update_option(pdu, COAP_OPTION_SIZE1, coap_encode_var_safe(buf, sizeof(buf), @@ -865,6 +870,8 @@ coap_block_new_lg_crcv(coap_session_t *session, coap_pdu_t *pdu) { return NULL; } memcpy(lg_crcv->app_token->s, pdu->token, lg_crcv->token_length); + /* In case it is there - must not be in continuing request PDUs */ + coap_remove_option(&lg_crcv->pdu, COAP_OPTION_BLOCK1); return lg_crcv; } @@ -983,7 +990,7 @@ coap_handle_request_send_block(coap_session_t *session, coap_pdu_t *out_pdu = response; static coap_string_t empty = { 0, NULL}; - if (!COAP_PDU_IS_RESPONSE(&p->pdu) || resource != p->b.b2.resource || + if (COAP_PDU_IS_REQUEST(&p->pdu) || resource != p->b.b2.resource || !coap_string_equal(query ? query : &empty, p->b.b2.query ? p->b.b2.query : &empty)) { /* try out the next one */ @@ -1258,6 +1265,8 @@ coap_handle_request_put_block(coap_context_t *context, } if (block_option) { coap_lg_srcv_t *p; + coap_lg_xmit_t *lg_xmit; + coap_string_t empty = { 0, NULL}; coap_opt_t *size_opt = coap_check_option(pdu, COAP_OPTION_SIZE1, &opt_iter); @@ -1383,7 +1392,17 @@ coap_handle_request_put_block(coap_context_t *context, coap_show_pdu(LOG_DEBUG, pdu); /* Need to do this here as we need to free off p */ h(context, resource, session, pdu, token, query, response); - + /* Check if lg_xmit generated and update PDU code if so */ + LL_FOREACH(session->lg_xmit, lg_xmit) { + if (!COAP_PDU_IS_REQUEST(&lg_xmit->pdu) && + lg_xmit->b.b2.resource == resource && + coap_string_equal(query ? query : &empty, + lg_xmit->b.b2.query ? lg_xmit->b.b2.query : &empty)) { + /* lg_xmit found */ + lg_xmit->pdu.code = response->code; + break; + } + } /* Last chunk - free off shortly */ coap_ticks(&p->last_used); goto skip_app_handler; @@ -1407,7 +1426,7 @@ coap_handle_request_put_block(coap_context_t *context, block.szx), buf); h(context, resource, session, pdu, token, query, response); - if (COAP_RESPONSE_CLASS(response->code == 2)) { + if (COAP_RESPONSE_CLASS(response->code) == 2) { /* Just in case, as there are more to go */ response->code = COAP_RESPONSE_CODE(231); } diff --git a/src/net.c b/src/net.c index 6e1fc574..4432b65f 100644 --- a/src/net.c +++ b/src/net.c @@ -1004,6 +1004,7 @@ coap_send_large(coap_session_t *session, coap_pdu_t *pdu) { coap_lg_crcv_t *lg_crcv = NULL; coap_opt_iterator_t opt_iter; int observe_action = -1; + int have_block1 = 0; coap_opt_t *opt; assert(pdu); @@ -1016,20 +1017,25 @@ coap_send_large(coap_session_t *session, coap_pdu_t *pdu) { } if (COAP_PDU_IS_REQUEST(pdu)) { + coap_block_t block; + opt = coap_check_option(pdu, COAP_OPTION_OBSERVE, &opt_iter); if (opt) { observe_action = coap_decode_var_bytes(coap_opt_value(opt), coap_opt_length(opt)); } + + if (coap_get_block(pdu, COAP_OPTION_BLOCK1, &block) && block.m == 1) + have_block1 = 1; } /* * If type is CON and protocol is not reliable, there is no need to set up * lg_crcv here as it can be built up based on sent PDU if there is a - * Block2 in the response. + * Block2 in the response. However, still need it for observe and block1. */ - if (observe_action != -1 || + if (observe_action != -1 || have_block1 || ((pdu->type == COAP_MESSAGE_NON || COAP_PROTO_RELIABLE(session->proto)) && COAP_PDU_IS_REQUEST(pdu) && pdu->code != COAP_REQUEST_DELETE)) { /* See if this token is already in use for large body responses */ @@ -1061,6 +1067,22 @@ coap_send_large(coap_session_t *session, coap_pdu_t *pdu) { lg_crcv = coap_block_new_lg_crcv(session, pdu); if (lg_crcv == NULL) return COAP_INVALID_TID; + if (have_block1 && session->lg_xmit) { + coap_lg_xmit_t *lg_xmit; + + LL_FOREACH(session->lg_xmit, lg_xmit) { + if (COAP_PDU_IS_REQUEST(&lg_xmit->pdu) && + lg_xmit->b.b1.app_token && + token_match(pdu->token, pdu->token_length, + lg_xmit->b.b1.app_token->s, + lg_xmit->b.b1.app_token->length)) { + /* Need to update the token as set up in the session->lg_xmit */ + coap_update_token(pdu, session->lg_xmit->b.b1.token_length, + session->lg_xmit->b.b1.token); + break; + } + } + } } send_it: