Skip to content

Conversation

@MihaZupan
Copy link
Member

Removes unsafe code from CheckCanonical and all its callers.

InternalIsWellFormedOriginalString was also checking the same thing twice, and we also check for it during parsing, so I added a new flag for it to avoid the duplication (SchemeNotCanonical_NoTrailingSlashes).

Reviewing is easier without whitespace diffs.

Draft while I gett more perf numbers.

@MihaZupan MihaZupan added this to the 11.0.0 milestone Nov 1, 2025
@MihaZupan MihaZupan self-assigned this Nov 1, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan
Copy link
Member Author

@MihuBot benchmark Perf_Uri -long -noTimeLimit

{
valid = IriHelper.CheckIriUnicodeRange(c, str[i + 1], out _, true);
valid = IriHelper.CheckIriUnicodeRange(c, span[i + 1], out _, true);
i++;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug in previous logic - since we didn't increment i when encountering a surrogate pair, the next iteration would see an unpaired low surrogate and mark the input as non-iri-canonical.

@MihuBot
Copy link

MihuBot commented Nov 3, 2025

System.Tests.Perf_Uri
BenchmarkDotNet v0.14.1-nightly.20250107.205, Linux Ubuntu 22.04.5 LTS (Jammy Jellyfish)
AMD EPYC 9V74, 1 CPU, 8 logical and 4 physical cores
LongRun : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
Job=LongRun  IterationCount=100  LaunchCount=3
WarmupCount=15
Method Toolchain input Mean Error P95 Ratio Allocated Alloc Ratio
ParseAbsoluteUri Main ? 165.5719 ns 0.3480 ns 168.7582 ns 1.00 304 B 1.00
ParseAbsoluteUri PR ? 170.9013 ns 0.3740 ns 173.6986 ns 1.03 304 B 1.00
DnsSafeHost Main ? 165.1498 ns 1.2993 ns 178.4041 ns 1.00 328 B 1.00
DnsSafeHost PR ? 166.7840 ns 0.4855 ns 170.9062 ns 1.01 328 B 1.00
BuilderToString Main ? 70.1461 ns 0.0648 ns 70.7084 ns 1.00 216 B 1.00
BuilderToString PR ? 72.0784 ns 0.2661 ns 73.8651 ns 1.03 216 B 1.00
UriBuilderReplacePort Main ? 66.7879 ns 0.2376 ns 68.8186 ns 1.00 216 B 1.00
UriBuilderReplacePort PR ? 65.9095 ns 0.0750 ns 66.6127 ns 0.99 216 B 1.00
GetComponents Main ? 10.9271 ns 0.0312 ns 11.2100 ns 1.00 80 B 1.00
GetComponents PR ? 10.7526 ns 0.0230 ns 10.9198 ns 0.98 80 B 1.00
PathAndQuery Main ? 0.8668 ns 0.0565 ns 1.8666 ns 1.07 - NA
PathAndQuery PR ? 0.7692 ns 0.0229 ns 0.8403 ns 0.95 - NA
EscapeDataString Main {{{{{{{{{{{{(...){{{{{{{{{{{{ [1000] 4,782.3351 ns 4.0869 ns 4,817.0185 ns 1.00 6024 B 1.00
EscapeDataString PR {{{{{{{{{{{{(...){{{{{{{{{{{{ [1000] 4,788.0341 ns 6.4869 ns 4,836.9449 ns 1.00 6024 B 1.00
CombineAbsoluteRelative Main /new/path 94.1380 ns 0.1055 ns 94.9918 ns 1.00 200 B 1.00
CombineAbsoluteRelative PR /new/path 93.9020 ns 0.0476 ns 94.2785 ns 1.00 200 B 1.00
UnescapeDataString Main %E4%BD%A0%E5%A5%BD 37.8972 ns 0.1065 ns 38.6942 ns 1.00 32 B 1.00
UnescapeDataString PR %E4%BD%A0%E5%A5%BD 37.5308 ns 0.0223 ns 37.7110 ns 0.99 32 B 1.00
EscapeDataString Main a{üa{üa{üa{ü(...)a{üa{üa{üa{ü [999] 7,387.2787 ns 9.6952 ns 7,456.3239 ns 1.00 6688 B 1.00
EscapeDataString PR a{üa{üa{üa{ü(...)a{üa{üa{üa{ü [999] 7,546.1135 ns 8.6053 ns 7,612.2079 ns 1.02 6688 B 1.00
EscapeDataString Main aaaaaaaaaaaa(...)aaaaaaaaaaaa [1000] 26.6628 ns 0.0186 ns 26.7594 ns 1.00 - NA
EscapeDataString PR aaaaaaaaaaaa(...)aaaaaaaaaaaa [1000] 26.3225 ns 0.0215 ns 26.4944 ns 0.99 - NA
UnescapeDataString Main abc%20def%20ghi%20 29.4827 ns 0.0791 ns 29.9335 ns 1.00 48 B 1.00
UnescapeDataString PR abc%20def%20ghi%20 30.6707 ns 0.2333 ns 32.5417 ns 1.04 48 B 1.00
Ctor Main http://dot.net 50.5849 ns 0.0483 ns 50.8468 ns 1.00 56 B 1.00
Ctor PR http://dot.net 50.1904 ns 0.0424 ns 50.5088 ns 0.99 56 B 1.00
CtorIdnHostPathAndQuery Main http://dot.ne(...)alue#fragment [43] 184.7182 ns 0.2019 ns 186.7217 ns 1.00 248 B 1.00
CtorIdnHostPathAndQuery PR http://dot.ne(...)alue#fragment [43] 192.2427 ns 0.3051 ns 194.6211 ns 1.04 248 B 1.00
Ctor Main http://höst.with.ünicode 227.3324 ns 0.3973 ns 230.9932 ns 1.00 256 B 1.00
Ctor PR http://höst.with.ünicode 222.4252 ns 1.8804 ns 236.2161 ns 0.98 256 B 1.00
CtorIdnHostPathAndQuery Main http://höst.w(...)alue#fragment [53] 1,063.8598 ns 2.5461 ns 1,085.6317 ns 1.00 936 B 1.00
CtorIdnHostPathAndQuery PR http://höst.w(...)alue#fragment [53] 1,081.9218 ns 4.2003 ns 1,111.5276 ns 1.02 936 B 1.00
CtorIdnHostPathAndQuery Main http://host/ 108.6913 ns 0.1150 ns 109.5783 ns 1.00 200 B 1.00
CtorIdnHostPathAndQuery PR http://host/ 112.0170 ns 0.1612 ns 113.5217 ns 1.03 200 B 1.00
CtorIdnHostPathAndQuery Main http://host/p(...)s?key=ünicode [50] 457.3145 ns 1.6727 ns 468.6085 ns 1.00 752 B 1.00
CtorIdnHostPathAndQuery PR http://host/p(...)s?key=ünicode [50] 457.0893 ns 0.6682 ns 462.1179 ns 1.00 752 B 1.00
CtorIdnHostPathAndQuery Main http://host/p(...)es?key=va lue [49] 277.8531 ns 0.3749 ns 280.7162 ns 1.00 296 B 1.00
CtorIdnHostPathAndQuery PR http://host/p(...)es?key=va lue [49] 280.0348 ns 0.1500 ns 281.1408 ns 1.01 296 B 1.00
CtorIdnHostPathAndQuery Main http://host/p(...)3&key4=value4 [64] 228.5983 ns 0.1650 ns 230.1201 ns 1.00 304 B 1.00
CtorIdnHostPathAndQuery PR http://host/p(...)3&key4=value4 [64] 238.0109 ns 0.4028 ns 240.9209 ns 1.04 304 B 1.00
CtorIdnHostPathAndQuery Main http://host/p(...)=%C3%BCnicode [61] 496.7966 ns 0.6181 ns 501.1303 ns 1.00 752 B 1.00
CtorIdnHostPathAndQuery PR http://host/p(...)=%C3%BCnicode [61] 511.6140 ns 3.0735 ns 523.6821 ns 1.03 752 B 1.00
CtorIdnHostPathAndQuery Main http://host/p(...)?key=va%20lue [57] 227.8241 ns 0.2377 ns 229.6564 ns 1.00 296 B 1.00
CtorIdnHostPathAndQuery PR http://host/p(...)?key=va%20lue [57] 231.6233 ns 0.0943 ns 232.3321 ns 1.02 296 B 1.00
Ctor Main http://xn--hs(...)n--nicode-2ya [38] 76.0055 ns 0.4043 ns 78.7504 ns 1.00 56 B 1.00
Ctor PR http://xn--hs(...)n--nicode-2ya [38] 78.8860 ns 0.9319 ns 87.1759 ns 1.04 56 B 1.00
CtorIdnHostPathAndQuery Main http://xn--hs(...)alue#fragment [67] 231.7594 ns 0.2575 ns 233.7422 ns 1.00 296 B 1.00
CtorIdnHostPathAndQuery PR http://xn--hs(...)alue#fragment [67] 239.9377 ns 0.3053 ns 241.6685 ns 1.04 296 B 1.00
Ctor Main https://a.much.longer.domain.name 86.5454 ns 0.0388 ns 86.8394 ns 1.00 56 B 1.00
Ctor PR https://a.much.longer.domain.name 85.7820 ns 0.2609 ns 87.2754 ns 0.99 56 B 1.00
CtorIdnHostPathAndQuery Main https://a.muc(...)alue#fragment [62] 235.6889 ns 0.3535 ns 238.5228 ns 1.00 280 B 1.00
CtorIdnHostPathAndQuery PR https://a.muc(...)alue#fragment [62] 245.1232 ns 0.6807 ns 250.2221 ns 1.04 280 B 1.00
Ctor Main https://contoso.com 49.8830 ns 0.1187 ns 50.7290 ns 1.00 56 B 1.00
Ctor PR https://contoso.com 50.6731 ns 0.1768 ns 52.0653 ns 1.02 56 B 1.00
Ctor Main https://CONTOSO.com 50.0956 ns 0.0376 ns 50.3699 ns 1.00 56 B 1.00
Ctor PR https://CONTOSO.com 50.3606 ns 0.0635 ns 50.7781 ns 1.01 56 B 1.00
CtorIdnHostPathAndQuery Main https://conto(...)alue#fragment [48] 186.0344 ns 0.1007 ns 186.9050 ns 1.00 256 B 1.00
CtorIdnHostPathAndQuery PR https://conto(...)alue#fragment [48] 197.0772 ns 0.3185 ns 199.2947 ns 1.06 256 B 1.00
CtorIdnHostPathAndQuery Main https://CONTO(...)alue#fragment [48] 187.8457 ns 0.2661 ns 189.9762 ns 1.00 256 B 1.00
CtorIdnHostPathAndQuery PR https://CONTO(...)alue#fragment [48] 197.6420 ns 0.5129 ns 200.9643 ns 1.05 256 B 1.00
EscapeDataString Main üüüüüüüüüüüü(...)üüüüüüüüüüüü [1000] 8,698.8407 ns 5.8625 ns 8,745.8565 ns 1.00 12024 B 1.00
EscapeDataString PR üüüüüüüüüüüü(...)üüüüüüüüüüüü [1000] 8,708.0605 ns 6.2922 ns 8,765.3683 ns 1.00 12024 B 1.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants