Skip to content

Commit 5bd5ae4

Browse files
antonfirsovantonfirsov
authored
Modernize CookieContainer domain-matching (#112604)
With #39781 and #64038 we shifted the .NET `Cookie`/`CookieContainer` behavior towards RFC 6265 for handling the leading dot in the `Domain` attribute, however there were gaps left primarily because the the cookie code is complicated and full of logic that handles legacy requirements from old RFC-s related to now obsolete attributes. The existing obscure logic results in weird unintended behaviors under the hood and the bug #84820. This PR is addressing this problem by adapting the [RFC 6265 domain matching algorithm](https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.3) with a few modifications in order to avoid breaking changes: - Single-label domains: in order for a host string to match a single-label domain string, they have to be identical. - `GetCookies(uri)` does not apply the RFC 6265 domain-matching for implicit domains. An implicit domain string has to be identical to host in order to match it.
1 parent 8fffd57 commit 5bd5ae4

File tree

9 files changed

+250
-450
lines changed

9 files changed

+250
-450
lines changed

‎src/libraries/Common/src/System/Net/CookieComparer.cs

+2-5
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@ internal static bool Equals(Cookie left, Cookie right)
2323
}
2424

2525
internal static bool EqualDomains(ReadOnlySpan<char> left, ReadOnlySpan<char> right)
26-
{
27-
if (left.StartsWith('.')) left = left.Slice(1);
28-
if (right.StartsWith('.')) right = right.Slice(1);
26+
=> StripLeadingDot(left).Equals(StripLeadingDot(right), StringComparison.OrdinalIgnoreCase);
2927

30-
return left.Equals(right, StringComparison.OrdinalIgnoreCase);
31-
}
28+
internal static ReadOnlySpan<char> StripLeadingDot(ReadOnlySpan<char> s) => s.StartsWith('.') ? s[1..] : s;
3229
}
3330
}

‎src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj

-2
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,6 @@
7676
Link="Common\System\Net\SocketAddressExtensions.cs" />
7777
<Compile Include="$(CommonPath)System\Net\NegotiationInfoClass.cs"
7878
Link="Common\System\Net\NegotiationInfoClass.cs" />
79-
<Compile Include="$(CommonPath)System\Net\NetworkInformation\HostInformation.cs"
80-
Link="Common\System\Net\NetworkInformation\HostInformation.cs" />
8179
<Compile Include="$(CommonPath)System\Text\StringBuilderCache.cs"
8280
Link="Common\System\Text\StringBuilderCache.cs" />
8381
<Compile Include="$(CommonPath)System\HexConverter.cs"

‎src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs

+72-149
Large diffs are not rendered by default.

‎src/libraries/System.Net.Primitives/src/System/Net/CookieContainer.cs

+43-205
Large diffs are not rendered by default.

‎src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest.cs

+5
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ public static void Equals_Compare_Success()
294294
Cookie c14 = new Cookie("name", "value", "path", "domain") { Version = 5 };
295295
Cookie c15 = new Cookie("name", "value", "path", "domain") { Version = 100 };
296296

297+
Cookie c9dot = new Cookie("name", "value", "path", ".domain");
298+
297299
Assert.False(c2.Equals(null));
298300
Assert.False(c2.Equals(""));
299301

@@ -329,6 +331,9 @@ public static void Equals_Compare_Success()
329331
Assert.NotEqual(c13, c15);
330332
Assert.Equal(c13.GetHashCode(), c14.GetHashCode());
331333
Assert.NotEqual(c13.GetHashCode(), c15.GetHashCode());
334+
335+
Assert.Equal(c9, c9dot);
336+
Assert.Equal(c9.GetHashCode(), c9dot.GetHashCode());
332337
}
333338

334339
[Fact]

‎src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest/CookieContainerTest.cs

-33
Original file line numberDiff line numberDiff line change
@@ -93,39 +93,6 @@ public void GetCookies_AddCookieVersion0WithExplicitDomain_CookieReturnedForDoma
9393
Assert.Equal(1, cookies.Count);
9494
}
9595

96-
[Fact]
97-
public void GetCookies_AddCookieVersion1WithExplicitDomain_CookieReturnedForDomainAndOneLevelSubDomain()
98-
{
99-
const string SchemePrefix = "http://";
100-
const string OriginalDomain = "contoso.com";
101-
const string OriginalDomainWithLeadingDot = "." + OriginalDomain;
102-
103-
var container = new CookieContainer();
104-
var cookie1 = new Cookie(CookieName1, CookieValue1) { Domain = OriginalDomainWithLeadingDot, Version = 1 };
105-
container.Add(new Uri(SchemePrefix + OriginalDomain), cookie1);
106-
107-
var uri = new Uri(SchemePrefix + OriginalDomain);
108-
var cookies = container.GetCookies(uri);
109-
Assert.Equal(1, cookies.Count);
110-
Assert.Equal(OriginalDomainWithLeadingDot, cookies[CookieName1].Domain);
111-
112-
uri = new Uri(SchemePrefix + "www." + OriginalDomain);
113-
cookies = container.GetCookies(uri);
114-
Assert.Equal(1, cookies.Count);
115-
116-
uri = new Uri(SchemePrefix + "x.www." + OriginalDomain);
117-
cookies = container.GetCookies(uri);
118-
Assert.Equal(0, cookies.Count);
119-
120-
uri = new Uri(SchemePrefix + "y.x.www." + OriginalDomain);
121-
cookies = container.GetCookies(uri);
122-
Assert.Equal(0, cookies.Count);
123-
124-
uri = new Uri(SchemePrefix + "z.y.x.www." + OriginalDomain);
125-
cookies = container.GetCookies(uri);
126-
Assert.Equal(0, cookies.Count);
127-
}
128-
12996
[Fact]
13097
public void GetAllCookies_Empty_ReturnsEmptyCollection()
13198
{

‎src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs

+123-36
Original file line numberDiff line numberDiff line change
@@ -467,39 +467,6 @@ public void GetCookies_AddCookieVersion0WithExplicitDomain_CookieReturnedForDoma
467467
Assert.Equal(1, cookies.Count);
468468
}
469469

470-
[Fact]
471-
public void GetCookies_AddCookieVersion1WithExplicitDomain_CookieReturnedForDomainAndOneLevelSubDomain()
472-
{
473-
const string SchemePrefix = "http://";
474-
const string OriginalDomain = "contoso.com";
475-
const string OriginalDomainWithLeadingDot = "." + OriginalDomain;
476-
477-
var container = new CookieContainer();
478-
var cookie1 = new Cookie(CookieName1, CookieValue1) { Domain = OriginalDomainWithLeadingDot, Version = 1 };
479-
container.Add(new Uri(SchemePrefix + OriginalDomain), cookie1);
480-
481-
var uri = new Uri(SchemePrefix + OriginalDomain);
482-
CookieCollection cookies = container.GetCookies(uri);
483-
Assert.Equal(1, cookies.Count);
484-
Assert.Equal(OriginalDomainWithLeadingDot, cookies[CookieName1].Domain);
485-
486-
uri = new Uri(SchemePrefix + "www." + OriginalDomain);
487-
cookies = container.GetCookies(uri);
488-
Assert.Equal(1, cookies.Count);
489-
490-
uri = new Uri(SchemePrefix + "x.www." + OriginalDomain);
491-
cookies = container.GetCookies(uri);
492-
Assert.Equal(0, cookies.Count);
493-
494-
uri = new Uri(SchemePrefix + "y.x.www." + OriginalDomain);
495-
cookies = container.GetCookies(uri);
496-
Assert.Equal(0, cookies.Count);
497-
498-
uri = new Uri(SchemePrefix + "z.y.x.www." + OriginalDomain);
499-
cookies = container.GetCookies(uri);
500-
Assert.Equal(0, cookies.Count);
501-
}
502-
503470
[Fact]
504471
public void Ctor_Capacity_Success()
505472
{
@@ -590,13 +557,20 @@ public void Add_SameCookieDifferentVairants_OverridesOlderVariant()
590557
{
591558
Uri uri = new Uri("http://domain.com");
592559

560+
Cookie c0 = new Cookie("name1", "value", "", "domain.com");
593561
Cookie c1 = new Cookie("name1", "value", "", ".domain.com"); // Variant = Plain
594562
Cookie c2 = new Cookie("name1", "value", "", ".domain.com") { Port = "\"80\"" }; // Variant = RFC2965 (should override)
595563
Cookie c3 = new Cookie("name1", "value", "", ".domain.com") { Port = "\"80, 90\"" }; // Variant = RFC2965 (should override)
596564
Cookie c4 = new Cookie("name1", "value", "", ".domain.com") { Version = 1 }; // Variant = RFC2109 (should be rejected)
597565

598566
CookieContainer cc = new CookieContainer();
567+
568+
cc.Add(c0);
569+
Assert.Equal("domain.com", cc.GetCookies(uri)[0].Domain);
570+
599571
cc.Add(c1);
572+
Assert.Equal(1, cc.Count);
573+
Assert.Equal(".domain.com", cc.GetCookies(uri)[0].Domain);
600574

601575
// Adding a newer variant should override an older one
602576
cc.Add(c2);
@@ -614,6 +588,24 @@ public void Add_SameCookieDifferentVairants_OverridesOlderVariant()
614588
Assert.Equal(1, cc.Count);
615589
}
616590

591+
[Fact]
592+
public static void Add_SetCookies_SameCookieDifferentVairants_OverridesOlderVariant()
593+
{
594+
Uri uri = new Uri("http://domain.com");
595+
CookieContainer cc = new();
596+
Cookie a = new()
597+
{
598+
Domain = "domain.com",
599+
Name = "lol",
600+
Value = "0"
601+
};
602+
cc.Add(uri, a);
603+
cc.SetCookies(uri, "lol=42");
604+
605+
Assert.Equal(1, cc.Count);
606+
Assert.Equal("42", cc.GetCookies(uri).Single().Value);
607+
}
608+
617609
[Fact]
618610
public void Add_Cookie_Invalid()
619611
{
@@ -781,10 +773,10 @@ private void VerifyGetCookies(CookieContainer cc1, Uri uri, Cookie[] expected)
781773

782774
for (int i = 0; i < expected.Length; i++)
783775
{
784-
Cookie c1 = expected[i];
785776
Cookie c2 = cc2[i];
786-
Assert.Equal(c1.Name, c2.Name); // Primitive check for equality
787-
Assert.Equal(c1.Value, c2.Value);
777+
Cookie c1 = expected.Single(c => c.Name == c2.Name);
778+
779+
Assert.Equal(c1.Value, c2.Value); // Primitive check for equality
788780
}
789781
}
790782

@@ -983,5 +975,100 @@ public void GetCookies_PathMatchingFollowsRfc6265(bool useDefaultPath, string[]
983975
CookieCollection collection = container.GetCookies(requestUri);
984976
Assert.Equal(expectedMatches, collection.Count);
985977
}
978+
979+
[Fact]
980+
public void Add_ImplicitDomainOfIPv6Hostname_Success()
981+
{
982+
CookieContainer container = new CookieContainer();
983+
Cookie cookie = new Cookie("lol", "haha");
984+
Uri uri = new Uri("https://[::FFFF:192.168.0.1]/test");
985+
container.Add(uri, cookie);
986+
Assert.Equal(uri.Host, container.GetCookies(uri).Single().Domain);
987+
}
988+
989+
public static TheoryData<string, string> DomainMatching_WhenMatches_Success_Data = new TheoryData<string, string>()
990+
{
991+
{ "https://q", "q" },
992+
{ "https://localhost/", "localhost" },
993+
{ "https://test.com", "test.com" },
994+
{ "https://test.COM", "tEsT.com" },
995+
{ "https://test.com", ".test.com" },
996+
{ "https://yay.test.com", "yay.test.com" },
997+
{ "https://yay.test.com", ".yay.test.com" },
998+
{ "https://yay.test.com", ".test.com" },
999+
{ "https://42.aaaa.bbb.cc.d.test.com", "d.test.com" },
1000+
{ "https://yay.test.com/foo/bar", "test.com" },
1001+
{ "https://127.0.1.1", "127.0.1.1" },
1002+
{ "https://42.42.100.100", "42.42.100.100" },
1003+
};
1004+
1005+
[Theory]
1006+
[MemberData(nameof(DomainMatching_WhenMatches_Success_Data))]
1007+
public void DomainMatching_WhenMatches_Success(string uriString, string domain)
1008+
{
1009+
CookieContainer container = new CookieContainer();
1010+
Cookie cookie = new Cookie("lol", "haha")
1011+
{
1012+
Domain = domain
1013+
};
1014+
1015+
Uri uri = new Uri(uriString);
1016+
container.Add(uri, cookie);
1017+
Assert.Equal(1, container.Count);
1018+
1019+
CookieCollection collection = container.GetCookies(uri);
1020+
Assert.Equal(1, collection.Count);
1021+
}
1022+
1023+
[Fact]
1024+
public void DomainMatching_ExplicitDomain_MatchesMultiple()
1025+
{
1026+
CookieContainer container = new CookieContainer();
1027+
Cookie a = new Cookie("a", "aa", null, "a.com");
1028+
Cookie b = new Cookie("b", "bb", null, "b.a.com");
1029+
Cookie c = new Cookie("c", "cc", null, "c.b.a.com");
1030+
1031+
container.Add(new Uri("http://a.com"), a);
1032+
container.Add(new Uri("http://b.a.com"), b);
1033+
container.Add(new Uri("http://c.b.a.com"), c);
1034+
1035+
CookieCollection matches = container.GetCookies(new Uri("http://c.b.a.com"));
1036+
Assert.Equal(3, matches.Count);
1037+
}
1038+
1039+
public static TheoryData<string, string> DomainMatching_WhenDoesNotMatch_ThrowsCookieException_Data = new TheoryData<string, string>()
1040+
{
1041+
{ "https://test.com", "test.co" }, // Domain is not a suffix
1042+
{ "https://test.com", "x.test.com" }, // Domain is not a suffix (extra chars at start)
1043+
{ "https://test.com", "ttest.com" }, // Suffix but not separated by dot
1044+
{ "https://test.com", "test.com." }, // Trailing dot
1045+
{ "https://test.com", "..test.com" }, // 2 leading dots
1046+
{ "https://foo.test.com", "yay.test.com" }, // subdomain mismatch
1047+
{ "https://42.42.100.100", "41.42.100.100" }, // different IP
1048+
1049+
// Single label domain without a full match.
1050+
{ "https://test.com", ".com" },
1051+
{ "https://test.com", "com" },
1052+
1053+
// If Host is an IP address, it should be equal to the domain.
1054+
// See https://issues.chromium.org/issues/40126142 and the last condition in
1055+
// https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.3
1056+
{ "https://1.2.3.4", ".2.3.4" }
1057+
};
1058+
1059+
1060+
[Theory]
1061+
[MemberData(nameof(DomainMatching_WhenDoesNotMatch_ThrowsCookieException_Data))]
1062+
public void DomainMatching_WhenDoesNotMatch_ThrowsCookieException(string uriString, string domain)
1063+
{
1064+
CookieContainer container = new CookieContainer();
1065+
Cookie cookie = new Cookie("lol", "haha")
1066+
{
1067+
Domain = domain
1068+
};
1069+
1070+
Uri uri = new Uri(uriString);
1071+
Assert.Throws<CookieException>(() => container.Add(uri, cookie));
1072+
}
9861073
}
9871074
}

‎src/libraries/System.Net.Primitives/tests/UnitTests/Fakes/HostInformation.cs

-16
This file was deleted.

‎src/libraries/System.Net.Primitives/tests/UnitTests/System.Net.Primitives.UnitTests.Tests.csproj

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
44
<NoWarn>$(NoWarn);169;649</NoWarn>
@@ -28,6 +28,10 @@
2828
Link="ProductionCode\System\Net\Sockets\SocketError.cs" />
2929
<Compile Include="..\..\src\System\Net\IPAddress.cs"
3030
Link="ProductionCode\System\Net\IPAddress.cs" />
31+
<Compile Include="$(CommonPath)System\Net\IPv4AddressHelper.Common.cs"
32+
Link="ProductionCode\Common\System\Net\IPv4AddressHelper.Common.cs" />
33+
<Compile Include="$(CommonPath)System\Net\IPv6AddressHelper.Common.cs"
34+
Link="ProductionCode\Common\System\Net\IPv6AddressHelper.Common.cs" />
3135
<Compile Include="..\..\src\System\Net\EndPoint.cs"
3236
Link="ProductionCode\System\Net\EndPoint.cs" />
3337
<Compile Include="..\..\src\System\Net\Sockets\AddressFamily.cs"
@@ -58,9 +62,6 @@
5862
<ItemGroup>
5963
<Compile Include="CookieCollectionTest.cs" />
6064
<Compile Include="Fakes\CookieException.cs" />
61-
<Compile Include="Fakes\HostInformation.cs" />
62-
<Compile Include="Fakes\IPv4AddressHelper.cs" />
63-
<Compile Include="Fakes\IPv6AddressHelper.cs" />
6465
<Compile Include="$(CommonPath)System\Net\HttpKnownHeaderNames.cs"
6566
Link="ProductionCode\Common\System\Net\HttpKnownHeaderNames.cs" />
6667
<Compile Include="$(CommonPath)System\Net\IPAddressParserStatics.cs"

0 commit comments

Comments
 (0)