Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nanocoap: fix coap_block2_finish() when option size is shrinking #20688

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

nanocoap: fix coap_block2_finish() when option size is shrinking #20688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f56eb0f
nanocoap: fix coap_block2_finish() when option size is shrinking
benpicco May 22, 2024
d1f60d7
fixup! nanocoap: fix coap_block2_finish() when option size is shrinking
benpicco May 23, 2024
1c9b17f
nanocoap_fileserver: adjust pkt length if CoAP header shrunk
benpicco May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions sys/include/net/nanocoap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@
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 */
uint8_t obytes; /**< size of the placed option */
} coap_block_slicer_t;

#if defined(MODULE_NANOCOAP_RESOURCES) || DOXYGEN
Expand Down Expand Up @@ -993,10 +994,9 @@
* @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
* @return number of bytes by which the CoAP header shrunk
*/
bool coap_block_finish(coap_block_slicer_t *slicer, uint16_t option);
int coap_block_finish(coap_block_slicer_t *slicer, uint16_t option);

/**
* @brief Finish a block1 request
Expand All @@ -1009,10 +1009,9 @@
*
* @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
* @return number of bytes by which the CoAP header shrunk
*/
static inline bool coap_block1_finish(coap_block_slicer_t *slicer)
static inline int coap_block1_finish(coap_block_slicer_t *slicer)
{
return coap_block_finish(slicer, COAP_OPT_BLOCK1);
}
Expand All @@ -1028,10 +1027,9 @@
*
* @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
* @return number of bytes by which the CoAP header shrunk
*/
static inline bool coap_block2_finish(coap_block_slicer_t *slicer)
static inline int coap_block2_finish(coap_block_slicer_t *slicer)
{
return coap_block_finish(slicer, COAP_OPT_BLOCK2);
}
Expand Down Expand Up @@ -1859,7 +1857,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);

Check warning on line 1860 in sys/include/net/nanocoap.h

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters

/**
* @brief Insert block1 option into buffer
Expand Down Expand Up @@ -2293,4 +2291,4 @@
}
#endif
#endif /* NET_NANOCOAP_H */
/** @} */

Check warning on line 2294 in sys/include/net/nanocoap.h

View workflow job for this annotation

GitHub Actions / static-tests 

source file is too long
2 changes: 1 addition & 1 deletion sys/net/application_layer/nanocoap/fileserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,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);
resp_len -= coap_block2_finish(&slicer);

if (read == 0) {
/* Rewind to clear payload marker */
Expand Down
16 changes: 12 additions & 4 deletions sys/net/application_layer/nanocoap/nanocoap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@
return offset;
}

size_t coap_put_option(uint8_t *buf, uint16_t lastonum, uint16_t onum, const void *odata, size_t olen)

Check warning on line 889 in sys/net/application_layer/nanocoap/nanocoap.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
{
assert(lastonum <= onum);

Expand Down Expand Up @@ -1163,8 +1163,9 @@
bool more, uint16_t option)
{
slicer->opt = pkt->payload;
slicer->obytes = coap_opt_add_uint(pkt, option, _slicer2blkopt(slicer, more));

return coap_opt_add_uint(pkt, option, _slicer2blkopt(slicer, more));
return slicer->obytes;
}

ssize_t coap_opt_add_proxy_uri(coap_pkt_t *pkt, const char *uri)
Expand Down Expand Up @@ -1325,7 +1326,7 @@
coap_block_slicer_init(slicer, blknum, coap_szx2size(szx));
}

bool coap_block_finish(coap_block_slicer_t *slicer, uint16_t option)
int coap_block_finish(coap_block_slicer_t *slicer, uint16_t option)
{
assert(slicer->opt);

Expand All @@ -1339,8 +1340,15 @@
uint32_t blkopt = _slicer2blkopt(slicer, more);
size_t olen = _encode_uint(&blkopt);

coap_put_option(slicer->opt, option - delta, option, (uint8_t *)&blkopt, olen);
return more;
uint8_t obytes = coap_put_option(slicer->opt, option - delta, option,
(uint8_t *)&blkopt, olen);
if (obytes != slicer->obytes) {
/* if the option grew larger, we already corrupted content */
assert(obytes < slicer->obytes);
memmove(slicer->opt + obytes, slicer->opt + slicer->obytes,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The length of the move is not correct. Look how the payload changed.
Screenshot from 2024-05-23 11-31-30

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg, that's because the size generated by coap_opt_add_block2() is used to calculate the header and therefore the packet length.
Even when updating the coap_pkt_t struct in coap_block2_finish(), the header length will already have been calculated by coap_opt_finish().

1 + slicer->cur - slicer->obytes);
}
return slicer->obytes - obytes;
}

ssize_t coap_block2_build_reply(coap_pkt_t *pkt, unsigned code,
Expand Down
Loading