Skip to content

Commit 5e58ce7

Browse files
committed
feat(uri): Uri::CheckHostName lands, closing #1997 (group A-2, SR-AUD-151)
UriHostNameType has documented this member since the enum was ported and nothing in this runtime could produce a value of that type. THE RECORDED COST WAS UNDERSTATED. #1997 priced A-2 as a new public module edge to System::Net::IPAddress or a second address-literal parser. The first is impossible rather than expensive: modules/net declares PUBLIC_DEPENDENCIES ... Uri, so that edge is a CYCLE -- the dependency inversion Guid.cpp refused for cryptography, and the reason a first cut written against IPAddress was rejected by the validator. A third route neither option named: modules/net's IPv4/IPv6 scanners are pure string-to-number scanners with no platform call and no dependency on IPAddress, so they moved verbatim into Core.Base. Both modules already depend on it, so the graph is unchanged, there is one definition instead of two, and IPAddress.cpp is -197/+11. The move is behaviour-preserving and measured: Net runs 340/340 unchanged. IPv4-before-DNS decides an answer rather than tidying: CheckHostName passes allowIPv6=false and unknownScheme=false, selecting ParseNonCanonical, so 1, 0x7F.1 and 3232235777 are IPv4 though all three are good DNS labels. Two rules are new with the member: the label rules (first char ASCII alnum, length 1..63, trailing dot accepted), which the constructor never needed; and an unbracketed IPv6 literal is still IPv6, which .NET gets by retrying IsValid([name]). The constructor's character loop was factored so there is one definition -- two grammars for one question is the #2393 shape -- while the two remain different questions, asserted together. Seven mutations all caught. M6 was NOT CAUGHT at first and found a defect in my test rather than the code: [::1]junk fails on the front/back guard and never enters the bracketed branch, so a body measuring to the first ] passed it. The discriminating input is [::1]]. Gate 17,694/38 green. Graph 41/94. Zero downstream sites. #1997 closed.
1 parent 19f6876 commit 5e58ce7

10 files changed

Lines changed: 702 additions & 235 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

NEXT.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,46 @@
139139
> and each is itemised in §2 below. **§4b says where to look next, and the method that found all
140140
> three of today's tickets.**
141141

142+
## 2026-08-20 — #1997 group A-2: `Uri::CheckHostName`, and the move that made it possible — **#1997 CLOSED**
143+
144+
**Gate: 17,694 / 38, 0 failed, 0 skipped** (+5, `Uri` 314 → 319; **`Net` unchanged at 340**, which
145+
is the evidence the move below changed no behaviour). Graph 41/94.
146+
147+
`UriHostNameType` had documented this member since the enum was ported and **nothing in this
148+
runtime could produce a value of that type**.
149+
150+
**The recorded cost was understated.** #1997 priced A-2 as *"a new public module edge to
151+
`System::Net::IPAddress`, or a second address-literal parser"*. **The first is impossible rather
152+
than expensive**: `modules/net` declares `PUBLIC_DEPENDENCIES … Uri`, so that edge is a **cycle** —
153+
the dependency inversion `Guid.cpp` refused for cryptography, and the reason a first cut written
154+
against `IPAddress` was rejected by the boundary validator.
155+
156+
**A third route neither option named.** `modules/net`'s IPv4/IPv6 scanners are pure
157+
string-to-number scanners with no platform call and no dependency on `IPAddress`, so they moved
158+
verbatim into `Core.Base`. Both modules already depend on it → **graph unchanged, one definition
159+
instead of two**, `IPAddress.cpp` **−197 / +11**.
160+
161+
**IPv4-before-DNS decides an answer rather than tidying**: `CheckHostName` passes
162+
`allowIPv6=false, unknownScheme=false`, selecting `ParseNonCanonical` — so `"1"`, `"0x7F.1"` and
163+
`"3232235777"` are **IPv4** though all three are good DNS labels, while `"1.2.3.4.5"` falls through
164+
to **Dns**.
165+
166+
Two rules new with the member: the **label** rules (first char an ASCII letter or digit, length
167+
**1..63**, **trailing dot accepted**), which the constructor never needed; and **an unbracketed IPv6
168+
literal is still IPv6**, which .NET gets by retrying `IsValid($"[{name}]")`.
169+
170+
The constructor's character loop was **factored so there is one definition** (#2393's shape), while
171+
the two remain different questions — the constructor applies no label rules, so `Uri("http://-x/")`
172+
parses where `CheckHostName("-x")` is `Unknown`, asserted together.
173+
174+
Seven mutations, all caught. **M6 was NOT CAUGHT at first and found a defect in my test, not the
175+
code**: *"the entire name must be consumed"* was asserted with `"[::1]junk"`, which fails on the
176+
front/back guard and never enters the bracketed branch, so a body measuring to the **first** `]`
177+
passed it. The discriminating input is `"[::1]]"`.
178+
179+
`docs/Migration-UriCheckHostName.md`. Downstream zero sites. **#1997's four groups A-1…A-4 are all
180+
landed and the ticket is closed.**
181+
142182
## 2026-08-20 — #1997 group A-4: `UriParser` could not extend anything
143183

