sys/nanocoap_sock: fix handling bockwise requests

sys/include/net/coap.h

@@ -18,6 +18,10 @@
  *
  */
 
+#include <stdbool.h>
+
+#include "bitarithm.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -622,7 +626,7 @@ typedef enum {
 /** @} */
 
 /**
- * @brief Coap block-wise-transfer size SZX
+ * @brief CoAP block-wise-transfer size SZX
  */
 typedef enum {
     COAP_BLOCKSIZE_16 = 0,
@@ -634,6 +638,53 @@ typedef enum {
     COAP_BLOCKSIZE_1024,
 } coap_blksize_t;
 
+/**
+ * @brief   Convert a `coap_blksize_t` constant into the corresponding size in B
+ *
+ * @param[in]   blocksize   Constant to convert
+ *
+ * @return      The number of bytes that constant encodes.
+ *
+ * Example usage:
+ * ```c
+ * assert(coap_blksize_to_bytes(COAP_BLOCKSIZE_128) == 128);
+ * ```
+ */
+static inline unsigned coap_blksize_to_bytes(coap_blksize_t blocksize)
+{
+    if ((unsigned)blocksize > (unsigned)COAP_BLOCKSIZE_1024) {
+        return 0;
+    }
+
+    /* > The block size is represented as a three-bit
+     * > unsigned integer indicating the size of a block to the power of
+     * > two.  Thus, block size = 2**(SZX + 4).
+     *
+     * https://www.rfc-editor.org/info/rfc7959/#section-2.2 */
+    return (unsigned)1 << (4U + (unsigned)blocksize);
+}
+
+/**
+ * @brief   Convert a block size in bytes to the corresponding `coap_blksize_t`
+ *
+ * @param[out]  dest    The corresponding blocksize is written here
+ * @param[in]   bytes   The block size to converted
+ *
+ * @retval      true    Success and result is written to @p dest
+ * @retval      false   @p bytes is no valid blocksize, @p dest is not touched
+ */
+static inline bool coap_bytes_to_blocksize(coap_blksize_t *dest, unsigned bytes)
+{
+    /* bytes must be a power of 2 between 16 and 1024 inclusively */
+    if ((bytes < 16) || (bytes > 1024) || (bytes & (bytes - 1))) {
+        return false;
+    }
+
+    *dest = bitarithm_msb(bytes) - 4;
+
+    return true;
+}
+
 #ifdef __cplusplus
 }
 #endif

sys/include/net/nanocoap_sock.h

@@ -136,13 +136,15 @@
 #include <stdint.h>
 #include <unistd.h>
 
-#include "random.h"
+#include "event/thread.h"
 #include "net/nanocoap.h"
 #include "net/sock/udp.h"
 #include "net/sock/util.h"
+#include "random.h"
+
 #if IS_USED(MODULE_NANOCOAP_DTLS)
-#include "net/credman.h"
-#include "net/sock/dtls.h"
+#  include "net/credman.h"
+#  include "net/sock/dtls.h"
 #endif
 
 #ifdef __cplusplus
