Merge pull request #45 in XP/xp-v-golden-gate from gbg/fix-block2-hel…

…per to master * commit '5a6dff24becbc1e616bf7039718dd3429f698c25': fix GG_CoapBlockwiseServerHelper's handling of BLOCK2 transfers
Fitbit · Mar 19, 2021 · 86f5f42 · 86f5f42
2 parents aee0666 + 5a6dff2
commit 86f5f42
Show file tree

Hide file tree

Showing 3 changed files with 374 additions and 31 deletions.
diff --git 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
@@ -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;