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.

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

Set correct buffer size in into_options() on libcoap<4.3.5rc2 #31

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

pulsastrix
Copy link
Member

Versions of libcoap older than 4.3.5rc2 used a user-provided buffer for temporarily storing the options during conversion.

This buffer, however, must be larger than what the libcoap documentation originally suggested, as this buffer area must also hold the option headers.
If this is not done, trailing elements of the URI are silently removed.

This PR increases the used buffer size for this temporary buffer to a value that should be large enough to store all options that could be created from this URI.

Additionally, it adds a check for newer libcoap versions that don't use this buffer anymore.
In that case, we just pass an empty buffer instead, saving memory and avoiding some iterations over the URI path and query string to determine the buffer size.

@pulsastrix pulsastrix added the bug Something isn't working label Aug 22, 2024
@pulsastrix pulsastrix added this to the v0.3.0 milestone Aug 22, 2024
@pulsastrix pulsastrix requested a review from JKRhb August 22, 2024 16:49
@pulsastrix pulsastrix self-assigned this Aug 22, 2024
Copy link

github-actions bot commented Aug 22, 2024

Workflow Status Report

Generated for commit 722fddc on Fri Aug 23 12:44:20 UTC 2024.

CI Status

Linting Report

Clippy check result: success

Refer to the "Files Changed" tab for identified issues.

Code Coverage Report

Filename                                    Stmts    Miss  Cover    Missing
----------------------------------------  -------  ------  -------  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
libcoap/src/session/client.rs                 135      62  54.07%   69-74, 76-84, 88-94, 96, 100, 111, 114, 119, 140, 146, 152-153, 155, 174, 180, 186-187, 189, 211, 213, 215, 217-223, 265-266, 272-282, 293-303, 314-324, 371-376
libcoap/src/session/mod.rs                    154      86  44.16%   61-67, 96-97, 111-225, 236-252, 268-270, 289-291, 301-311, 347, 362, 377-396, 419, 427, 451, 455, 476-480, 558, 564
libcoap/src/session/server.rs                  48      12  75.00%   71-73, 140-141, 149-150, 181-186
libcoap-sys/src/lib.rs                         91      32  64.84%   115-151, 183, 188, 193, 198, 228, 233, 238, 243, 248, 253, 258, 263, 268, 278, 320-337, 459-460
libcoap/src/context.rs                        217     114  47.47%   51, 105, 110-115, 121-122, 127, 132, 134, 136, 138, 140, 142-148, 184-227, 236-244, 261, 297, 380, 385, 401, 409-542, 567-574, 589-599, 612-619, 630-657
libcoap/src/crypto.rs                          31       2  93.55%   109, 183
libcoap/src/error.rs                            4       4  0.00%    87-166
libcoap/src/event.rs                            9       0  100.00%
libcoap/src/mem.rs                            118      49  58.47%   144-158, 192-193, 206-207, 267-293, 315-319, 349-350, 363-365, 377-388, 448-484, 503, 521-523, 526-527
libcoap/src/prng.rs                            18      14  22.22%   67-179
libcoap/src/protocol.rs                       150     110  26.67%   145-146, 152-157, 159-174, 176-177, 184-189, 191-206, 208-209, 266-267, 289, 303-304, 347-352, 361-380, 437-482, 515, 517
libcoap/src/resource.rs                       139      69  50.36%   96-98, 173-208, 216-221, 248, 251, 254, 257, 268-289, 302-304, 329-333, 340-341, 350-352, 358-360, 366-374, 496, 498-509
libcoap/src/transport.rs                       10       3  70.00%   28-33, 54
libcoap/src/types.rs                          235     160  31.91%   77-122, 129-158, 201-276, 286, 352, 426-466, 519-593, 651, 677-727, 781, 796, 826-846, 868-869, 902-986
libcoap/tests/dtls_client_server_test.rs       10       0  100.00%
libcoap/src/message/mod.rs                    245     124  49.39%   107-110, 112-133, 141-147, 149-170, 174, 176, 210, 212, 215-224, 227-245, 247-251, 267-276, 297-303, 400, 443, 449, 480, 488, 490-492, 499-500, 508, 513-526, 531, 551-552, 563-564
libcoap/src/message/request.rs                220     150  31.82%   52, 69-202, 229-257, 265-375, 381, 384-386, 390, 397-401, 412, 416, 431-432, 437, 440-441, 445-446, 450, 453, 456, 459, 462, 473-477
libcoap/src/message/response.rs               143     111  22.38%   33-156, 162, 165, 168, 171, 174, 192-322, 326-330, 356-357
libcoap/tests/common/mod.rs                    43       5  88.37%   66, 91, 93-94, 97
TOTAL                                        2020    1107  45.20%

Diff against main

Filename                Stmts    Miss  Cover
--------------------  -------  ------  -------
libcoap/src/mem.rs          0      +1  -0.85%
libcoap/src/types.rs       -2      +8  -3.95%
TOTAL                      -2      +9  -0.50%

badge

Results for commit: 906e151

Coverage target is 80%

Copy link
Member

@JKRhb JKRhb left a comment

Choose a reason for hiding this comment

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

Great work finding and solving the issue here :) See a couple of small suggestions below, other than that: Looks good to me :)

libcoap/src/message/request.rs Outdated Show resolved Hide resolved
libcoap/src/message/request.rs Show resolved Hide resolved
libcoap/src/types.rs Outdated Show resolved Hide resolved
libcoap/src/types.rs Outdated Show resolved Hide resolved
@pulsastrix pulsastrix merged commit 8904857 into main Aug 23, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants