nanocoap: Make coap_block_finish more resilient

examples/gcoap_block_server/gcoap_block.c

@@ -28,8 +28,8 @@
 #define ENABLE_DEBUG (0)
 #include "debug.h"
 
 static ssize_t _sha256_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx);
 static ssize_t _riot_block2_handler(coap_pkt_t *pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx);
 
 /* CoAP resources */
 static const coap_resource_t _resources[] = {
@@ -47,7 +47,7 @@
 static const uint8_t block2_board[] = " running on a ";
 static const uint8_t block2_mcu[] = " board with a ";
 
 static ssize_t _riot_block2_handler(coap_pkt_t *pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx)
 {
     (void)ctx;
     coap_block_slicer_t slicer;
@@ -60,7 +60,7 @@
 
     /* Add actual content */
     plen += coap_blockwise_put_bytes(&slicer, buf+plen, block2_intro, sizeof(block2_intro)-1);
     plen += coap_blockwise_put_bytes(&slicer, buf+plen, (uint8_t*)RIOT_VERSION, strlen(RIOT_VERSION));
     plen += coap_blockwise_put_char(&slicer, buf+plen, ')');
     plen += coap_blockwise_put_bytes(&slicer, buf+plen, block2_board, sizeof(block2_board)-1);
     plen += coap_blockwise_put_bytes(&slicer, buf+plen, (uint8_t*)RIOT_BOARD, strlen(RIOT_BOARD));
@@ -73,7 +73,7 @@
     plen += coap_blockwise_put_char(&slicer, buf+plen, 'U');
     plen += coap_blockwise_put_char(&slicer, buf+plen, '.');
 
-    coap_block2_finish(&slicer);
+    coap_block2_finish(pdu, &slicer);
 
     return plen;
 }

sys/include/net/nanocoap.h

@@ -405,7 +405,6 @@
     size_t start;                   /**< Start offset of the current block  */
     size_t end;                     /**< End offset of the current block    */
     size_t cur;                     /**< Offset of the generated content    */
-    uint8_t *opt;                   /**< Pointer to the placed option       */
 } coap_block_slicer_t;
 
 #if defined(MODULE_NANOCOAP_RESOURCES) || DOXYGEN
@@ -958,13 +957,14 @@
  * sets/clears it if required.  Doesn't return the number of bytes, as this
  * function overwrites bytes in the packet rather than adding new.
  *
- * @param[in]     slicer      Preallocated slicer struct to use
- * @param[in]     option      option number (block1 or block2)
+ * @param[in]   pkt         packet to work on
+ * @param[in]   slicer      Preallocated slicer struct to use
+ * @param[in]   option      option number (block1 or block2)
  *
  * @return      true if the `more` bit is set in the block option
  * @return      false if the `more` bit is not set the block option
  */
-bool coap_block_finish(coap_block_slicer_t *slicer, uint16_t option);
+bool coap_block_finish(const coap_pkt_t *pkt, coap_block_slicer_t *slicer, uint16_t option);
 
 /**
  * @brief Finish a block1 request
@@ -975,14 +975,15 @@
  * sets/clears it if required.  Doesn't return the number of bytes, as this
  * function overwrites bytes in the packet rather than adding new.
  *
- * @param[in]     slicer      Preallocated slicer struct to use
+ * @param[in]   pkt         packet to work on
+ * @param[in]   slicer      Preallocated slicer struct to use
  *
  * @return      true if the `more` bit is set in the block option
  * @return      false if the `more` bit is not set the block option
  */
-static inline bool coap_block1_finish(coap_block_slicer_t *slicer)
+static inline bool coap_block1_finish(const coap_pkt_t *pkt, coap_block_slicer_t *slicer)
 {
-    return coap_block_finish(slicer, COAP_OPT_BLOCK1);
+    return coap_block_finish(pkt, slicer, COAP_OPT_BLOCK1);
 }
 
 /**
@@ -994,14 +995,15 @@
  * sets/clears it if required.  Doesn't return the number of bytes, as this
  * function overwrites bytes in the packet rather than adding new.
  *
- * @param[in]     slicer      Preallocated slicer struct to use
+ * @param[in]   pkt         packet to work on
+ * @param[in]   slicer      Preallocated slicer struct to use
  *
  * @return      true if the `more` bit is set in the block option
  * @return      false if the `more` bit is not set the block option
  */
-static inline bool coap_block2_finish(coap_block_slicer_t *slicer)
+static inline bool coap_block2_finish(const coap_pkt_t *pkt, coap_block_slicer_t *slicer)
 {
-    return coap_block_finish(slicer, COAP_OPT_BLOCK2);
+    return coap_block_finish(pkt, slicer, COAP_OPT_BLOCK2);
 }
 
 /**
@@ -1826,7 +1828,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);
 
 /**
  * @brief   Insert block1 option into buffer
@@ -2214,4 +2216,4 @@
 }
 #endif
 #endif /* NET_NANOCOAP_H */
 /** @} */

sys/net/application_layer/gcoap/dns.c

@@ -546,7 +546,7 @@ static int _do_block(coap_pkt_t *pdu, const sock_udp_ep_t *remote,
                                     context->dns_buf,
                                     context->dns_buf_len);
 
