Skip to content

Commit 7afe3a6

Browse files
BrennanConroywtgodbe
BrennanConroy
authored and
wtgodbe
committed
Merged PR 50693: Fix Cookie parsing
Fix Cookie parsing.
1 parent 3e967f7 commit 7afe3a6

File tree

6 files changed

+104
-16
lines changed

6 files changed

+104
-16
lines changed

‎eng/PatchConfig.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ Later on, this will be checked using this condition:
3434
</PropertyGroup>
3535
<PropertyGroup Condition=" '$(VersionPrefix)' == '2.3.4' ">
3636
<PackagesInPatch>
37+
Microsoft.Net.Http.Headers;
3738
</PackagesInPatch>
3839
</PropertyGroup>
3940
</Project>

‎src/Http/Headers/src/CookieHeaderParser.cs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ public sealed override bool TryParseValue(StringSegment value, ref int index, ou
4747
CookieHeaderValue result = null;
4848
if (!CookieHeaderValue.TryGetCookieLength(value, ref current, out result))
4949
{
50+
var separatorIndex = value.IndexOf(';', current);
51+
if (separatorIndex > 0)
52+
{
53+
// Skip the invalid values and keep trying.
54+
index = separatorIndex;
55+
}
56+
else
57+
{
58+
// No more separators, so we're done.
59+
index = value.Length;
60+
}
5061
return false;
5162
}
5263

@@ -55,6 +66,17 @@ public sealed override bool TryParseValue(StringSegment value, ref int index, ou
5566
// If we support multiple values and we've not reached the end of the string, then we must have a separator.
5667
if ((separatorFound && !SupportsMultipleValues) || (!separatorFound && (current < value.Length)))
5768
{
69+
var separatorIndex = value.IndexOf(';', current);
70+
if (separatorIndex > 0)
71+
{
72+
// Skip the invalid values and keep trying.
73+
index = separatorIndex;
74+
}
75+
else
76+
{
77+
// No more separators, so we're done.
78+
index = value.Length;
79+
}
5880
return false;
5981
}
6082

@@ -71,7 +93,7 @@ private static int GetNextNonEmptyOrWhitespaceIndex(StringSegment input, int sta
7193
separatorFound = false;
7294
var current = startIndex + HttpRuleParser.GetWhitespaceLength(input, startIndex);
7395

74-
if ((current == input.Length) || (input[current] != ',' && input[current] != ';'))
96+
if ((current == input.Length) || (input[current] != ';'))
7597
{
7698
return current;
7799
}
@@ -84,8 +106,8 @@ private static int GetNextNonEmptyOrWhitespaceIndex(StringSegment input, int sta
84106

85107
if (skipEmptyValues)
86108
{
87-
// Most headers only split on ',', but cookies primarily split on ';'
88-
while ((current < input.Length) && ((input[current] == ',') || (input[current] == ';')))
109+
// Cookies are split on ';'
110+
while ((current < input.Length) && (input[current] == ';'))
89111
{
90112
current++; // skip delimiter.
91113
current = current + HttpRuleParser.GetWhitespaceLength(input, current);

‎src/Http/Headers/src/CookieHeaderValue.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,18 @@ public static bool TryParseStrictList(IList<string> inputs, out IList<CookieHead
112112
return MultipleValueParser.TryParseStrictValues(inputs, out parsedValues);
113113
}
114114

115+
/*
116+
* https://www.rfc-editor.org/rfc/rfc6265#section-4.1.1
117+
* cookie-pair = cookie-name "=" cookie-value
118+
* cookie-name = token
119+
* token = 1*<any CHAR except CTLs or separators>
120+
separators = "(" | ")" | "<" | ">" | "@"
121+
| "," | ";" | ":" | "\" | <">
122+
| "/" | "[" | "]" | "?" | "="
123+
| "{" | "}" | SP | HT
124+
CTL = <any US-ASCII control character
125+
(octets 0 - 31) and DEL (127)>
126+
*/
115127
// name=value; name="value"
116128
internal static bool TryGetCookieLength(StringSegment input, ref int offset, out CookieHeaderValue parsedValue)
117129
{

‎src/Http/Headers/test/CookieHeaderValueTest.cs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public static TheoryData<string> InvalidCookieValues
8080
}
8181
}
8282

83-
public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfCookieHeaderDataSet
83+
public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfStrictCookieHeaderDataSet
8484
{
8585
get
8686
{
@@ -99,19 +99,30 @@ public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfCookieHeaderD
9999

100100
dataset.Add(new[] { header1 }.ToList(), new[] { string1 });
101101
dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, string1 });
102-
dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, null, "", " ", ";", " , ", string1 });
103102
dataset.Add(new[] { header2 }.ToList(), new[] { string2 });
104103
dataset.Add(new[] { header1, header2 }.ToList(), new[] { string1, string2 });
105-
dataset.Add(new[] { header1, header2 }.ToList(), new[] { string1 + ", " + string2 });
106104
dataset.Add(new[] { header2, header1 }.ToList(), new[] { string2 + "; " + string1 });
107105
dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string1, string2, string3, string4 });
108-
dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string.Join(",", string1, string2, string3, string4) });
109106
dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string.Join(";", string1, string2, string3, string4) });
110107

111108
return dataset;
112109
}
113110
}
114111

112+
public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfCookieHeaderDataSet
113+
{
114+
get
115+
{
116+
var header1 = new CookieHeaderValue("name1", "n1=v1&n2=v2&n3=v3");
117+
var string1 = "name1=n1=v1&n2=v2&n3=v3";
118+
119+
var dataset = new TheoryData<IList<CookieHeaderValue>, string[]>();
120+
dataset.Concat(ListOfStrictCookieHeaderDataSet);
121+
dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, null, "", " ", ";", " , ", string1 });
122+
return dataset;
123+
}
124+
}
125+
115126
public static TheoryData<IList<CookieHeaderValue>, string[]> ListWithInvalidCookieHeaderDataSet
116127
{
117128
get
@@ -132,18 +143,19 @@ public static TheoryData<IList<CookieHeaderValue>, string[]> ListWithInvalidCook
132143
dataset.Add(new[] { header1 }.ToList(), new[] { validString1, invalidString1 });
133144
dataset.Add(new[] { header1 }.ToList(), new[] { validString1, null, "", " ", ";", " , ", invalidString1 });
134145
dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1, null, "", " ", ";", " , ", validString1 });
135-
dataset.Add(new[] { header1 }.ToList(), new[] { validString1 + ", " + invalidString1 });
136-
dataset.Add(new[] { header2 }.ToList(), new[] { invalidString1 + ", " + validString2 });
146+
dataset.Add(null, new[] { validString1 + ", " });
147+
dataset.Add(null, new[] { invalidString1 + ", " + validString2 });
137148
dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1 + "; " + validString1 });
138149
dataset.Add(new[] { header2 }.ToList(), new[] { validString2 + "; " + invalidString1 });
139-
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { invalidString1, validString1, validString2, validString3 });
150+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { invalidString1, validString1, validString2, validString3 });
140151
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, invalidString1, validString2, validString3 });
141152
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, invalidString1, validString3 });
142153
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, validString3, invalidString1 });
143-
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", invalidString1, validString1, validString2, validString3) });
144-
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, invalidString1, validString2, validString3) });
145-
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, invalidString1, validString3) });
146-
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, validString3, invalidString1) });
154+
dataset.Add(null, new[] { string.Join(",", invalidString1, validString1, validString2, validString3) });
155+
dataset.Add(null, new[] { string.Join(",", validString1, invalidString1, validString2, validString3) });
156+
dataset.Add(null, new[] { string.Join(",", validString1, validString2, invalidString1, validString3) });
157+
dataset.Add(null, new[] { string.Join(",", validString1, validString2, validString3, invalidString1) });
158+
dataset.Add(null, new[] { string.Join(",", validString1, validString2, validString3) });
147159
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", invalidString1, validString1, validString2, validString3) });
148160
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, invalidString1, validString2, validString3) });
149161
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, validString2, invalidString1, validString3) });
@@ -253,7 +265,7 @@ public void CookieHeaderValue_ParseList_AcceptsValidValues(IList<CookieHeaderVal
253265
}
254266

255267
[Theory]
256-
[MemberData(nameof(ListOfCookieHeaderDataSet))]
268+
[MemberData(nameof(ListOfStrictCookieHeaderDataSet))]
257269
public void CookieHeaderValue_ParseStrictList_AcceptsValidValues(IList<CookieHeaderValue> cookies, string[] input)
258270
{
259271
var results = CookieHeaderValue.ParseStrictList(input);
@@ -272,7 +284,7 @@ public void CookieHeaderValue_TryParseList_AcceptsValidValues(IList<CookieHeader
272284
}
273285

274286
[Theory]
275-
[MemberData(nameof(ListOfCookieHeaderDataSet))]
287+
[MemberData(nameof(ListOfStrictCookieHeaderDataSet))]
276288
public void CookieHeaderValue_TryParseStrictList_AcceptsValidValues(IList<CookieHeaderValue> cookies, string[] input)
277289
{
278290
var result = CookieHeaderValue.TryParseStrictList(input, out var results);

‎src/Http/Http/test/Microsoft.AspNetCore.Http.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<ItemGroup>
88
<Reference Include="Microsoft.AspNetCore.Http" />
99
<Reference Include="Microsoft.Extensions.DependencyInjection" />
10+
<ProjectReference Include="..\..\Headers\src\Microsoft.Net.Http.Headers.csproj" />
1011
</ItemGroup>
1112

1213
</Project>

‎src/Http/Http/test/RequestCookiesCollectionTests.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,45 @@ public void AppContextSwitchUnEscapesKeyValues(string input, string expectedKey,
4242
Assert.Equal(expectedKey, cookies.Keys.Single());
4343
Assert.Equal(expectedValue, cookies[expectedKey]);
4444
}
45+
46+
[Theory]
47+
[InlineData(",", null)]
48+
[InlineData(";", null)]
49+
[InlineData("er=dd,cc,bb", null)]
50+
[InlineData("er=dd,err=cc,errr=bb", null)]
51+
[InlineData("errorcookie=dd,:(\"sa;", null)]
52+
[InlineData("s;", null)]
53+
[InlineData("a@a=a;", null)]
54+
[InlineData("a@ a=a;", null)]
55+
[InlineData("a a=a;", null)]
56+
[InlineData(",a=a;", null)]
57+
[InlineData(",a=a", null)]
58+
[InlineData("a=a;,b=b", new []{ "a" })] // valid cookie followed by invalid cookie
59+
[InlineData(",a=a;b=b", new[] { "b" })] // invalid cookie followed by valid cookie
60+
public void ParseInvalidCookies(string cookieToParse, string[] expectedCookieValues)
61+
{
62+
var cookies = RequestCookieCollection.Parse(new StringValues(new[] { cookieToParse }));
63+
64+
if (expectedCookieValues == null)
65+
{
66+
Assert.Equal(0, cookies.Count);
67+
return;
68+
}
69+
70+
Assert.Equal(expectedCookieValues.Length, cookies.Count);
71+
for (int i = 0; i < expectedCookieValues.Length; i++)
72+
{
73+
var value = expectedCookieValues[i];
74+
Assert.Equal(value, cookies.ElementAt(i).Value);
75+
}
76+
}
77+
78+
[Fact]
79+
public void ParseManyCookies()
80+
{
81+
var cookies = RequestCookieCollection.Parse(new StringValues(new[] { "a=a", "b=b", "c=c", "d=d", "e=e", "f=f", "g=g", "h=h", "i=i", "j=j", "k=k", "l=l" }));
82+
83+
Assert.Equal(12, cookies.Count);
84+
}
4585
}
4686
}

0 commit comments

Comments
 (0)