uri: fix off-by-one in port bounds check - #140
Merged
Merged
Conversation
sc_uri_create() rejected ports > 65536 instead of > 65535, so the invalid port 65536 was accepted as valid. Valid TCP/UDP ports are 0-65535 (16-bit range).
Owner
|
@94xhn could you please add a test? |
The off-by-one fix needs an executable test for both the maximum valid port and the first invalid port, so future changes cannot silently reopen the boundary. Constraint: The maintainer requested a regression test on PR tezc#140 Rejected: Add a standalone test binary | The existing uri test target already covers the parser and keeps the change focused Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep 65535 and 65536 as adjacent explicit boundary cases Tested: Old origin/master fails at the 65536 assertion; fixed tree passes Linux GCC with ASan/UBSan, MinGW GCC 8, and official CMake/CTest Not-tested: Zig Windows workflow remains blocked by its upstream 153-byte download Related: tezc#140 (comment)
Contributor
Author
|
Added the requested regression in uri/uri_test.c: 65535 is accepted and preserved, while 65536 is rejected. The new 65536 assertion fails against the parent origin/master implementation and passes with this fix. Reran official uri CMake/CTest, Linux GCC ASan/UBSan with -Werror, and MinGW GCC 8 strict builds. Pushed as 6200737. |
tezc
approved these changes
Jul 18, 2026
Owner
|
@94xhn thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sc_uri_create() rejects ports strictly greater than 65536, which means the invalid port `65536` is currently accepted as valid. Valid TCP/UDP ports are 0-65535 (16-bit range), so the check should reject anything > 65535.
Before: `http://host:65536/path\` -> accepted, port stored as `"65536"`
After: `http://host:65536/path\` -> correctly rejected (`sc_uri_create` returns NULL)
`65535` and `65537` behavior is unchanged (still accepted / still rejected respectively).
Verified by compiling `uri/sc_uri.c` standalone and exercising `sc_uri_create()` on the boundary values, and by running the existing unmodified `uri/uri_test.c` against the patched file (exit code 0, all asserts pass — its existing port assertions use 8042/123/80/9090/1 and never exercised this 65535/65536 boundary).
One-line fix, no behavior change for any in-range or clearly-out-of-range port value.