144184
**Gate: 17,689 / 38, 0 failed, 0 skipped** (+6, `Uri` 308 → 314). Graph 41/94. Fixture set

docs/Migration-UriCheckHostName.md

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# `Uri::CheckHostName`#1997 group A-2 (SR-AUD-151), and the scanner move that made it possible
5+
6+
**Purely additive: no existing signature, layout, vtable, `noexcept` specification or accepted input
7+
changed, and no consumer needs editing or rebuilding.** It lands under **SA-5**. It also **closes
8+
#1997**, whose four groups A-1…A-4 are now all landed.
9+
10+
## The defect
11+
12+
`System::UriHostNameType` has documented `CheckHostName` since the enum was ported, and **nothing in
13+
this runtime could produce a value of that type**. The member did not exist.
14+
15+
## The recorded cost was understated, and the measurement corrects it
16+
17+
#1997's own record priced group A-2 as:
18+
19+
> *"a new public module edge to reach `System::Net::IPAddress`, or a second address-literal parser
20+
> inside this module — the duplication #2354 spent a ticket removing"*
21+
22+
**The first of those is impossible rather than expensive.** `modules/net` declares
23+
`PUBLIC_DEPENDENCIES Collections.Core ComponentModel Core.Base Uri` — it depends on `Uri` **already**
24+
— so an edge from `modules/uri` to `System::Net::IPAddress` is a **cycle**, the dependency inversion
25+
`Guid.cpp` refused for cryptography. A first cut written against `IPAddress` was rejected by the
26+
module-boundary validator, and this is why.
27+
28+
**The route taken is a third one neither option named.** `modules/net`'s IPv4 and IPv6 scanners
29+
lived in an anonymous namespace in `IPAddress.cpp` and are **pure string-to-number scanners** — no
30+
platform call, no dependency on `IPAddress` itself. They moved verbatim to
31+
`modules/core/include/System/detail/IPAddressLiteral.hpp`. Both modules already depend on
32+
`Core.Base`, so:
33+
34+
* **the module graph does not change** — still **41 / 94**;
35+
* there is **one definition**, not two;
36+
* `modules/net/src/System/Net/IPAddress.cpp` is **−197 / +11 lines** and delegates.
37+
38+
`IPAddress`'s own `validatedScopeId`, `formatIPv4` and `formatIPv6` stayed behind: they are about
39+
that type rather than about the grammar. **The move is behaviour-preserving and that is measured
40+
rather than asserted**`SharpRuntimeTests_Net` runs 340/340 unchanged, which is the whole of
41+
`IPAddress`'s parse coverage.
42+
43+
## The classification, and where the order decides an answer
44+
45+
.NET's order (`Uri.cs:1286-1325`) is transcribed rather than rearranged, and **IPv4-before-DNS is
46+
the step that decides an answer rather than merely tidying**. `CheckHostName` calls
47+
`IPv4AddressHelper.IsValid(name, out end, false, false, false)`, whose `allowIPv6` and
48+
`unknownScheme` both being `false` selects **`ParseNonCanonical`** rather than the canonical
49+
dotted-quad grammar. So:
50+
51+
| Input | Result | Why |
52+
|---|---|---|
53+
| `"1"` | **IPv4** | a bare number is a non-canonical IPv4 — and also a perfectly good DNS label |
54+
| `"0x7F.1"` | **IPv4** | hex-prefixed short form |
55+
| `"3232235777"` | **IPv4** | single 32-bit value |
56+
| `"1.2.3.4.5"` | **Dns** | more than four parts is not IPv4; every label is legal |
57+
| `"example.com"` | **Dns** | |
58+
| `"[::1]"` | **IPv6** | |
59+
| `"::1"` | **IPv6** | see below |
60+
61+
Reordering IPv4 and DNS answers `Dns` for the first three and passes every other row.
62+
63+
## Two rules that are new with this member
64+
65+
**The label rules.** This port's `Uri` constructor only ever asked about the host's **characters**
66+
(#2359's DNS character set). `DomainNameHelper.IsValid` also requires that every label begin with an
67+
ASCII letter or digit, and that every label be **1..63** characters. Deriving `Dns` from the
68+
character set alone answers `Dns` for `"-x"`, for the empty inner label in `"a..b"` and for a
69+
64-character label. **A trailing dot is accepted and ends the walk**`"example.com."` is a valid
70+
DNS name, easy to get wrong in either direction because an empty final label would otherwise fail
71+
the 1..63 rule.
72+
73+
**An unbracketed IPv6 literal is still IPv6**, which reads like a bug until the reference is read:
74+
.NET's last resort retries `IPv6AddressHelper.IsValid($"[{name}]")` (`Uri.cs:1320-1324`). So
75+
`CheckHostName("::1")` is `IPv6` even though a `Uri` authority requires the brackets. **The two
76+
questions are different** — this one asks what a string *is*, not whether it may appear in an
77+
authority.
78+
79+
## One definition, not two
80+
81+
The constructor's host-character loop was **factored into `detail::hostCharactersAreValid`** and
82+
both callers now use it, so a host the constructor accepts cannot be one `CheckHostName` calls
83+
malformed *on characters*. Two grammars for one question is the **#2393** shape, where it was found
84+
only after a caller could construct a `Uri` this port's own `TryCreate` reported as invalid.
85+
86+
**They are still not the same question, and that is stated rather than left to be discovered:** the
87+
constructor does **not** apply the label rules, so `Uri("http://-x/")` parses while
88+
`CheckHostName("-x")` is `Unknown`. .NET has the same split, reaching `DomainNameHelper` by a
89+
different path with different flags. A case asserts both halves together.
90+
91+
## Evidence
92+
93+
Seven mutations, all caught. **M6 was NOT CAUGHT at first, and it found a defect in my test rather
94+
than in the code.** The "entire name must be consumed" property was asserted with `"[::1]junk"`
95+
which fails on the *front/back* guard, its last character being `k`, so it never enters the
96+
bracketed branch at all. A body measuring the literal to the **first** `]` instead of to the end of
97+
the name passed it anyway. The input that separates the two is bracketed at both ends with junk
98+
**inside**: `"[::1]]"`. Both rows are now asserted.
99+
100+
Gate: **17,694 / 38, 0 failed, 0 skipped** (+5; `SharpRuntimeTests_Uri` 314 → 319; `Net` unchanged
101+
at 340). Module graph **41 / 94**, unchanged. Negative fixture set **53 / 269**, unchanged — no
102+
spelling was outlawed. Downstream: **zero sites** in `cna` and `mobile-eggbert`, the member having
103+
not existed.
Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
// SPDX-License-Identifier: MIT
2+
// Copyright (c) Robert Vokac and contributors
3+
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
4+
#pragma once
5+
6+
/**
7+
* @file
8+
* @brief The one definition of this runtime's IPv4 and IPv6 literal scanners.
9+
*
10+
* MOVED HERE BY #1997 GROUP A-2, AND THE MOVE IS WHAT MADE THAT GROUP POSSIBLE AT ALL.
11+
* `Uri::CheckHostName` classifies a host by asking whether it is a valid IPv6 literal, a valid
12+
* IPv4 literal, or a valid DNS name (`Uri.cs:1286-1325`), and the first two answers lived in
13+
* `modules/net`'s `IPAddress.cpp` in an anonymous namespace.
14+
*
15+
* **THE OBVIOUS ROUTE IS NOT MERELY DEAR, IT IS A CYCLE.** `modules/net` declares
16+
* `PUBLIC_DEPENDENCIES ... Uri`, so an edge from `modules/uri` to `System::Net::IPAddress` would
17+
* invert an existing dependency -- the shape `Guid.cpp` refused for cryptography. #1997's own
18+
* record priced A-2 as *"a new public module edge to reach `System::Net::IPAddress`, or a second
19+
* address-literal parser inside this module"*; **the first of those is impossible rather than
20+
* expensive**, and the second is the duplication #2354 spent a ticket removing.
21+
*
22+
* So the scanners move to where both modules can already reach them. `modules/net` and
23+
* `modules/uri` both depend on `Core.Base` today, so **the module graph does not change**, and
24+
* there is exactly one definition rather than two.
25+
*
26+
* The bodies below are moved **verbatim**, comments included, from
27+
* `modules/net/src/System/Net/IPAddress.cpp`. They are pure string-to-number scanners: no platform
28+
* call, no allocation beyond `std::string`/`std::vector`, and no dependency on `IPAddress` itself
29+
* -- which is why they could move. `IPAddress`'s own `validatedScopeId`, `formatIPv4` and
30+
* `formatIPv6` stayed behind, because they are about that type rather than about the grammar.
31+
*/
32+
33+
#include <algorithm>
34+
#include <array>
35+
#include <charconv>
36+
#include <cstdint>
37+
#include <string>
38+
#include <vector>
39+
40+
namespace System::detail {
41+
42+
// Returns the numeric value of a hex digit (0-9/a-f/A-F), or a sentinel >= any
43+
// supported base (8/10/16) for a character that isn't a valid digit in any of them.
44+
inline int hexDigitValue(char ch) {
45+
if (ch >= '0' && ch <= '9') return ch - '0';
46+
if (ch >= 'a' && ch <= 'f') return ch - 'a' + 10;
47+
if (ch >= 'A' && ch <= 'F') return ch - 'A' + 10;
48+
return 255;
49+
}
50+
51+
// Verified against IPv4AddressHelper.Common.cs's ParseNonCanonical, which
52+
// IPAddressParser.cs's IPAddress.TryParse delegates to (requiring the entire string be
53+
// consumed, matching this function's semantics). Parses any canonical (4-part decimal)
54+
// or non-canonical (octal/hex-prefixed segments, "short forms" with fewer than 3 dots
55+
// where the last segment absorbs the remaining bytes, e.g. "0xFF.0xFFFFFF" or a single
56+
// "3232235777") IPv4 literal into a 32-bit host-order value.
57+
//
58+
// Replaces the previous sscanf("%u.%u.%u.%u%c", ...) implementation, which: (1) invoked
59+
// undefined behavior per C11 7.21.6.2p10 on a %u conversion whose value doesn't fit in
60+
// unsigned int (a long enough digit run); (2) accepted a leading '-' via %u's
61+
// implementation-defined sign handling, which real .NET rejects; (3) rejected octal/hex
62+
// segments and short forms that real .NET accepts.
63+
inline bool tryParseIPv4Groups(const std::string& s, uint32_t& outAddr) {
64+
uint32_t parts[3] = {0, 0, 0};
65+
uint64_t currentValue = 0;
66+
bool atLeastOneChar = false;
67+
int dotCount = 0;
68+
size_t current = 0;
69+
char ch = 0;
70+
71+
while (current < s.size()) {
72+
ch = s[current];
73+
currentValue = 0;
74+
int numberBase = 10;
75+
76+
if (ch == '0') {
77+
++current;
78+
atLeastOneChar = true;
79+
if (current < s.size()) {
80+
ch = s[current];
81+
if (ch == 'x' || ch == 'X') {
82+
numberBase = 16;
83+
++current;
84+
atLeastOneChar = false;
85+
} else {
86+
numberBase = 8;
87+
}
88+
}
89+
}
90+
91+
for (; current < s.size(); ++current) {
92+
ch = s[current];
93+
int digitValue = hexDigitValue(ch);
94+
if (digitValue >= numberBase) break;
95+
currentValue = currentValue * static_cast<uint64_t>(numberBase) + static_cast<uint64_t>(digitValue);
96+
if (currentValue > 0xFFFFFFFFULL) return false; // overflow past uint.MaxValue
97+
atLeastOneChar = true;
98+
}
99+
100+
if (current < s.size() && ch == '.') {
101+
if (dotCount >= 3 || !atLeastOneChar || currentValue > 0xFF) return false;
102+
parts[dotCount] = static_cast<uint32_t>(currentValue);
103+
++dotCount;
104+
atLeastOneChar = false;
105+
++current; // consume the dot
106+
continue;
107+
}
108+
break;
109+
}
110+
111+
if (!atLeastOneChar) return false; // empty segment, e.g. "1.1.1." or ""
112+
if (current != s.size()) return false; // trailing garbage (IPAddress.Parse requires full consumption)
113+
114+
switch (dotCount) {
115+
case 0: // e.g. "3232235777" -- the whole 32-bit value in one segment
116+
outAddr = static_cast<uint32_t>(currentValue);
117+
return true;
118+
case 1: // e.g. "192.11534091" -- parts[0].rest
119+
if (currentValue > 0xFFFFFFULL) return false;
120+
outAddr = (parts[0] << 24) | static_cast<uint32_t>(currentValue);
121+
return true;
122+
case 2: // e.g. "192.168.257" -- parts[0].parts[1].rest
123+
if (currentValue > 0xFFFFULL) return false;
124+
outAddr = (parts[0] << 24) | (parts[1] << 16) | static_cast<uint32_t>(currentValue);
125+
return true;
126+
case 3: // standard four-octet form
127+
if (currentValue > 0xFFULL) return false;
128+
outAddr = (parts[0] << 24) | (parts[1] << 16) | (parts[2] << 8) | static_cast<uint32_t>(currentValue);
129+
return true;
130+
default:
131+
return false;
132+
}
133+
}
134+
135+
inline std::vector<std::string> splitOn(const std::string& s, char sep) {
136+
std::vector<std::string> parts;
137+
size_t start = 0;
138+
while (true) {
139+
size_t pos = s.find(sep, start);
140+
if (pos == std::string::npos) {
141+
parts.push_back(s.substr(start));
142+
break;
143+
}
144+
parts.push_back(s.substr(start, pos - start));
145+
start = pos + 1;
146+
}
147+
return parts;
148+
}
149+
150+
inline bool parseHexGroup(const std::string& s, uint16_t& out) {
151+
if (s.empty() || s.size() > 4) return false;
152+
uint32_t value = 0;
153+
for (char c : s) {
154+
value <<= 4;
155+
if (c >= '0' && c <= '9') value |= static_cast<uint32_t>(c - '0');
156+
else if (c >= 'a' && c <= 'f') value |= static_cast<uint32_t>(c - 'a' + 10);
157+
else if (c >= 'A' && c <= 'F') value |= static_cast<uint32_t>(c - 'A' + 10);
158+
else return false;
159+
}
160+
out = static_cast<uint16_t>(value);
161+
return true;
162+
}
163+
164+
// Expands a list of ':'-separated group tokens into uint16 groups, handling
165+
// an embedded IPv4 dotted-quad as the last token (e.g. "ffff:192.168.1.1").
166+
inline bool expandGroups(const std::vector<std::string>& tokens, std::vector<uint16_t>& out) {
167+
for (size_t i = 0; i < tokens.size(); ++i) {
168+
const std::string& tok = tokens[i];
169+
if (tok.find('.') != std::string::npos) {
170+
if (i != tokens.size() - 1) return false;
171+
uint32_t v4;
172+
if (!tryParseIPv4Groups(tok, v4)) return false;
173+
out.push_back(static_cast<uint16_t>(v4 >> 16));
174+
out.push_back(static_cast<uint16_t>(v4 & 0xFFFF));
175+
} else {
176+
uint16_t g;
177+
if (!parseHexGroup(tok, g)) return false;
178+
out.push_back(g);
179+
}
180+
}
181+
return true;
182+
}
183+
184+
inline bool tryParseIPv6(const std::string& input, std::array<uint16_t, 8>& groups, uint32_t& scopeId) {
185+
std::string s = input;
186+
scopeId = 0;
187+
188+
size_t pctPos = s.find('%');
189+
if (pctPos != std::string::npos) {
190+
std::string scopeStr = s.substr(pctPos + 1);
191+
if (scopeStr.empty() || !std::all_of(scopeStr.begin(), scopeStr.end(), [](unsigned char c) { return std::isdigit(c) != 0; }))
192+
return false;
193+
// Verified against IPAddressParser.cs: real .NET parses the numeric scope ID
194+
// with uint.TryParse (non-throwing) and fails the whole address parse on
195+
// overflow. std::stoul here previously threw std::out_of_range -- an unrelated
196+
// std:: exception type, uncaught anywhere in this call chain -- for a
197+
// many-all-digit scope string exceeding unsigned long's range, violating
198+
// tryParseIPv6's (and TryParse's) "never throws" contract; it would also have
199+
// silently truncated any value between UINT32_MAX and ULONG_MAX when narrowed
200+
// to uint32_t instead of failing the parse.
201+
uint32_t parsedScope = 0;
202+
auto scopeResult = std::from_chars(scopeStr.data(), scopeStr.data() + scopeStr.size(), parsedScope);
203+
if (scopeResult.ec != std::errc() || scopeResult.ptr != scopeStr.data() + scopeStr.size())
204+
return false;
205+
scopeId = parsedScope;
206+
s = s.substr(0, pctPos);
207+
}
208+
209+
if (s.find(':') == std::string::npos) return false;
210+
211+
size_t dcPos = s.find("::");
212+
std::vector<uint16_t> allGroups;
213+
214+
if (dcPos != std::string::npos) {
215+
if (s.find("::", dcPos + 1) != std::string::npos) return false; // more than one "::"
216+
217+
std::string left = s.substr(0, dcPos);
218+
std::string right = s.substr(dcPos + 2);
219+
220+
std::vector<uint16_t> leftGroups, rightGroups;
221+
if (!left.empty() && !expandGroups(splitOn(left, ':'), leftGroups)) return false;
222+
if (!right.empty() && !expandGroups(splitOn(right, ':'), rightGroups)) return false;
223+
224+
size_t total = leftGroups.size() + rightGroups.size();
225+
if (total > 7) return false; // "::" must represent at least one group
226+
227+
allGroups = leftGroups;
228+
allGroups.resize(allGroups.size() + (8 - total), 0);
229+
allGroups.insert(allGroups.end(), rightGroups.begin(), rightGroups.end());
230+
} else {
231+
if (!expandGroups(splitOn(s, ':'), allGroups)) return false;
232+
if (allGroups.size() != 8) return false;
233+
}
234+
235+
if (allGroups.size() != 8) return false;
236+
std::copy(allGroups.begin(), allGroups.end(), groups.begin());
237+
return true;
238+
}
239+
240+
} // namespace System::detail

0 commit comments

Comments
 (0)