From 7afe3a60d4603e73c14ccdae92515c1a64a0b8db Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 12 Jun 2025 23:50:15 +0000 Subject: [PATCH] Merged PR 50693: Fix Cookie parsing Fix Cookie parsing. --- eng/PatchConfig.props | 1 + src/Http/Headers/src/CookieHeaderParser.cs | 28 +++++++++++-- src/Http/Headers/src/CookieHeaderValue.cs | 12 ++++++ .../Headers/test/CookieHeaderValueTest.cs | 38 ++++++++++++------ .../Microsoft.AspNetCore.Http.Tests.csproj | 1 + .../test/RequestCookiesCollectionTests.cs | 40 +++++++++++++++++++ 6 files changed, 104 insertions(+), 16 deletions(-) diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 176d7cde2e66..0c425d6323a0 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -34,6 +34,7 @@ Later on, this will be checked using this condition: + Microsoft.Net.Http.Headers; diff --git a/src/Http/Headers/src/CookieHeaderParser.cs b/src/Http/Headers/src/CookieHeaderParser.cs index a94b61d319e9..de62a315ab0e 100644 --- a/src/Http/Headers/src/CookieHeaderParser.cs +++ b/src/Http/Headers/src/CookieHeaderParser.cs @@ -47,6 +47,17 @@ public sealed override bool TryParseValue(StringSegment value, ref int index, ou CookieHeaderValue result = null; if (!CookieHeaderValue.TryGetCookieLength(value, ref current, out result)) { + var separatorIndex = value.IndexOf(';', current); + if (separatorIndex > 0) + { + // Skip the invalid values and keep trying. + index = separatorIndex; + } + else + { + // No more separators, so we're done. + index = value.Length; + } return false; } @@ -55,6 +66,17 @@ public sealed override bool TryParseValue(StringSegment value, ref int index, ou // If we support multiple values and we've not reached the end of the string, then we must have a separator. if ((separatorFound && !SupportsMultipleValues) || (!separatorFound && (current < value.Length))) { + var separatorIndex = value.IndexOf(';', current); + if (separatorIndex > 0) + { + // Skip the invalid values and keep trying. + index = separatorIndex; + } + else + { + // No more separators, so we're done. + index = value.Length; + } return false; } @@ -71,7 +93,7 @@ private static int GetNextNonEmptyOrWhitespaceIndex(StringSegment input, int sta separatorFound = false; var current = startIndex + HttpRuleParser.GetWhitespaceLength(input, startIndex); - if ((current == input.Length) || (input[current] != ',' && input[current] != ';')) + if ((current == input.Length) || (input[current] != ';')) { return current; } @@ -84,8 +106,8 @@ private static int GetNextNonEmptyOrWhitespaceIndex(StringSegment input, int sta if (skipEmptyValues) { - // Most headers only split on ',', but cookies primarily split on ';' - while ((current < input.Length) && ((input[current] == ',') || (input[current] == ';'))) + // Cookies are split on ';' + while ((current < input.Length) && (input[current] == ';')) { current++; // skip delimiter. current = current + HttpRuleParser.GetWhitespaceLength(input, current); diff --git a/src/Http/Headers/src/CookieHeaderValue.cs b/src/Http/Headers/src/CookieHeaderValue.cs index 3061b7d2fa56..cb3ba6b81a49 100644 --- a/src/Http/Headers/src/CookieHeaderValue.cs +++ b/src/Http/Headers/src/CookieHeaderValue.cs @@ -112,6 +112,18 @@ public static bool TryParseStrictList(IList inputs, out IList" | "@" + | "," | ";" | ":" | "\" | <"> + | "/" | "[" | "]" | "?" | "=" + | "{" | "}" | SP | HT + CTL = + */ // name=value; name="value" internal static bool TryGetCookieLength(StringSegment input, ref int offset, out CookieHeaderValue parsedValue) { diff --git a/src/Http/Headers/test/CookieHeaderValueTest.cs b/src/Http/Headers/test/CookieHeaderValueTest.cs index 416441991d7c..c4e17d2c8ea1 100644 --- a/src/Http/Headers/test/CookieHeaderValueTest.cs +++ b/src/Http/Headers/test/CookieHeaderValueTest.cs @@ -80,7 +80,7 @@ public static TheoryData InvalidCookieValues } } - public static TheoryData, string[]> ListOfCookieHeaderDataSet + public static TheoryData, string[]> ListOfStrictCookieHeaderDataSet { get { @@ -99,19 +99,30 @@ public static TheoryData, string[]> ListOfCookieHeaderD dataset.Add(new[] { header1 }.ToList(), new[] { string1 }); dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, string1 }); - dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, null, "", " ", ";", " , ", string1 }); dataset.Add(new[] { header2 }.ToList(), new[] { string2 }); dataset.Add(new[] { header1, header2 }.ToList(), new[] { string1, string2 }); - dataset.Add(new[] { header1, header2 }.ToList(), new[] { string1 + ", " + string2 }); dataset.Add(new[] { header2, header1 }.ToList(), new[] { string2 + "; " + string1 }); dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string1, string2, string3, string4 }); - dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string.Join(",", string1, string2, string3, string4) }); dataset.Add(new[] { header1, header2, header3, header4 }.ToList(), new[] { string.Join(";", string1, string2, string3, string4) }); return dataset; } } + public static TheoryData, string[]> ListOfCookieHeaderDataSet + { + get + { + var header1 = new CookieHeaderValue("name1", "n1=v1&n2=v2&n3=v3"); + var string1 = "name1=n1=v1&n2=v2&n3=v3"; + + var dataset = new TheoryData, string[]>(); + dataset.Concat(ListOfStrictCookieHeaderDataSet); + dataset.Add(new[] { header1, header1 }.ToList(), new[] { string1, null, "", " ", ";", " , ", string1 }); + return dataset; + } + } + public static TheoryData, string[]> ListWithInvalidCookieHeaderDataSet { get @@ -132,18 +143,19 @@ public static TheoryData, string[]> ListWithInvalidCook dataset.Add(new[] { header1 }.ToList(), new[] { validString1, invalidString1 }); dataset.Add(new[] { header1 }.ToList(), new[] { validString1, null, "", " ", ";", " , ", invalidString1 }); dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1, null, "", " ", ";", " , ", validString1 }); - dataset.Add(new[] { header1 }.ToList(), new[] { validString1 + ", " + invalidString1 }); - dataset.Add(new[] { header2 }.ToList(), new[] { invalidString1 + ", " + validString2 }); + dataset.Add(null, new[] { validString1 + ", " }); + dataset.Add(null, new[] { invalidString1 + ", " + validString2 }); dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1 + "; " + validString1 }); dataset.Add(new[] { header2 }.ToList(), new[] { validString2 + "; " + invalidString1 }); - dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { invalidString1, validString1, validString2, validString3 }); + dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { invalidString1, validString1, validString2, validString3 }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, invalidString1, validString2, validString3 }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, invalidString1, validString3 }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, validString3, invalidString1 }); - dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", invalidString1, validString1, validString2, validString3) }); - dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, invalidString1, validString2, validString3) }); - dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, invalidString1, validString3) }); - dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, validString3, invalidString1) }); + dataset.Add(null, new[] { string.Join(",", invalidString1, validString1, validString2, validString3) }); + dataset.Add(null, new[] { string.Join(",", validString1, invalidString1, validString2, validString3) }); + dataset.Add(null, new[] { string.Join(",", validString1, validString2, invalidString1, validString3) }); + dataset.Add(null, new[] { string.Join(",", validString1, validString2, validString3, invalidString1) }); + dataset.Add(null, new[] { string.Join(",", validString1, validString2, validString3) }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", invalidString1, validString1, validString2, validString3) }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, invalidString1, validString2, validString3) }); dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, validString2, invalidString1, validString3) }); @@ -253,7 +265,7 @@ public void CookieHeaderValue_ParseList_AcceptsValidValues(IList cookies, string[] input) { var results = CookieHeaderValue.ParseStrictList(input); @@ -272,7 +284,7 @@ public void CookieHeaderValue_TryParseList_AcceptsValidValues(IList cookies, string[] input) { var result = CookieHeaderValue.TryParseStrictList(input, out var results); diff --git a/src/Http/Http/test/Microsoft.AspNetCore.Http.Tests.csproj b/src/Http/Http/test/Microsoft.AspNetCore.Http.Tests.csproj index fe14f4306da3..ccca19ec9078 100644 --- a/src/Http/Http/test/Microsoft.AspNetCore.Http.Tests.csproj +++ b/src/Http/Http/test/Microsoft.AspNetCore.Http.Tests.csproj @@ -7,6 +7,7 @@ + diff --git a/src/Http/Http/test/RequestCookiesCollectionTests.cs b/src/Http/Http/test/RequestCookiesCollectionTests.cs index def99424c81e..c3593fe8876a 100644 --- a/src/Http/Http/test/RequestCookiesCollectionTests.cs +++ b/src/Http/Http/test/RequestCookiesCollectionTests.cs @@ -42,5 +42,45 @@ public void AppContextSwitchUnEscapesKeyValues(string input, string expectedKey, Assert.Equal(expectedKey, cookies.Keys.Single()); Assert.Equal(expectedValue, cookies[expectedKey]); } + + [Theory] + [InlineData(",", null)] + [InlineData(";", null)] + [InlineData("er=dd,cc,bb", null)] + [InlineData("er=dd,err=cc,errr=bb", null)] + [InlineData("errorcookie=dd,:(\"sa;", null)] + [InlineData("s;", null)] + [InlineData("a@a=a;", null)] + [InlineData("a@ a=a;", null)] + [InlineData("a a=a;", null)] + [InlineData(",a=a;", null)] + [InlineData(",a=a", null)] + [InlineData("a=a;,b=b", new []{ "a" })] // valid cookie followed by invalid cookie + [InlineData(",a=a;b=b", new[] { "b" })] // invalid cookie followed by valid cookie + public void ParseInvalidCookies(string cookieToParse, string[] expectedCookieValues) + { + var cookies = RequestCookieCollection.Parse(new StringValues(new[] { cookieToParse })); + + if (expectedCookieValues == null) + { + Assert.Equal(0, cookies.Count); + return; + } + + Assert.Equal(expectedCookieValues.Length, cookies.Count); + for (int i = 0; i < expectedCookieValues.Length; i++) + { + var value = expectedCookieValues[i]; + Assert.Equal(value, cookies.ElementAt(i).Value); + } + } + + [Fact] + public void ParseManyCookies() + { + 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" })); + + Assert.Equal(12, cookies.Count); + } } }