Repository navigation
Conversation
inet_pton6() committed a hex group when it saw a ':' but never checked what followed it, so a trailing colon after a completed group was silently accepted as if it were not there. This let addresses with an illegal bare trailing colon parse successfully, producing the same binary result as the equivalent address without the stray colon. A colon is only ever valid when followed by another hex digit group or by a second colon forming the "::" compression marker, never by the end of the string. Reject that case at the point the colon is parsed. This also fixes "::1:", a related trailing-colon input that was likewise wrongly accepted for the same underlying reason. Added test cases to unit1961 covering both malformed inputs and a set of valid addresses (::, ::1, 1::, 1::8, 1:2:3:4:5:6:7:8, 1:2:3:4:5:6::8, fe80::1, ::ffff:192.0.2.1) to guard against regressions. Reported-by: Joshua Rogers
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused validation correctly preserves valid compression syntax while rejecting malformed trailing-colon inputs.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes IPv6 parsing so addresses ending in a bare colon are rejected.
Changes:
- Rejects trailing colons in
inet_pton6(). - Adds valid and malformed IPv6 regression cases.
| File | Description |
|---|---|
lib/curlx/inet_pton.c |
Validates that a colon is not terminal. |
tests/unit/unit1961.c |
Adds IPv6 edge-case coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Analysis of PR #23028 at 25be1fea: Test ../../tests/http/test_02_download.py::TestDownload::test_02_07_download_reuse[h3] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_02_download.py::TestDownload::test_02_34_max_total_conns[True-0-h2] failed, but it has been 0.4% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR. Generated by Testclutch |
inet_pton6() committed a hex group when it saw a ':' but never checked what followed it, so a trailing colon after a completed group was silently accepted as if it were not there. This let addresses with an illegal bare trailing colon parse successfully, producing the same binary result as the equivalent address without the stray colon.
A colon is only ever valid when followed by another hex digit group or by a second colon forming the "::" compression marker, never by the end of the string. Reject that case at the point the colon is parsed.
This also fixes "::1:", a related trailing-colon input that was likewise wrongly accepted for the same underlying reason.
Added test cases to unit1961 covering both malformed inputs and a set of valid addresses (::, ::1, 1::, 1::8, 1:2:3:4:5:6:7:8, 1:2:3:4:5:6::8, fe80::1, ::ffff:192.0.2.1) to guard against regressions.
Reported-by: Joshua Rogers