From 9ce4a970a21bace3fb262da9591ed52359309592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0torc?= Date: Tue, 9 Jun 2020 17:48:39 +0200 Subject: [PATCH] Parsing extension-av on Set Cookie header (#22181) --- .../Microsoft.Net.Http.Headers.netcoreapp.cs | 1 + src/Http/Headers/src/SetCookieHeaderValue.cs | 63 +++++++++++++++---- .../Headers/test/SetCookieHeaderValueTest.cs | 55 +++++++++++++--- 3 files changed, 100 insertions(+), 19 deletions(-) diff --git a/src/Http/Headers/ref/Microsoft.Net.Http.Headers.netcoreapp.cs b/src/Http/Headers/ref/Microsoft.Net.Http.Headers.netcoreapp.cs index ff176b7f4e..c35b6d1964 100644 --- a/src/Http/Headers/ref/Microsoft.Net.Http.Headers.netcoreapp.cs +++ b/src/Http/Headers/ref/Microsoft.Net.Http.Headers.netcoreapp.cs @@ -329,6 +329,7 @@ namespace Microsoft.Net.Http.Headers public SetCookieHeaderValue(Microsoft.Extensions.Primitives.StringSegment name, Microsoft.Extensions.Primitives.StringSegment value) { } public Microsoft.Extensions.Primitives.StringSegment Domain { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } public System.DateTimeOffset? Expires { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } + public System.Collections.Generic.IList Extensions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } } public bool HttpOnly { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } public System.TimeSpan? MaxAge { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } public Microsoft.Extensions.Primitives.StringSegment Name { get { throw null; } set { } } diff --git a/src/Http/Headers/src/SetCookieHeaderValue.cs b/src/Http/Headers/src/SetCookieHeaderValue.cs index 5e7c28564e..2360cf0daa 100644 --- a/src/Http/Headers/src/SetCookieHeaderValue.cs +++ b/src/Http/Headers/src/SetCookieHeaderValue.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; +using System.Linq; using System.Text; using Microsoft.Extensions.Primitives; @@ -99,6 +100,8 @@ namespace Microsoft.Net.Http.Headers public bool HttpOnly { get; set; } + public IList Extensions { get; } = new List(); + // name="value"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite={strict|lax|none}; httponly public override string ToString() { @@ -155,6 +158,11 @@ namespace Microsoft.Net.Http.Headers length += SeparatorToken.Length + HttpOnlyToken.Length; } + foreach (var extension in Extensions) + { + length += SeparatorToken.Length + extension.Length; + } + return string.Create(length, (this, maxAge, sameSite), (span, tuple) => { var (headerValue, maxAgeValue, sameSite) = tuple; @@ -204,6 +212,11 @@ namespace Microsoft.Net.Http.Headers { AppendSegment(ref span, HttpOnlyToken, null); } + + foreach (var extension in Extensions) + { + AppendSegment(ref span, extension, null); + } }); } @@ -281,6 +294,11 @@ namespace Microsoft.Net.Http.Headers { AppendSegment(builder, HttpOnlyToken, null); } + + foreach (var extension in Extensions) + { + AppendSegment(builder, extension, null); + } } private static void AppendSegment(StringBuilder builder, StringSegment name, StringSegment value) @@ -399,7 +417,8 @@ namespace Microsoft.Net.Http.Headers { return 0; } - var dateString = ReadToSemicolonOrEnd(input, ref offset); + // We don't want to include comma, becouse date may contain it (eg. Sun, 06 Nov...) + var dateString = ReadToSemicolonOrEnd(input, ref offset, includeComma: false); DateTimeOffset expirationDate; if (!HttpRuleParser.TryStringToDate(dateString, out expirationDate)) { @@ -499,13 +518,9 @@ namespace Microsoft.Net.Http.Headers // extension-av = else { - // TODO: skiping it for now to avoid parsing failure? Store it in a list? - // = (no spaces) - if (!ReadEqualsSign(input, ref offset)) - { - return 0; - } - ReadToSemicolonOrEnd(input, ref offset); + var tokenStart = offset - itemLength; + ReadToSemicolonOrEnd(input, ref offset, includeComma: true); + result.Extensions.Add(input.Subsegment(tokenStart, offset - tokenStart)); } } @@ -524,14 +539,32 @@ namespace Microsoft.Net.Http.Headers return true; } - private static StringSegment ReadToSemicolonOrEnd(StringSegment input, ref int offset) + private static StringSegment ReadToSemicolonOrEnd(StringSegment input, ref int offset, bool includeComma = true) { var end = input.IndexOf(';', offset); + if (end < 0) + { + // Also valid end of cookie + if (includeComma) + { + end = input.IndexOf(',', offset); + } + } + else if (includeComma) + { + var commaPosition = input.IndexOf(',', offset); + if (commaPosition >= 0 && commaPosition < end) + { + end = commaPosition; + } + } + if (end < 0) { // Remainder of the string end = input.Length; } + var itemLength = end - offset; var result = input.Subsegment(offset, itemLength); offset += itemLength; @@ -555,12 +588,13 @@ namespace Microsoft.Net.Http.Headers && StringSegment.Equals(Path, other.Path, StringComparison.OrdinalIgnoreCase) && Secure == other.Secure && SameSite == other.SameSite - && HttpOnly == other.HttpOnly; + && HttpOnly == other.HttpOnly + && HeaderUtilities.AreEqualCollections(Extensions, other.Extensions, StringSegmentComparer.OrdinalIgnoreCase); } public override int GetHashCode() { - return StringSegmentComparer.OrdinalIgnoreCase.GetHashCode(_name) + var hash = StringSegmentComparer.OrdinalIgnoreCase.GetHashCode(_name) ^ StringSegmentComparer.OrdinalIgnoreCase.GetHashCode(_value) ^ (Expires.HasValue ? Expires.GetHashCode() : 0) ^ (MaxAge.HasValue ? MaxAge.GetHashCode() : 0) @@ -569,6 +603,13 @@ namespace Microsoft.Net.Http.Headers ^ Secure.GetHashCode() ^ SameSite.GetHashCode() ^ HttpOnly.GetHashCode(); + + foreach (var extension in Extensions) + { + hash ^= extension.GetHashCode(); + } + + return hash; } } } diff --git a/src/Http/Headers/test/SetCookieHeaderValueTest.cs b/src/Http/Headers/test/SetCookieHeaderValueTest.cs index da75465926..3a6d64842e 100644 --- a/src/Http/Headers/test/SetCookieHeaderValueTest.cs +++ b/src/Http/Headers/test/SetCookieHeaderValueTest.cs @@ -4,7 +4,10 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net; using System.Text; +using Microsoft.Extensions.Primitives; +using Moq; using Xunit; namespace Microsoft.Net.Http.Headers @@ -24,9 +27,11 @@ namespace Microsoft.Net.Http.Headers HttpOnly = true, MaxAge = TimeSpan.FromDays(1), Path = "path1", - Secure = true + Secure = true, }; - dataset.Add(header1, "name1=n1=v1&n2=v2&n3=v3; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite=strict; httponly"); + header1.Extensions.Add("extension1"); + header1.Extensions.Add("extension2=value"); + dataset.Add(header1, "name1=n1=v1&n2=v2&n3=v3; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite=strict; httponly; extension1; extension2=value"); var header2 = new SetCookieHeaderValue("name2", ""); dataset.Add(header2, "name2="); @@ -59,6 +64,10 @@ namespace Microsoft.Net.Http.Headers }; dataset.Add(header7, "name7=value7; samesite=none"); + var header8 = new SetCookieHeaderValue("name8", "value8"); + header8.Extensions.Add("extension1"); + header8.Extensions.Add("extension2=value"); + dataset.Add(header8, "name8=value8; extension1; extension2=value"); return dataset; } @@ -126,7 +135,10 @@ namespace Microsoft.Net.Http.Headers Path = "path1", Secure = true }; - var string1 = "name1=n1=v1&n2=v2&n3=v3; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite=strict; httponly"; + header1.Extensions.Add("extension1"); + header1.Extensions.Add("extension2=value"); + + var string1 = "name1=n1=v1&n2=v2&n3=v3; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; samesite=strict; httponly; extension1; extension2=value"; var header2 = new SetCookieHeaderValue("name2", "value2"); var string2 = "name2=value2"; @@ -170,6 +182,12 @@ namespace Microsoft.Net.Http.Headers var string8a = "name8=value8; samesite"; var string8b = "name8=value8; samesite=invalid"; + var header9 = new SetCookieHeaderValue("name9", "value9"); + header9.Extensions.Add("extension1"); + header9.Extensions.Add("extension2=value"); + var string9 = "name9=value9; extension1; extension2=value"; + + 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 }); @@ -185,6 +203,22 @@ namespace Microsoft.Net.Http.Headers dataset.Add(new[] { header7 }.ToList(), new[] { string7 }); dataset.Add(new[] { header8 }.ToList(), new[] { string8a }); dataset.Add(new[] { header8 }.ToList(), new[] { string8b }); + dataset.Add(new[] { header9 }.ToList(), new[] { string9 }); + + foreach (var item1 in SetCookieHeaderDataSet) + { + var pair_cookie1 = (SetCookieHeaderValue)item1[0]; + var pair_string1 = item1[1].ToString(); + + foreach (var item2 in SetCookieHeaderDataSet) + { + var pair_cookie2 = (SetCookieHeaderValue)item2[0]; + var pair_string2 = item2[1].ToString(); + + dataset.Add(new[] { pair_cookie1, pair_cookie2 }.ToList(), new[] { string.Join(", ", pair_string1, pair_string2) }); + + } + } return dataset; } @@ -378,13 +412,18 @@ namespace Microsoft.Net.Http.Headers } [Fact] - public void SetCookieHeaderValue_TryParse_SkipExtensionValues() + public void SetCookieHeaderValue_TryParse_ExtensionOrderDoesntMatter() { - string cookieHeaderValue = "cookiename=value; extensionname=value;"; + string cookieHeaderValue1 = "cookiename=value; extensionname1=value; extensionname2=value;"; + string cookieHeaderValue2 = "cookiename=value; extensionname2=value; extensionname1=value;"; + SetCookieHeaderValue setCookieHeaderValue1; + SetCookieHeaderValue setCookieHeaderValue2; - SetCookieHeaderValue.TryParse(cookieHeaderValue, out var setCookieHeaderValue); - Assert.Equal("value", setCookieHeaderValue!.Value); + SetCookieHeaderValue.TryParse(cookieHeaderValue1, out setCookieHeaderValue1); + SetCookieHeaderValue.TryParse(cookieHeaderValue2, out setCookieHeaderValue2); + + Assert.Equal(setCookieHeaderValue1, setCookieHeaderValue2); } [Theory] @@ -428,7 +467,7 @@ namespace Microsoft.Net.Http.Headers [MemberData(nameof(ListWithInvalidSetCookieHeaderDataSet))] public void SetCookieHeaderValue_ParseStrictList_ThrowsForAnyInvalidValues( #pragma warning disable xUnit1026 // Theory methods should use all of their parameters - IList cookies, + IList cookies, #pragma warning restore xUnit1026 // Theory methods should use all of their parameters string[] input) {