-    coap_block1_finish(&slicer);
+    coap_block1_finish(pdu, &slicer);
 
     if ((len = _send(pdu->hdr, len, remote, slicer.start == 0, context, tl_type)) <= 0) {
         DEBUG("gcoap_dns: msg send failed: %d\n", (int)len);

sys/net/application_layer/gcoap/fileserver.c

@@ -250,7 +250,7 @@ static ssize_t _get_file(coap_pkt_t *pdu, uint8_t *buf, size_t len,
     vfs_close(fd);
 
     slicer.cur = slicer.end + more;
-    coap_block2_finish(&slicer);
+    coap_block2_finish(pdu, &slicer);
 
     if (read == 0) {
         /* Rewind to clear payload marker */
@@ -489,7 +489,7 @@ static ssize_t _get_directory(coap_pkt_t *pdu, uint8_t *buf, size_t len,
     }
 
     vfs_closedir(&dir);
-    coap_block2_finish(&slicer);
+    coap_block2_finish(pdu, &slicer);
 
     return (uintptr_t)buf - (uintptr_t)pdu->hdr;
 }

sys/net/application_layer/nanocoap/nanocoap.c

@@ -199,22 +199,32 @@
     return res;
 }
 
-uint8_t *coap_find_option(coap_pkt_t *pkt, unsigned opt_num)
+static uint8_t *_get_option(const coap_pkt_t *pkt, unsigned opt_num, const coap_optpos_t **found_optpos)
 {
     const coap_optpos_t *optpos = pkt->options;
     unsigned opt_count = pkt->options_len;
 
     while (opt_count--) {
         if (optpos->opt_num == opt_num) {
-            unsigned idx = index_of(pkt->options, optpos);
-            bf_unset(pkt->opt_crit, idx);
+            *found_optpos = optpos;
             return (uint8_t*)pkt->hdr + optpos->offset;
         }
         optpos++;
     }
     return NULL;
 }
 
+uint8_t *coap_find_option(coap_pkt_t *pkt, unsigned opt_num)
+{
+    const coap_optpos_t *optpos = NULL;
+    uint8_t *optloc = _get_option(pkt, opt_num, &optpos);
+    if (optloc) {
+        unsigned idx = index_of(pkt->options, optpos);
+        bf_unset(pkt->opt_crit, idx);
+    }
+    return optloc;
+}
+
 /*
  * Parse option attributes
  *
@@ -782,7 +792,7 @@
     return offset;
 }
 
 size_t coap_put_option(uint8_t *buf, uint16_t lastonum, uint16_t onum, const void *odata, size_t olen)
 {
     assert(lastonum <= onum);
 
@@ -855,8 +865,6 @@
 size_t coap_opt_put_block(uint8_t *buf, uint16_t lastonum, coap_block_slicer_t *slicer,
                           bool more, uint16_t option)
 {
-    slicer->opt = buf;
-
     return coap_opt_put_uint(buf, lastonum, option, _slicer2blkopt(slicer, more));
 }
 
@@ -1046,8 +1054,6 @@
 ssize_t coap_opt_add_block(coap_pkt_t *pkt, coap_block_slicer_t *slicer,
                            bool more, uint16_t option)
 {
-    slicer->opt = pkt->payload;
-
     return coap_opt_add_uint(pkt, option, _slicer2blkopt(slicer, more));
 }
 
@@ -1209,21 +1215,22 @@
     coap_block_slicer_init(slicer, blknum, coap_szx2size(szx));
 }
 
-bool coap_block_finish(coap_block_slicer_t *slicer, uint16_t option)
+bool coap_block_finish(const coap_pkt_t *pkt, coap_block_slicer_t *slicer, uint16_t option)
 {
-    assert(slicer->opt);
+    const coap_optpos_t *optpos = NULL;
+    uint8_t *startpos = _get_option(pkt, option, &optpos);
 
     /* The third parameter for _decode_value() points to the end of the header.
      * We don't know this position, but we know we can read the option because
      * it's already in the buffer. So just point past the option. */
-    uint8_t *pos = slicer->opt + 1;
-    uint16_t delta = _decode_value(*slicer->opt >> 4, &pos, slicer->opt + 3);
+    uint8_t *endpos = startpos + 1;
+    uint16_t delta = _decode_value(*startpos >> 4, &endpos, pkt->payload);
 
     bool more = slicer->cur > slicer->end;
     uint32_t blkopt = _slicer2blkopt(slicer, more);
     size_t olen = _encode_uint(&blkopt);
 
-    coap_put_option(slicer->opt, option - delta, option, (uint8_t *)&blkopt, olen);
+    coap_put_option(startpos, option - delta, option, (uint8_t *)&blkopt, olen);
     return more;
 }
 
@@ -1235,7 +1242,7 @@
     if (slicer->cur < slicer->start) {
         return coap_build_reply(pkt, COAP_CODE_BAD_OPTION, rbuf, rlen, 0);
     }
-    coap_block2_finish(slicer);
+    coap_block2_finish(pkt, slicer);
     return coap_build_reply(pkt, code, rbuf, rlen, payload_len);
 }