From 5a6dff24becbc1e616bf7039718dd3429f698c25 Mon Sep 17 00:00:00 2001 From: Gilles Boccon-Gibod Date: Wed, 17 Mar 2021 11:40:50 -0700 Subject: [PATCH] fix GG_CoapBlockwiseServerHelper's handling of BLOCK2 transfers --- xp/coap/gg_coap_blockwise.c | 78 +++-- xp/coap/gg_coap_blockwise.h | 4 +- xp/unit_tests/coap/test_gg_coap_blockwise.cpp | 323 +++++++++++++++++- 3 files changed, 374 insertions(+), 31 deletions(-) diff --git a/xp/coap/gg_coap_blockwise.c b/xp/coap/gg_coap_blockwise.c index f182aa60..a9a3d84f 100644 --- a/xp/coap/gg_coap_blockwise.c +++ b/xp/coap/gg_coap_blockwise.c @@ -1018,41 +1018,63 @@ GG_CoapBlockwiseServerHelper_OnRequest(GG_CoapBlockwiseServerHelper* self, // check that the block is either a resent block, or the next expected one bool resent = false; - size_t block_end_offset = self->block_info.offset + GG_CoapMessage_GetPayloadSize(request); - if (self->block_info.offset == self->next_offset) { - // this is the next expected block - if (self->done) { - // we're done, check that the option is consistent with this state + if (self->block_type == GG_COAP_MESSAGE_OPTION_BLOCK1) { + // we're receiving data + size_t block_end_offset = self->block_info.offset + GG_CoapMessage_GetPayloadSize(request); + if (self->block_info.offset == self->next_offset) { + // this is the next expected block + if (self->done) { + // we're done, check that the option is consistent with this state + if (self->block_info.more) { + // shouldn't happen + return GG_COAP_MESSAGE_CODE_BAD_OPTION; + } + resent = true; + } + } else { + // this is not the next expected block, check if it is a resent block or a gap + if (self->block_info.offset) { + if (block_end_offset != self->next_offset) { + // gap! + GG_LOG_WARNING("unexpected block offset received (got %u, expected %u)", + (int)self->block_info.offset, + (int)self->next_offset); + return GG_COAP_MESSAGE_CODE_REQUEST_ENTITY_INCOMPLETE; + } + resent = true; + } else { + // new transfer + self->done = false; + } + } + + // update our expectations + if (!resent) { if (self->block_info.more) { - // shouldn't happen - return GG_COAP_MESSAGE_CODE_BAD_OPTION; + self->next_offset = block_end_offset; + } else { + self->done = true; } - resent = true; } } else { - // this is not the next expected block, check if it is a resent block or a gap - if (self->block_info.offset) { - if (block_end_offset != self->next_offset) { - // gap! - GG_LOG_WARNING("unexpected block offset (got %u, expected %u)", - (int)self->block_info.offset, - (int)self->next_offset); - return GG_COAP_MESSAGE_CODE_REQUEST_ENTITY_INCOMPLETE; + // we're returning data + size_t block_end_offset = self->block_info.offset + self->block_info.size; + if (self->block_info.offset != self->next_offset) { + // this is not the next expected block, check if it is a resent block request or a gap + if (self->block_info.offset) { + if (block_end_offset != self->next_offset) { + // gap! + GG_LOG_WARNING("unexpected block offset requested (got %u, expected %u)", + (int)self->block_info.offset, + (int)self->next_offset); + return GG_COAP_MESSAGE_CODE_PRECONDITION_FAILED; + } + resent = true; } - resent = true; - } else { - // new transfer - self->done = false; } - } - // update our expectations - if (!resent) { - if (self->block_info.more) { - self->next_offset = block_end_offset; - } else { - self->done = true; - } + // update our expectations + self->next_offset = block_end_offset; } // let the caller know if this was a resent request or not diff --git a/xp/coap/gg_coap_blockwise.h b/xp/coap/gg_coap_blockwise.h index 4b1f20b8..0a418911 100644 --- a/xp/coap/gg_coap_blockwise.h +++ b/xp/coap/gg_coap_blockwise.h @@ -89,9 +89,9 @@ typedef struct { typedef struct { uint32_t block_type; ///< GG_COAP_MESSAGE_OPTION_BLOCK1 or GG_COAP_MESSAGE_OPTION_BLOCK2 size_t next_offset; ///< Next expected block offset - bool done; ///< True when we've received the last block + bool done; ///< True when we've received the last block (BLOCK1 only) size_t preferred_block_size; ///< Preferred block size - GG_CoapMessageBlockInfo block_info; ///< Last parsed BLOCK1 option + GG_CoapMessageBlockInfo block_info; ///< Last parsed BLOCK1/BLOCK2 option uint8_t etag[GG_COAP_MESSAGE_MAX_ETAG_OPTION_SIZE]; ///< ETag for the transfer session size_t etag_size; ///< ETag size } GG_CoapBlockwiseServerHelper; diff --git a/xp/unit_tests/coap/test_gg_coap_blockwise.cpp b/xp/unit_tests/coap/test_gg_coap_blockwise.cpp index 7fd8f5fa..164ead64 100644 --- a/xp/unit_tests/coap/test_gg_coap_blockwise.cpp +++ b/xp/unit_tests/coap/test_gg_coap_blockwise.cpp @@ -1384,7 +1384,7 @@ TEST(GG_COAP_BLOCKWISE, Test_ApiEdgeCases) { GG_CoapMessage_Destroy(message); } -TEST(GG_COAP_BLOCKWISE, Test_ServerBlockwiseHelper) { +TEST(GG_COAP_BLOCKWISE, Test_ServerBlockwiseHelper_BLOCK1) { GG_CoapBlockwiseServerHelper helper; GG_CoapBlockwiseServerHelper_Init(&helper, GG_COAP_MESSAGE_OPTION_BLOCK1, 0); @@ -1490,6 +1490,29 @@ TEST(GG_COAP_BLOCKWISE, Test_ServerBlockwiseHelper) { // cleanup GG_CoapMessage_Destroy(request); + // create a message for block 3 (not sequential) + block_info.offset = 3072; + block_info.size = 1024; + block_info.more = false; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK1, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + payload, + sizeof(payload), + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 3 request + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_COAP_MESSAGE_CODE_REQUEST_ENTITY_INCOMPLETE, result); + // create a message for block 2 block_info.offset = 2048; block_info.size = 1024; @@ -1680,6 +1703,304 @@ TEST(GG_COAP_BLOCKWISE, Test_ServerBlockwiseHelper) { GG_TimerScheduler_Destroy(timer_scheduler); } +TEST(GG_COAP_BLOCKWISE, Test_ServerBlockwiseHelper_BLOCK2) { + GG_CoapBlockwiseServerHelper helper; + + GG_CoapBlockwiseServerHelper_Init(&helper, GG_COAP_MESSAGE_OPTION_BLOCK2, 0); + + GG_CoapMessage* request = NULL; + uint8_t token[1] = { 0 }; + + // create a message for a block request with offset 0 + GG_CoapMessageBlockInfo block_info; + block_info.offset = 0; + block_info.size = 1024; + block_info.more = false; + uint32_t block_option_value; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + GG_CoapMessageOptionParam block_option = GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + GG_Result result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 0 request + bool was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(helper.next_offset, 1024); + LONGS_EQUAL(0, (int)helper.done); + + // process the block 0 request again + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(helper.next_offset, 1024); + LONGS_EQUAL(0, (int)helper.done); + + // cleanup + GG_CoapMessage_Destroy(request); + + // create a message for a block with offset out of range + block_info.offset = 10000; + block_info.size = 1024; + block_info.more = true; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the out of range block, it should fail + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_COAP_MESSAGE_CODE_PRECONDITION_FAILED, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(helper.next_offset, 1024); + LONGS_EQUAL(0, (int)helper.done); + + // cleanup + GG_CoapMessage_Destroy(request); + + // create a message for block 1 + block_info.offset = 1024; + block_info.size = 1024; + block_info.more = true; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 1 request + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + LONGS_EQUAL(helper.next_offset, 2048); + + // process the same block again and check that it was marked as resent + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(1, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + LONGS_EQUAL(helper.next_offset, 2048); + + // cleanup + GG_CoapMessage_Destroy(request); + + // create a message for block 3 (not sequential) + block_info.offset = 3072; + block_info.size = 1024; + block_info.more = false; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 3 request + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_COAP_MESSAGE_CODE_PRECONDITION_FAILED, result); + + // create a message for block 2 + block_info.offset = 2048; + block_info.size = 1024; + block_info.more = false; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 2 request + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + LONGS_EQUAL(helper.next_offset, 3072); + + // process the same block and check that it is signaled as resent + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(1, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + LONGS_EQUAL(helper.next_offset, 3072); + + // cleanup + GG_CoapMessage_Destroy(request); + + // create a message for block 0 + block_info.offset = 0; + block_info.size = 1024; + block_info.more = true; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 0 request, which should start a new transfer + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + + // cleanup + GG_CoapMessage_Destroy(request); + + // set an ETag for the transfer + uint8_t etag1[3] = { 0x01, 0x02, 0x03 }; + GG_CoapBlockwiseServerHelper_SetEtag(&helper, etag1, sizeof(etag1)); + + // create a message for block 0, with an If-Match option + GG_CoapMessageOptionParam etag_option = GG_COAP_MESSAGE_OPTION_PARAM_OPAQUE(IF_MATCH, etag1, sizeof(etag1)); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &etag_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // process the block 0 request, which should start a new transfer + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + LONGS_EQUAL(0, (int)helper.done); + + // set a different ETag for the transfer + uint8_t etag2[3] = { 0x04, 0x05, 0x06 }; + GG_CoapBlockwiseServerHelper_SetEtag(&helper, etag2, sizeof(etag2)); + + // process the block 0 request again, this time it should fail with an ETag mismatch + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_COAP_MESSAGE_CODE_PRECONDITION_FAILED, result); + LONGS_EQUAL(0, (int)was_resent); + + // now set a matching Etag and try again + GG_CoapBlockwiseServerHelper_SetEtag(&helper, etag1, sizeof(etag1)); + was_resent = false; + result = GG_CoapBlockwiseServerHelper_OnRequest(&helper, request, &was_resent); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)was_resent); + + // cleanup + GG_CoapMessage_Destroy(request); + + // create an endpoint + GG_CoapEndpoint* endpoint = NULL; + GG_TimerScheduler* timer_scheduler = NULL; + result = GG_TimerScheduler_Create(&timer_scheduler); + LONGS_EQUAL(GG_SUCCESS, result); + result = GG_CoapEndpoint_Create(timer_scheduler, NULL, NULL, &endpoint); + LONGS_EQUAL(GG_SUCCESS, result); + + // create a message for block 0 + block_info.offset = 0; + block_info.size = 1024; + block_info.more = true; + GG_CoapMessageBlockInfo_ToOptionValue(&block_info, &block_option_value); + block_option = (GG_CoapMessageOptionParam) GG_COAP_MESSAGE_OPTION_PARAM_UINT(BLOCK2, block_option_value); + result = GG_CoapMessage_Create(GG_COAP_METHOD_GET, + GG_COAP_MESSAGE_TYPE_CON, + &block_option, + 1, + 0, + token, + sizeof(token), + NULL, + 0, + &request); + LONGS_EQUAL(GG_SUCCESS, result); + + // ask the helper to create a response + GG_CoapMessage* response = NULL; + result = GG_CoapBlockwiseServerHelper_CreateResponse(&helper, + endpoint, + request, + GG_COAP_MESSAGE_CODE_CREATED, + NULL, + 0, + NULL, + 0, + &response); + LONGS_EQUAL(GG_SUCCESS, result); + + GG_CoapMessageOption option; + result = GG_CoapMessage_GetOption(response, GG_COAP_MESSAGE_OPTION_BLOCK2, &option, 0); + LONGS_EQUAL(GG_SUCCESS, result); + result = GG_CoapMessage_GetBlockInfo(response, GG_COAP_MESSAGE_OPTION_BLOCK2, &block_info, 0); + LONGS_EQUAL(GG_SUCCESS, result); + LONGS_EQUAL(0, (int)block_info.more); + LONGS_EQUAL(0, block_info.offset); + + // cleanup + GG_CoapMessage_Destroy(request); + GG_CoapMessage_Destroy(response); + GG_CoapEndpoint_Destroy(endpoint); + GG_TimerScheduler_Destroy(timer_scheduler); +} + //----------------------------------------------------------------------- // Test that we can cancel a blockwise request from *within* a listener //-----------------------------------------------------------------------