@@ -913,8 +915,8 @@ int nanocoap_sock_get_blockwise(nanocoap_sock_t *sock, const char *path,
  * @param[in]   dst        Target buffer
  * @param[in]   len        Target buffer length
  *
- * @returns     <0 on error
- * @returns     -EINVAL    if an invalid url is provided
+ * @retval      <0         error
+ * @retval      -EINVAL    if an invalid url is provided or @p blksize is invalid
  * @returns     size of the response payload on success
  */
 int nanocoap_sock_get_slice(nanocoap_sock_t *sock, const char *path,

sys/net/application_layer/nanocoap/sock.c

@@ -21,24 +21,23 @@
  * @}
  */
 
-#include <errno.h>
+#include <errno.h> /* IWYU pragma: keep (exports EINVAL, ...) */
 #include <string.h>
 #include <stdio.h>
 
-#include "container.h"
-#include "event/thread.h"
 #include "net/credman.h"
 #include "net/nanocoap.h"
 #include "net/nanocoap_sock.h"
-#ifdef MODULE_SOCK_ASYNC_EVENT
-#include "net/sock/async/event.h"
-#endif
 #include "net/sock/udp.h"
 #include "net/sock/util.h"
 #include "random.h"
 #include "sys/uio.h" /* IWYU pragma: keep (exports struct iovec) */
 #include "ztimer.h"
 
+#ifdef MODULE_SOCK_ASYNC_EVENT
+#  include "net/sock/async/event.h"
+#endif
+
 #define ENABLE_DEBUG 0
 #include "debug.h"
 
@@ -66,8 +65,9 @@ enum {
 typedef struct {
     coap_blockwise_cb_t callback;
     void *arg;
-    uint32_t blknum;
+    uint32_t offset;
     bool more;
+    coap_blksize_t blocksize;
 #if CONFIG_NANOCOAP_SOCK_BLOCK_TOKEN
     uint8_t token[4];
 #endif
@@ -584,7 +584,7 @@ ssize_t nanocoap_sock_unobserve_url(const char *url, coap_observe_client_t *ctx)
 }
 #endif /* MODULE_NANOCOAP_SOCK_OBSERVE */
 
-ssize_t _sock_put_post(nanocoap_sock_t *sock, const char *path, unsigned code,
+static ssize_t _sock_put_post(nanocoap_sock_t *sock, const char *path, unsigned code,
                        uint8_t type, const void *request, size_t len,
                        void *response, size_t max_len)
 {
@@ -788,29 +788,34 @@ static int _block_cb(void *arg, coap_pkt_t *pkt)
 
     /* response was not block-wise */
     if (!coap_get_block2(pkt, &block2)) {
-        block2.offset = 0;
-        block2.more = false;
+        block2 = (coap_block1_t){ .offset = 0 };
     }
 
     DEBUG("nanocoap: got block %"PRIu32" (offset %u)\n",
           block2.blknum, (unsigned)block2.offset);
 
-    if (block2.blknum != ctx->blknum) {
+    if (block2.offset != ctx->offset) {
         return -EAGAIN;
     }
 
     ctx->more = block2.more;
+    ctx->offset += coap_blksize_to_bytes(block2.szx);
+    ctx->blocksize = block2.szx;
     return ctx->callback(ctx->arg, block2.offset, pkt->payload, pkt->payload_len, block2.more);
 }
 
 static int _fetch_block(nanocoap_sock_t *sock,
-                        const char *path, coap_blksize_t blksize,
-                        _block_ctx_t *ctx)
+                        const char *path,_block_ctx_t *ctx)
 {
 
     void *token = NULL;
     size_t token_len = 0;
 
+    if ((unsigned)ctx->blocksize > COAP_BLOCKSIZE_1024) {
+        assert(0);
+        return -EINVAL;
+    }
+
 #if CONFIG_NANOCOAP_SOCK_BLOCK_TOKEN
     /* HACK: Older versions of go-coap always expected a token
      * see https://github.com/plgd-dev/go-coap/issues/512
@@ -829,7 +834,8 @@ static int _fetch_block(nanocoap_sock_t *sock,
     }
 
     coap_opt_put_uri_pathquery(&state, path);
-    coap_opt_put_uint(&state, COAP_OPT_BLOCK2, (ctx->blknum << 4) | blksize);
+    uint32_t blocknum = ctx->offset >> ((unsigned)ctx->blocksize + 4);
+    coap_opt_put_uint(&state, COAP_OPT_BLOCK2, (blocknum << 4) | ctx->blocksize);
 
     if (coap_builder_has_overflown(&state)) {
         return -EOVERFLOW;
@@ -900,6 +906,7 @@ int nanocoap_sock_get_blockwise(nanocoap_sock_t *sock, const char *path,
         .callback = callback,
         .arg = arg,
         .more = true,
+        .blocksize = blksize,
     };
 
 #if CONFIG_NANOCOAP_SOCK_BLOCK_TOKEN
@@ -908,21 +915,21 @@ int nanocoap_sock_get_blockwise(nanocoap_sock_t *sock, const char *path,
 
     uint8_t retries = CONFIG_COAP_MAX_RETRANSMIT;
     while (ctx.more) {
-        DEBUG("nanocoap: fetching block %"PRIu32"\n", ctx.blknum);
+        DEBUG("nanocoap: fetching block at offset %" PRIu32 "\n", ctx.offset);
 
-        int res = _fetch_block(sock, path, blksize, &ctx);
+        int res = _fetch_block(sock, path, &ctx);
         if (res == -EAGAIN) {
             if (--retries) {
                 continue;
             }
             res = -EBADMSG;
         }
         if (res < 0) {
-            DEBUG("nanocoap: error fetching block %"PRIu32": %d\n", ctx.blknum, res);
+            DEBUG("nanocoap: error fetching block at offset %" PRIu32 ": %d\n",
+                  ctx.offset, res);
             return res;
         }
 
-        ctx.blknum += 1;
         retries = CONFIG_COAP_MAX_RETRANSMIT;
     }
 
@@ -944,7 +951,7 @@ static int _2buf_slice(void *arg, size_t offset, uint8_t *buf, size_t len, int m
         return 0;
     }
 
-    if (offset > ctx->offset + ctx->len) {
+    if (offset > ctx->offset) {
         return 0;
     }
 
@@ -989,7 +996,7 @@ int nanocoap_sock_get_slice(nanocoap_sock_t *sock, const char *path,
 {
     /* try to find optimal blocksize */
     unsigned num_blocks = _num_blks(offset, len, blksize);
-    for (uint8_t szx = 0; szx < blksize; ++szx) {
+    for (uint8_t szx = 0; szx < (uint8_t)blksize; ++szx) {
         if (_num_blks(offset, len, szx) <= num_blocks) {
             blksize = szx;
             break;
@@ -1005,8 +1012,9 @@ int nanocoap_sock_get_slice(nanocoap_sock_t *sock, const char *path,
     _block_ctx_t ctx = {
         .callback = _2buf_slice,
         .arg = &dst_ctx,
-        .blknum = offset >> (blksize + 4),
+        .offset = offset,
         .more = true,
+        .blocksize = blksize,
     };
 
 #if CONFIG_NANOCOAP_SOCK_BLOCK_TOKEN
@@ -1015,21 +1023,21 @@ int nanocoap_sock_get_slice(nanocoap_sock_t *sock, const char *path,
 
     uint8_t retries = CONFIG_COAP_MAX_RETRANSMIT;
     while (dst_ctx.len) {
-        DEBUG("nanocoap: fetching block %"PRIu32"\n", ctx.blknum);
+        DEBUG("nanocoap: fetching block at offset %" PRIu32 "\n", ctx.offset);
 
-        int res = _fetch_block(sock, path, blksize, &ctx);
+        int res = _fetch_block(sock, path, &ctx);
         if (res == -EAGAIN) {
             if (--retries) {
                 continue;
             }
             res = -EBADMSG;
         }
         if (res < 0) {
-            DEBUG("nanocoap: error fetching block %"PRIu32": %d\n", ctx.blknum, res);
+            DEBUG("nanocoap: error fetching block at offset %" PRIu32 ": %d\n",
+                  ctx.offset, res);
             return res;
         }
 
-        ctx.blknum += 1;
         retries = CONFIG_COAP_MAX_RETRANSMIT;
     }