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

sys/net/nanocoap: fix coap_get_total_hdr_len() #20946

Merged
merged 2 commits into from
Nov 1, 2024
Merged

sys/net/nanocoap: fix coap_get_total_hdr_len() #20946

merged 2 commits into from
Nov 1, 2024
+28 −12

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 1, 2024

Contribution description

Before coap_get_total_hdr_len() did not take the extended TKL field (RFC 8974) into account. This fixes the issue.

Testing procedure

Unit tests in master (cherry-picking the second commit into master):

$ make BOARD=native -C tests/unittests all test
[...]

main(): This is RIOT! (Version: 2024.10-devel-370-g00e25)
Help: Press s to start test, r to print it is ready
READY
s
START
.......................==83870==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
.....................................................................................................................................................................................................................==83870==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0xffcd4000; bottom 0xedf02000; size: 0x11dd2000 (299704320)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
.................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
nanocoap_tests.test_nanocoap__token_length_ext_16 (tests/unittests/tests-nanocoap/tests-nanocoap.c 1109) exp 21 was 20
.
nanocoap_tests.test_nanocoap__token_length_ext_269 (tests/unittests/tests-nanocoap/tests-nanocoap.c 1138) exp 275 was 273
...........................................................................................................................................................................................................................................................................................................................................................
run 1209 failures 2

Unit tests in this PR

$ make BOARD=native -C tests/unittests all test
[...]
main(): This is RIOT! (Version: 2024.10-devel-372-g2b3da-sys/nanocoap/fix-header-size-computation)
Help: Press s to start test, r to print it is ready
READY
s
START
.......................==81436==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
.....................................................................................................................................................................................................................==81436==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0xfff08000; bottom 0xed502000; size: 0x12a06000 (312500224)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
OK (1209 tests)

Issues/PRs references

None

@maribu
sys/net/nanocoap: fix coap_get_total_hdr_len()
469edf4 
Before `coap_get_total_hdr_len()` did not take the extended TKL field
(RFC 8974) into account. This fixes the issue.
@maribu
tests/unittests: increase coverage for coap_get_total_hdr_len()
2b3da39 
Previously the corner case when RFC 8974 extended TKL fields are used
the result of coap_get_totel_hdr_len() was not tested, resulting in a
bug slipping through the test. This increase the test coverage.
@maribu maribu requested a review from miri64 as a code owner November 1, 2024 13:02
@maribu maribu requested review from benpicco and removed request for miri64 November 1, 2024 13:02
@github-actions github-actions bot added Area: network Area: Networking Area: tests Area: tests and testing framework Area: sys Area: System labels Nov 1, 2024
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: network Area: Networking Area: tests Area: tests and testing framework Area: sys Area: System labels Nov 1, 2024
@riot-ci
Copy link

riot-ci commented Nov 1, 2024

Murdock results

✔️ PASSED

2b3da39 tests/unittests: increase coverage for coap_get_total_hdr_len()

Success Failures Total Runtime
10215 0 10215 14m:48s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 1, 2024
Merged via the queue into RIOT-OS:master with commit 5ae4c96 Nov 1, 2024
31 checks passed
@maribu
Copy link
Member Author

maribu commented Nov 1, 2024

Thx! :)

@maribu maribu deleted the sys/nanocoap/fix-header-size-computation branch November 1, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabian18 fabian18 fabian18 approved these changes

@benpicco benpicco Awaiting requested review from benpicco

Assignees
No one assigned
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maribu @riot-ci @fabian18