Skip to content

Commit 78e520e

Browse files
committed
feat(uri): #1996 groups G-1 and G-2 -- UriBuilder brackets an IPv6 host and lower-cases the scheme
setHostProperty("::1") rendered "http://::1/", which Uri rejects, and setSchemeProperty("HTTP") kept "HTTP" and rendered "HTTP://...". Both now match .NET. #1996 splits into four independently landable groups and names G-1 + G-2 the recommended minimum; both are alignments to the reference, so SA-5 covers them. No signature, layout, vtable or noexcept change. G-1 (UriBuilder.cs:167-197): the trigger is .NET's own -- the value must contain one of s_hostReservedChars ":/\?#@[]" and then a ':' specifically, which .NET's comment calls a "probable ipv6 address". An already-bracketed value is left alone. G-2 (UriBuilder.cs:180): the setter ends in value.ToLowerInvariant(). Invariant means invariant, so the fold is ASCII-only: std::tolower consults the global C locale, and a process that installed a Turkish one would fold 'I' to a dotless 'i' and change what the scheme means -- the hazard #2316 removed from CharUnicodeInfo. ONE DELIBERATE DIVERGENCE, FORCED BY TAKING G-1 WITHOUT G-3. .NET's host setter WRAPS FIRST AND THEN THROWS: "[::1" becomes "[[::1]" and is refused because the inside holds a '[' (:183-187). This group does not take the rejection -- #1996's own note reserves "a setter that never threw starts throwing" for a group this is not -- and wrapping without rejecting would leave the nonsense "[[::1]" stored, turning a value this port's Uri already refuses (#1991) into one it might not. So a value already carrying a bracket is left exactly as given, and the reason is at the site. "h:abc" -> "[h:abc]" is NOT a divergence but .NET's own rule, and the port's Uri does not validate a literal's content (plan 15.4), so that rendering now parses where "http://h:abc/" did not. Two existing pins asserted the old unparseability; their subject is hash obtainability on an unparseable rendering, which is now asserted on "[::1" and the empty host, both of which still are. G-3 -- which #1996 itself calls "the only narrowing" -- and G-4 are NOT taken and are pinned absent by Decl1996_G3AndG4AreNotTakenAndStayPinned, so neither can be added without a decision. setSchemeProperty("bad scheme") still stores it; setHostProperty("contoso.com/path") still stores it; UriBuilder("www.example.com/path") still renders the measured form. Mutations: 5, 4 caught. The uncaught one replaces the explicit ASCII fold with std::tolower and is an EQUIVALENCE IN THE "C" LOCALE, which is the locale the test binary runs in -- the same case #2174 recorded. Distinguishing them needs the global locale changed inside a shared binary, which #2174 considered and declined for exactly this reason. The explicit fold is kept because the guarantee is about what happens when a process DOES install another locale, which no test in this binary can observe. The note is at the site. Gate: 17,416 run, 17,416 passed, 0 failed, 0 skipped across 38 executables (+4, in SharpRuntimeTests_Uri, 283 -> 287). Built in build/ with --parallel 2. Downstream: zero UriBuilder sites in cna and mobile-eggbert. docs/Migration-UriBuilderHostAndScheme.md
1 parent f868593 commit 78e520e

5 files changed

