Skip to content

Commit 2562ace

Browse files
committed
fix(uri): the constructor and TryCreate share one absolute-URI grammar (#2393)
This port had TWO absolute-URI grammars in one type. Uri::Uri(const std::string&) called parse() and NEVER checked isAbsoluteUri_, while the (string, UriKind) overload did -- and TryCreate goes through that overload. So Uri("://example.com/") SUCCEEDED while TryCreate("://example.com/", UriKind::Absolute, u) returned FALSE: a caller could construct a Uri this port's own TryCreate says is not an absolute URI. Reachable from ordinary code, because a UriBuilder with an empty scheme renders exactly that string and getUriProperty() is Uri(ToString()). THE DIRECTION WAS DERIVED, NOT CHOSEN. .NET has one grammar by construction: new Uri(s) is CreateThis(s, false, UriKind.Absolute) (Uri.cs:424-429), TryCreate(s, Absolute, out u) is CreateHelper(s, false, UriKind.Absolute) (UriExt.cs:223-227), and CreateThis throws exactly what CreateHelper returns null for. The repair is one line. THE NARROWING IS WIDER THAN THE TICKET DESCRIBED, and is stated plainly rather than left to be discovered: the constructor no longer accepts ANY relative reference. Relative-URI support itself is untouched -- only the default kind moved, onto .NET's, and the migration is to name it. Measured: zero real consumer sites (cna's single System::Uri match is inside a comment), zero in mobile-eggbert, and 51 first-party constructions across 24 tests, each keeping its coverage through the two-argument constructor. Two shipped assertions were inverted because they asserted the defect. One consequence moved three times in a day and is recorded: #2391's mutation M1 was an equivalence, then measured wrong BECAUSE OF THIS DEFECT, and is an equivalence again now that the second grammar is gone. The original reasoning was right and was defeated only by a bug one layer down. Four mutations, all caught. The pin asserts the equivalence over an 11-row corpus rather than the one reported string. Gate: 17,559 run, 17,559 passed, 0 failed, 0 skipped across 38 executables (+1). Build directory: build/ only, --parallel 2 throughout.
1 parent b3b8e67 commit 2562ace

8 files changed

Lines changed: 267 additions & 78 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
# Migration — `Uri(std::string)` requires an absolute URI (#2393)
3+
4+
Ticket **#2393**, landed 2026-08-19 under SA-5 (*"aligning to the reference is ordinary work …
5+
including where a call that succeeds today starts throwing"*).
6+
7+
## The defect
8+
9+
This port had **two absolute-URI grammars in one type**:
10+
11+
```cpp
12+
Uri("://example.com/") // SUCCEEDED
13+
Uri::TryCreate("://example.com/", UriKind::Absolute, u) // returned FALSE
14+
```
15+
16+
`Uri::Uri(const std::string&)` called `parse()` and **never checked `isAbsoluteUri_`**, while the
17+
`(string, UriKind)` overload did — and `TryCreate` goes through that overload. So a caller could
18+
construct a `Uri` that this port's own `TryCreate` says is not an absolute URI.
19+
20+
**Reachable from ordinary code**: a `UriBuilder` with an empty scheme (accepted since #1996 G-3)
21+
renders `"://example.com/"`, and `UriBuilder::getUriProperty()` is `Uri(ToString())`.
22+
23+
## The direction was derived, not chosen
24+
25+
.NET has **one** grammar, by construction:
26+
27+
* `new Uri(s)` is `CreateThis(s, false, UriKind.Absolute)` (`Uri.cs:424-429`);
28+
* `TryCreate(s, UriKind.Absolute, out u)` is `CreateHelper(s, false, UriKind.Absolute)`
29+
(`UriExt.cs:223-227`);
30+
* `CreateThis` throws exactly what `CreateHelper` returns `null` for (`UriExt.cs:18-26`).
31+
32+
So the constructor was wrong and `TryCreate` was right. The repair is one line: delegate.
33+
34+
## The narrowing is wider than the ticket described — say so plainly
35+
36+
The reported symptom was one odd string. The actual change is that **the one-argument constructor no
37+
longer accepts *any* relative reference**:
38+
39+
```cpp
40+
Uri u("/relative/path"); // used to construct silently; now throws UriFormatException
41+
Uri u("a?b"); // likewise
42+
Uri u("#fragment"); // likewise
43+
```
44+
45+
**Migration**: name the kind.
46+
47+
```cpp
48+
Uri u("/relative/path", UriKind::RelativeOrAbsolute);
49+
```
50+
51+
Relative-URI support is **not** removed — every component accessor, the query/fragment split, and
52+
resolution against a base all behave exactly as before. Only the *default kind* of the
53+
one-argument constructor moved, and it moved onto .NET's.
54+
55+
**Measured impact**: **zero** real sites in `cna` (its single `System::Uri` match is inside a
56+
comment) and **zero** in `mobile-eggbert`. First-party, **51 constructions across 24 tests** needed
57+
the kind spelled out; every one of those tests keeps its coverage through the two-argument
58+
constructor, because they were testing *relative-URI behaviour*, not the constructor's default.
59+
60+
Two shipped assertions were **inverted** rather than adapted, because they asserted the defect:
61+
62+
* `Decl1997A3_BothOverloadsResolveAgainstAbsoluteNotRelativeOrAbsolute` ended with
63+
`EXPECT_NO_THROW(Uri("/relative/path"))` and called that *"what makes the distinction observable
64+
rather than theoretical"*. It was observable, and it was the defect. The case now asserts the
65+
throw, and keeps its real subject by contrasting against `UriKind::RelativeOrAbsolute`.
66+
* `Fix2391_TheComparandKindIsRelativeOrAbsoluteAndItIsLoadBearing` — see below.
67+
68+
## A consequence worth following, because it moved twice in one day
69+
70+
#2391 mutation **M1** (the comparand kind `Absolute` instead of `RelativeOrAbsolute`) has now
71+
changed status **three times**, and the sequence is the point:
72+
73+
1. First recorded as an **unobservable equivalence**, reasoning that `getUriProperty()` is always
74+
absolute, so a relative comparand can never compare equal and both spellings return `false`.
75+
2. Then measured **wrong**, because of *this* defect: the constructor accepted `"://example.com/"`
76+
while `TryCreate(Absolute)` refused it, so `self` could be a `Uri` the strict comparand parse
77+
would reject, and the two spellings disagreed. The case was rewritten to pin that.
78+
3. Now an **equivalence again** — #2393 removed the second grammar, so `self` can no longer be such
79+
a `Uri`; the builder throws before any comparand is parsed.
80+
81+
The original reasoning was right; it was defeated only by a bug one layer down. The case is
82+
renamed `Decl2391_TheComparandKindIsRelativeOrAbsoluteAndIsNowAnEquivalence` and records all three
83+
steps, so the line is kept because .NET writes it, not because it is load-bearing.
84+
85+
## Tests
86+
87+
`Fix2393_TheConstructorAndTryCreateShareOneGrammar` asserts the **equivalence over an 11-row
88+
corpus** — empty scheme, absolute-path, relative-path and network-path references, query-only,
89+
fragment-only, `":"`, `""`, and three genuinely absolute URIs — rather than on the one reported
90+
string, because a repair that fixed only the reported example would pass a single-row test.
91+
92+
## Mutation testing
93+
94+
Four mutations, all caught:
95+
96+
| # | Mutation | Caught by |
97+
|---|---|---|
98+
| M1 | revert to the two grammars | the equivalence pin + `Decl1997A3` + `Decl2391` |
99+
| M2 | delegate to `RelativeOrAbsolute` | the same |
100+
| M3 | delegate to `Relative` | broadly, including `UriBuilderTest.ToUri` |
101+
| M4 | the kind overload stops enforcing `Absolute` | the same set |

modules/uri/include/System/Uri.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,20 @@ namespace System {
8787
* would report the scheme's default port for a host that does not exist,
8888
* while `"file:///path"` (RFC 8089) and every hierarchical scheme with no
8989
* default-port entry keep an empty host with `Port == -1` and are accepted.
90+
*
91+
* @note **This constructor requires an ABSOLUTE URI** and throws `UriFormatException` for
92+
* a relative reference — ticket **#2393**, 2026-08-19. It is
93+
* `CreateThis(uriString, false, UriKind.Absolute)` in .NET (`Uri.cs:424-429`), the same
94+
* grammar `TryCreate(s, UriKind::Absolute, u)` uses, so the two accept exactly the same
95+
* strings and the constructor throws precisely where `TryCreate` returns `false`.
96+
*
97+
* **This is a narrowing.** It used to call `parse()` and never check, so it silently
98+
* accepted every relative reference — and, because the sibling overload *did* check, this
99+
* port carried **two absolute-URI grammars**: `Uri("://example.com/")` succeeded while
100+
* `TryCreate("://example.com/", UriKind::Absolute, u)` returned `false`. To construct a
101+
* relative reference, name the kind:
102+
* `Uri("/relative/path", UriKind::RelativeOrAbsolute)`. Relative-URI support itself is
103+
* unchanged. See `docs/Migration-UriConstructorRequiresAnAbsoluteUri.md`.
90104
*/
91105
explicit Uri(const std::string& uriString);
92106

modules/uri/src/System/Uri.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,21 @@ void Uri::parse(const std::string& rawUriString) {
405405
// Constructors
406406
// ---------------------------------------------------------------------------
407407

408-
Uri::Uri(const std::string& uriString) {
409-
parse(uriString);
410-
}
408+
// Ticket #2393. .NET's one-argument constructor is `CreateThis(uriString, false,
409+
// UriKind.Absolute)` (Uri.cs:424-429) and TryCreate(s, UriKind.Absolute, out u) is
410+
// `CreateHelper(uriString, false, UriKind.Absolute)` (UriExt.cs:223-227) -- the SAME grammar, so
411+
// in .NET the constructor throws exactly where TryCreate returns false.
412+
//
413+
// This port had TWO grammars: the body below called parse() and never checked isAbsoluteUri_,
414+
// while the (string, UriKind) overload did. So `Uri("://example.com/")` SUCCEEDED while
415+
// `Uri::TryCreate("://example.com/", UriKind::Absolute, u)` returned FALSE -- a caller could
416+
// construct a Uri this port's own TryCreate says is not an absolute URI. Reachable from ordinary
417+
// code: a UriBuilder with an empty scheme (accepted since #1996 G-3) renders "://example.com/",
418+
// and UriBuilder::getUriProperty() is Uri(ToString()).
419+
//
420+
// Delegating is the whole repair, and it is a NARROWING: a relative string that used to construct
421+
// silently now throws UriFormatException, which is what .NET does.
422+
Uri::Uri(const std::string& uriString) : Uri(uriString, UriKind::Absolute) {}
411423

412424
Uri::Uri(const std::string& uriString, UriKind uriKind) {
413425
// Ticket #1992 (SR-AUD-145b): reject a value outside the enum's declared domain BEFORE

modules/uri/tests/System/UriBuilderTests.cpp

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -404,38 +404,49 @@ TEST(UriBuilderTest, Fix2391_TheComparandIsAStringSoAnUnparseableOTHERIsMerelyUn
404404
<< "an unparseable SELF throws, because the Uri property does";
405405
}
406406

407-
TEST(UriBuilderTest, Fix2391_TheComparandKindIsRelativeOrAbsoluteAndItIsLoadBearing) {
408-
// .NET's comparand branch uses UriKind.RelativeOrAbsolute, NOT Absolute
409-
// (Uri.cs, the `comparand is string` branch). Absolute is the plausible wrong choice -- it is
410-
// what #1997 A-3 picked for the CREATION overloads, and the two questions have different
411-
// answers.
407+
TEST(UriBuilderTest, Decl2391_TheComparandKindIsRelativeOrAbsoluteAndIsNowAnEquivalence) {
408+
// .NET's comparand branch uses UriKind.RelativeOrAbsolute, NOT Absolute (Uri.cs, the
409+
// `comparand is string` branch), and this port transcribes that. HONEST RECORD, and it has
410+
// moved twice in one day:
412411
//
413-
// A first cut of this test recorded the kind as an unobservable equivalence, reasoning that
414-
// `getUriProperty()` is always absolute so a relative comparand can never be equal. MEASURED,
415-
// that reasoning is wrong at its premise: this port's `Uri(std::string)` constructor ACCEPTS
416-
// "://example.com/" while `TryCreate(s, UriKind::Absolute)` REJECTS it, so `self` can be a
417-
// Uri the strict comparand parse would refuse -- and then the two spellings disagree.
412+
// * The first cut of this case recorded the kind as an unobservable EQUIVALENCE, reasoning
413+
// that getUriProperty() is always ABSOLUTE, so a relative comparand can never compare
414+
// equal and both spellings return false.
415+
// * That reasoning was then measured WRONG, because this port's Uri(std::string)
416+
// constructor ACCEPTED "://example.com/" while its own TryCreate(s, Absolute) REJECTED
417+
// it -- two grammars in one type. `self` could therefore be a Uri the strict comparand
418+
// parse would refuse, and the spellings disagreed. The case was rewritten to pin that.
419+
// * #2393 REMOVED THE SECOND GRAMMAR. Uri(std::string) now delegates to
420+
// Uri(s, UriKind::Absolute), which is what .NET's does (Uri.cs:424-429), so the
421+
// constructor and TryCreate accept exactly the same strings -- and the original
422+
// reasoning becomes true. The kind is an equivalence again, this time for a reason that
423+
// holds by construction rather than by luck.
424+
//
425+
// So: mutating RelativeOrAbsolute to Absolute is a mutation NO TEST CAN CATCH, and the line
426+
// is kept as RelativeOrAbsolute because that is what .NET writes, not because it is
427+
// load-bearing. Simplifying it would be simplifying the reference rather than porting it.
418428
UriBuilder emptyScheme;
419429
emptyScheme.setSchemeProperty(""); // accepted since #1996 G-3
420430
emptyScheme.setHostProperty("example.com");
421431
ASSERT_EQ(emptyScheme.ToString(), "://example.com/");
422432

423-
// Reachability, measured: the two kinds really do disagree on a string a builder can render.
433+
// The premise, measured: the two kinds still disagree on this string, so the branch is
434+
// reachable and the equivalence is not vacuous.
424435
std::shared_ptr<System::Uri> parsed;
425436
EXPECT_TRUE(System::Uri::TryCreate(emptyScheme.ToString(),
426437
System::UriKind::RelativeOrAbsolute, parsed));
427438
EXPECT_FALSE(System::Uri::TryCreate(emptyScheme.ToString(),
428439
System::UriKind::Absolute, parsed));
429440

430-
// THIS is the assertion that discriminates. With RelativeOrAbsolute the comparand parses and
431-
// the builder is equal to itself; with Absolute the parse fails and Equals returns false,
432-
// making a builder unequal to ITSELF.
433-
EXPECT_TRUE(emptyScheme.Equals(emptyScheme))
434-
<< "a builder must be equal to itself wherever its own Uri is obtainable";
435-
436-
// Still unequal to a different builder, so the case above is not passing for a trivial reason.
441+
// And here is why it no longer discriminates: `self` can no longer BE such a Uri. The builder
442+
// throws before any comparand is parsed, so neither spelling is ever reached.
437443
UriBuilder good; good.setHostProperty("example.com");
438-
EXPECT_FALSE(emptyScheme.Equals(good));
444+
EXPECT_THROW((void)emptyScheme.getUriProperty(), UriFormatException);
445+
EXPECT_THROW((void)emptyScheme.Equals(emptyScheme), UriFormatException);
446+
EXPECT_THROW((void)emptyScheme.Equals(good), UriFormatException);
447+
448+
// An unparseable OTHER is still merely unequal, which is the asymmetry #2391 landed and
449+
// #2393 does not touch.
439450
EXPECT_FALSE(good.Equals(emptyScheme));
440451
}
441452

modules/uri/tests/System/UriCreationOptionsTests.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,28 @@ TEST(UriCreationOptionsTest, Fix1997A3_TheTryCreateOverloadExists) {
5959
}
6060

6161
TEST(UriCreationOptionsTest, Decl1997A3_BothOverloadsResolveAgainstAbsoluteNotRelativeOrAbsolute) {
62-
// THE DETAIL THAT IS EASY TO GET WRONG, because the sibling one-argument Uri constructor
63-
// accepts a relative string. .NET passes UriKind.Absolute in BOTH bodies, so a relative
64-
// string throws through the constructor and fails through TryCreate.
62+
// .NET passes UriKind.Absolute in BOTH bodies, so a relative string throws through the
63+
// constructor and fails through TryCreate.
6564
UriCreationOptions opts;
6665
EXPECT_THROW(System::Uri("/relative/path", opts), System::UriFormatException);
6766

6867
std::shared_ptr<System::Uri> result;
6968
EXPECT_FALSE(System::Uri::TryCreate("/relative/path", opts, result));
7069
EXPECT_EQ(result, nullptr);
7170

72-
// ...while the one-argument constructor still accepts it, which is what makes the
73-
// distinction observable rather than theoretical.
74-
EXPECT_NO_THROW(System::Uri("/relative/path"));
71+
// UPDATED BY #2393. This case used to end by asserting that the ONE-ARGUMENT constructor
72+
// still ACCEPTS "/relative/path", and called that "what makes the distinction observable
73+
// rather than theoretical". It was observable, and it was a DEFECT rather than a
74+
// distinction: .NET's one-argument constructor is CreateThis(uriString, false,
75+
// UriKind.Absolute) (Uri.cs:424-429), the same grammar TryCreate uses, so it rejects a
76+
// relative string too. #2393 made the two agree, and the assertion is inverted.
77+
EXPECT_THROW((void)System::Uri("/relative/path"), System::UriFormatException)
78+
<< "#2393: the one-argument constructor is UriKind::Absolute, as .NET's is";
79+
80+
// What genuinely IS a distinction, and what this case is really for: the options overloads
81+
// resolve against Absolute rather than RelativeOrAbsolute. Asserted against the kind that
82+
// WOULD have accepted it, so the case still discriminates now that the sibling agrees.
83+
EXPECT_NO_THROW((void)System::Uri("/relative/path", System::UriKind::RelativeOrAbsolute));
7584
}
7685

7786
TEST(UriCreationOptionsTest, Decl1997A3_TheFlagIsInertAndTheResultIsIdentical) {

0 commit comments

Comments
 (0)