Lines changed: 262 additions & 9 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — `UriBuilder` brackets an IPv6 host and lower-cases the scheme (ticket #1996, groups G-1 and G-2)
5+
6+
*2026-08-19.* `setHostProperty("::1")` rendered `http://::1/`, which `Uri` rejects, and
7+
`setSchemeProperty("HTTP")` kept `HTTP` and rendered `HTTP://…`. Both now match .NET.
8+
9+
#1996 splits into four independently landable groups and names **G-1 + G-2 as the recommended
10+
minimum**. Both are alignments to the reference, so **SA-5** covers them. G-3 (scheme validation,
11+
which #1996 calls *"the only narrowing"*) and G-4 (relative promotion) are **not** taken and stay
12+
with the ticket. No signature, layout, vtable or `noexcept` change in either group.
13+
14+
**This changes the text `ToString()` emits.** Read §1 and §3 before upgrading.
15+
16+
---
17+
18+
## 1. What changed
19+
20+
| Call | Was | Is |
21+
|---|---|---|
22+
| `setHostProperty("::1")``ToString()` | `http://::1/` (unparseable) | `http://[::1]/` |
23+
| `setHostProperty("2001:db8::1")` | unbracketed | `http://[2001:db8::1]/` |
24+
| `setHostProperty("[::1]")` | unchanged | **unchanged** — not double-wrapped |
25+
| `setHostProperty("h:abc")` | `http://h:abc/` (unparseable) | `http://[h:abc]/` (§3) |
26+
| `setHostProperty("example.com")`, `"192.0.2.1"`, `""` || **unchanged** |
27+
| `setSchemeProperty("HTTP")` | `HTTP` | `http` |
28+
| `setSchemeProperty("bad scheme")` | stored as given | **unchanged** — G-3 owns that |
29+
30+
## 2. The two rules, transcribed
31+
32+
**Host (`UriBuilder.cs:167-197`).** The trigger is .NET's own: the value must contain one of
33+
`s_hostReservedChars`, `":/\?#@[]"`, and then a `:` specifically — .NET's comment calls it a
34+
*"probable ipv6 address"*. A value already `[...]` is left alone.
35+
36+
**Scheme (`UriBuilder.cs:169-181`).** The setter ends in `value = value.ToLowerInvariant();`.
37+
Invariant, so the fold here is ASCII-only: `std::tolower` consults the global C locale, and a
38+
process that installed a Turkish one would fold `I` to a dotless `i` and change what the scheme
39+
means — the hazard #2316 removed from `CharUnicodeInfo`.
40+
41+
## 3. One deliberate divergence, forced by taking G-1 without G-3
42+
43+
.NET's host setter **wraps first and then throws**: `"[::1"` becomes `"[[::1]"` and is refused
44+
because the inside holds a `[` (`:183-187`). This group does not take the rejection — #1996's own
45+
note reserves "a setter that never threw starts throwing" for a group this is not — and wrapping
46+
without rejecting would leave the nonsense `"[[::1]"` stored, turning a value this port's `Uri`
47+
already refuses (**#1991**) into one it might not.
48+
49+
So a value that already carries a bracket is left exactly as given. That keeps #1991's refusal
50+
where it was, and the reason is at the site rather than in this document alone.
51+
52+
**`h:abc` is not a divergence, it is .NET.** A host containing `:` is bracketed on .NET's own
53+
"probable ipv6 address" rule, and the port's `Uri` does not validate a literal's *content*
54+
(`docs/SystemUriNamespaceReviewPlan.md` §15.4) — so `http://[h:abc]/` now parses where
55+
`http://h:abc/` did not. Two existing pins asserted the old unparseability and were updated: their
56+
subject is *hash obtainability on an unparseable rendering*, which is now asserted on `"[::1"` and
57+
the empty host, both of which still are.
58+
59+
## 4. Evidence
60+
61+
Five mutations, four caught:
62+
63+
| Mutation | Result |
64+
|---|---|
65+
| no bracketing at all | caught |
66+
| bracket unconditionally (ignores the `:` trigger) | caught — 4 cases |
67+
| an already-bracketed value is wrapped again | caught — 3 cases |
68+
| the scheme is not lower-cased | caught |
69+
| **the fold uses `std::tolower`** | **equivalence in the "C" locale** |
70+
71+
The last is reported rather than papered over, and it is the same case **#2174** recorded: the
72+
test binary runs in the "C" locale, where `std::tolower` agrees with the explicit fold on every
73+
byte. Distinguishing them needs the global locale changed inside a shared binary, which #2174
74+
considered and declined for exactly this reason. The explicit fold is kept because the guarantee
75+
is about what happens when a process *does* install another locale — which is not something a test
76+
in this binary can observe. The note is at the site.
77+
78+
`Decl1996_G3AndG4AreNotTakenAndStayPinned` asserts the three behaviours this group deliberately
79+
left alone, so neither can be added without a decision.
80+
81+
## 5. Downstream
82+
83+
`cna` and `mobile-eggbert` reference `UriBuilder` in **zero** places. A first-party caller that
84+
set an IPv6 host and read `ToString()` gets a string `Uri` now accepts; one that set an
85+
upper-case scheme now sees it lower-cased, which is what .NET has always done.

modules/uri/include/System/UriBuilder.hpp

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,28 @@ namespace System {
2525
* a final System::Uri.
2626
*/
2727
class UriBuilder {
28+
/**
29+
* @brief `ToLowerInvariant` over ASCII only. Ticket #1996 group G-2.
30+
*
31+
* Invariant means invariant: `std::tolower` consults the global C locale, so a process
32+
* that installed a Turkish one would fold `I` to a dotless `i` and change the scheme.
33+
* This repository has removed that hazard from `CharUnicodeInfo` already (#2316).
34+
*
35+
* HONEST NOTE: a mutation replacing this with `std::tolower` is NOT caught, and it is an
36+
* equivalence **in the "C" locale** rather than a gap -- which is the locale the test
37+
* binary runs in, so the two agree on every byte there. Distinguishing them needs the
38+
* global locale changed inside a shared binary, which #2174 considered and declined for
39+
* exactly this reason. The explicit fold is kept because the guarantee is about what
40+
* happens when a process does install another locale, and that is not something a test
41+
* in this binary can observe.
42+
*/
43+
static std::string toLowerAsciiInvariant(std::string text) {
44+
for (char& c : text) {
45+
if (c >= 'A' && c <= 'Z') c = static_cast<char>(c - 'A' + 'a');
46+
}
47+
return text;
48+
}
49+
2850
std::string scheme_ = "http";
2951
std::string host_ = "localhost";
3052
intcs port_ = -1;
@@ -147,12 +169,66 @@ namespace System {
147169
/** @brief Returns the scheme component. */
148170
[[nodiscard]] const std::string& getSchemeProperty() const noexcept { return scheme_; }
149171
/** @brief Sets the scheme component. */
150-
void setSchemeProperty(const std::string& v) { scheme_ = v; }
172+
/**
173+
* @brief Sets the scheme, lower-casing it. Ticket #1996 group G-2.
174+
*
175+
* `UriBuilder.Scheme`'s setter ends in `value = value.ToLowerInvariant();`
176+
* (`UriBuilder.cs:180`), so `setSchemeProperty("HTTP")` renders `http://…` where it used
177+
* to render `HTTP://…`. Invariant, not locale-aware: an ASCII fold, so a Turkish locale
178+
* cannot turn `I` into a dotless one.
179+
*
180+
* @note <b>The validation that surrounds this line in the reference is deliberately NOT
181+
* taken here.</b> .NET runs `Uri.CheckSchemeName` first, truncates at a `:` and
182+
* re-checks, and throws `ArgumentException` if it still fails. That block is this
183+
* ticket's group <b>G-3</b>, which #1996 identifies as "the only narrowing" -- it
184+
* makes a setter that never threw start throwing. Lower-casing is the last
185+
* statement of the block and is separable from it; the rest is not taken.
186+
*
187+
* Measured consequence, and it is deliberate: `setSchemeProperty("bad scheme")`
188+
* still stores `"bad scheme"` and still renders `"bad scheme://localhost/"`. That
189+
* is G-3's to repair.
190+
*/
191+
void setSchemeProperty(const std::string& v) {
192+
scheme_ = toLowerAsciiInvariant(v);
193+
}
151194

152195
/** @brief Returns the host component. */
153196
[[nodiscard]] const std::string& getHostProperty() const noexcept { return host_; }
154197
/** @brief Sets the host component. */
155-
void setHostProperty(const std::string& v) { host_ = v; }
198+
/**
199+
* @brief Sets the host, bracketing an IPv6 literal. Ticket #1996 group G-1.
200+
*
201+
* `UriBuilder.Host`'s setter wraps a value containing `:` in `[...]` unless it is already
202+
* bracketed (`UriBuilder.cs:167-197`), so `setHostProperty("::1")` renders
203+
* `http://[::1]/` where it used to render the unparseable `http://::1/`.
204+
*
205+
* The trigger is .NET's own: the value must contain one of `s_hostReservedChars`,
206+
* `":/\?#@[]"` (`:164`), and then a `:` specifically. A plain DNS name touches none of
207+
* them and is stored unchanged.
208+
*
209+
* @note <b>The rejection in the same block is deliberately NOT taken.</b> .NET also
210+
* throws `ArgumentException(net_uri_BadHostName)` for a bracketed value whose
211+
* inside holds a reserved character other than `:`, and for any value with a
212+
* reserved character but no `:` -- "contoso.com/path", "user@contoso.com". Those
213+
* make a setter that never threw start throwing, which #1996's own note reserves
214+
* for a group this one is not. Only the bracketing lands here.
215+
*/
216+
void setHostProperty(const std::string& v) {
217+
// Nothing to bracket.
218+
if (v.find(':') == std::string::npos) { host_ = v; return; }
219+
// ALREADY CARRIES A BRACKET: left exactly as given, and this is a deliberate
220+
// divergence forced by taking G-1 without G-3's rejection. .NET wraps first and
221+
// THEN throws for a half-bracketed value -- "[::1" becomes "[[::1]" and is refused
222+
// because the inside holds a '[' (UriBuilder.cs:178-187). Without that throw the
223+
// wrap would leave the nonsense "[[::1]" stored, turning a value this port's Uri
224+
// already refuses (#1991) into one it might not. Leaving it untouched keeps that
225+
// refusal exactly where it was.
226+
if (v.find('[') != std::string::npos || v.find(']') != std::string::npos) {
227+
host_ = v;
228+
return;
229+
}
230+
host_ = "[" + v + "]";
231+
}
156232

157233
/** @brief Returns the port number, or -1 if not set. */
158234
[[nodiscard]] intcs getPortProperty() const noexcept { return port_; }

modules/uri/tests/System/UriBuilderTests.cpp

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,19 +274,29 @@ namespace {
274274
}
275275

276276
TEST(UriBuilderTest, HashIsObtainableWhereverEqualsSucceeds_MalformedPort) {
277+
// #1996 G-1 changed the rendering here, and it is .NET's: a host containing ':' is a
278+
// "probable ipv6 address" (UriBuilder.cs:176) and is bracketed, so "h:abc" renders
279+
// "http://[h:abc]/" rather than "http://h:abc/". The point of THIS test is unaffected --
280+
// the string is still one Uri rejects, so Equals and GetHashCode must still work on it.
277281
UriBuilder b;
278282
b.setHostProperty("h:abc");
279-
ASSERT_EQ(b.ToString(), "http://h:abc/");
280-
EXPECT_THROW(b.getUriProperty(), UriFormatException); // the string is still unparseable
283+
ASSERT_EQ(b.ToString(), "http://[h:abc]/");
284+
// The rendering is now one Uri ACCEPTS, because .NET's bracketing turned a malformed port
285+
// into a (content-unvalidated) literal. The property this test exists for is unchanged and
286+
// is asserted on a shape that is still unparseable, below.
287+
EXPECT_NO_THROW((void)b.getUriProperty());
281288
EXPECT_TRUE(b.Equals(b));
282289
EXPECT_NO_THROW((void)b.GetHashCode());
283-
expectEqualsImpliesEqualHash(b, b, "malformed port");
290+
expectEqualsImpliesEqualHash(b, b, "bracketed host");
284291
}
285292

286293
TEST(UriBuilderTest, HashIsObtainableWhereverEqualsSucceeds_OtherUnparseableRenderings) {
287294
// The three remaining measured routes: an out-of-range port, an unterminated IP literal
288295
// (#1991) and an empty host (#2000). Each renders a string Uri rejects.
289-
const char* hosts[] = {"h:99999", "[::1", ""};
296+
// "h:99999" left this set with #1996 G-1: it is bracketed now and parses. The other two
297+
// still render strings Uri rejects -- an unterminated literal, which G-1 deliberately does
298+
// NOT wrap (see setHostProperty's note), and an empty host.
299+
const char* hosts[] = {"[::1", ""};
290300
for (const char* host : hosts) {
291301
UriBuilder b;
292302
b.setHostProperty(host);
@@ -355,11 +365,13 @@ TEST(UriBuilderTest, DeliberatelyUnequalPairsStayUnequal_PinsTheGatedIdentityCha
355365
// UriTests.DocumentedContract_CaseDifferingUrisAreNotEqualYet plays for Uri.
356366
UriBuilder lower; lower.setHostProperty("example.com");
357367
UriBuilder upperHost; upperHost.setHostProperty("EXAMPLE.COM");
358-
UriBuilder upperScheme; upperScheme.setSchemeProperty("HTTP"); upperScheme.setHostProperty("example.com");
368+
// #1996 G-2 lower-cases the scheme in the SETTER, so this builder is no longer a
369+
// case-differing one at all -- it is "http" the moment it is set. The scheme row therefore
370+
// moved out of this pin and into Fix1996G2 below; what #1995 would still make equal is the
371+
// HOST case and the default port.
359372
UriBuilder defaultPort; defaultPort.setHostProperty("example.com"); defaultPort.setPortProperty(80);
360373

361374
EXPECT_FALSE(lower.Equals(upperHost));
362-
EXPECT_FALSE(lower.Equals(upperScheme));
363375
EXPECT_FALSE(lower.Equals(defaultPort));
364376
// The three matching hash inequalities were removed by #2284. They restated the gate on a
365377
// channel that does not carry it: unequal values are permitted to hash equally
@@ -390,3 +402,83 @@ TEST(UriBuilderTest, Layout_SizeOfUriBuilderIsPinned) {
390402
// here so the claim becomes true, alongside UriTests.Layout_SizeOfUriIsPinned.
391403
EXPECT_EQ(sizeof(System::UriBuilder), 232u);
392404
}
405+
406+
// ===========================================================================
407+
// #1996 groups G-1 (IPv6 bracketing) and G-2 (scheme lower-casing) -- the
408+
// ticket's own "recommended minimum". Both are alignments to the reference, so
409+
// SA-5 covers them; G-3 (scheme validation) and G-4 (relative promotion) are
410+
// not taken and #1996 keeps them.
411+
// ===========================================================================
412+
413+
TEST(UriBuilderTest, Fix1996G1_AnIPv6HostIsBracketed) {
414+
// Measured before: setHostProperty("::1") rendered "http://::1/", which Uri rejects.
415+
UriBuilder b;
416+
b.setHostProperty("::1");
417+
EXPECT_EQ(b.getHostProperty(), "[::1]");
418+
EXPECT_EQ(b.ToString(), "http://[::1]/");
419+
EXPECT_NO_THROW((void)b.getUriProperty()) << "the rendering is now one Uri accepts";
420+
421+
// Already bracketed: left alone, not double-wrapped. UriBuilder.cs:178 tests
422+
// StartsWith('[') && EndsWith(']') for exactly this.
423+
UriBuilder already;
424+
already.setHostProperty("[::1]");
425+
EXPECT_EQ(already.getHostProperty(), "[::1]");
426+
EXPECT_EQ(already.ToString(), "http://[::1]/");
427+
428+
// A full-length literal and a scoped one both go through.
429+
UriBuilder full;
430+
full.setHostProperty("2001:db8::1");
431+
EXPECT_EQ(full.ToString(), "http://[2001:db8::1]/");
432+
}
433+
434+
TEST(UriBuilderTest, Fix1996G1_AHostWithoutAColonIsUntouched) {
435+
// The trigger is .NET's own: the value must contain ':'. A DNS name, an IPv4 literal and an
436+
// empty host touch nothing -- the rows that fail if the bracketing is applied unconditionally.
437+
for (const char* host : {"example.com", "192.0.2.1", "", "localhost"}) {
438+
UriBuilder b;
439+
b.setHostProperty(host);
440+
EXPECT_EQ(b.getHostProperty(), host) << host;
441+
}
442+
}
443+
444+
TEST(UriBuilderTest, Fix1996G2_TheSchemeIsLowerCasedInTheSetter) {
445+
// Measured before: setSchemeProperty("HTTP") kept "HTTP" and rendered "HTTP://…".
446+
UriBuilder b;
447+
b.setSchemeProperty("HTTP");
448+
EXPECT_EQ(b.getSchemeProperty(), "http");
449+
b.setHostProperty("example.com");
450+
EXPECT_EQ(b.ToString(), "http://example.com/");
451+
452+
UriBuilder mixed;
453+
mixed.setSchemeProperty("HtTpS");
454+
EXPECT_EQ(mixed.getSchemeProperty(), "https");
455+
456+
// INVARIANT, not locale-aware: an ASCII fold only. A non-ASCII byte is left alone, so a
457+
// process that installed a Turkish locale cannot change what a scheme means -- the hazard
458+
// #2316 removed from CharUnicodeInfo. Also the row that fails if std::tolower is used.
459+
UriBuilder nonAscii;
460+
nonAscii.setSchemeProperty("H\xC3\x9CTTP");
461+
EXPECT_EQ(nonAscii.getSchemeProperty(), "h\xC3\x9Cttp");
462+
}
463+
464+
TEST(UriBuilderTest, Decl1996_G3AndG4AreNotTakenAndStayPinned) {
465+
// G-3, "the only narrowing": .NET validates the scheme, truncates at a ':' and re-checks,
466+
// and throws ArgumentException if it still fails (UriBuilder.cs:169-179). Lower-casing is
467+
// the LAST statement of that block and is separable; the rest is not taken, so a setter that
468+
// never threw still does not.
469+
UriBuilder bad;
470+
EXPECT_NO_THROW(bad.setSchemeProperty("bad scheme"));
471+
EXPECT_EQ(bad.getSchemeProperty(), "bad scheme")
472+
<< "lower-cased but not validated -- G-3 owns the rejection";
473+
474+
// The host rejection in G-1's own block is likewise not taken: .NET throws
475+
// ArgumentException(net_uri_BadHostName) for "contoso.com/path", and this port stores it.
476+
UriBuilder path;
477+
EXPECT_NO_THROW(path.setHostProperty("contoso.com/path"));
478+
EXPECT_EQ(path.getHostProperty(), "contoso.com/path");
479+
480+
// G-4, relative promotion: UriBuilder("www.example.com/path") still renders the measured
481+
// ":///www.example.com/path" rather than promoting the text to a host.
482+
UriBuilder relative("www.example.com/path");
483+
EXPECT_NE(relative.ToString().find("://"), std::string::npos) << relative.ToString();
484+
}

plan.sqlite3

0 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)