Addressing feedback from asp.net team

uabarahona · uabarahona · commit ab3e8032aee9 · 2021-04-20T10:35:42.000-04:00
diff --git a/src/Http/Http.Features/src/Microsoft.AspNetCore.Http.Features.csproj b/src/Http/Http.Features/src/Microsoft.AspNetCore.Http.Features.csproj
@@ -10,7 +10,7 @@
     <Nullable>enable</Nullable>
   </PropertyGroup>
 
-   <ItemGroup>
+  <ItemGroup>
     <Reference Include="Microsoft.Extensions.Primitives" />
     <Reference Include="System.IO.Pipelines" />
   </ItemGroup>
diff --git a/src/Http/Http/src/Internal/ResponseCookies.cs b/src/Http/Http/src/Internal/ResponseCookies.cs
@@ -101,7 +101,7 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo
             {
                 if (_logger == null)
                 {
-                    var services = _features.Get<Features.IServiceProvidersFeature>()?.RequestServices;
+                    var services = _features.Get<IServiceProvidersFeature>()?.RequestServices;
                     _logger = services?.GetService<ILogger<ResponseCookies>>();
                 }
 
@@ -131,7 +131,8 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo
 
             foreach (var keyValuePair in keyValuePairs)
             {
-                cookies[position] = string.Concat(_enableCookieNameEncoding ? Uri.EscapeDataString(keyValuePair.Key) : keyValuePair.Key, "=", Uri.EscapeDataString(keyValuePair.Value), cookierHeaderValue);
+                var key = _enableCookieNameEncoding ? Uri.EscapeDataString(keyValuePair.Key) : keyValuePair.Key;
+                cookies[position] = string.Concat(key, "=", Uri.EscapeDataString(keyValuePair.Value), cookierHeaderValue);
                 position++;
             }
 
diff --git a/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs b/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+﻿// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
diff --git a/src/Security/CookiePolicy/src/ResponseCookiesWrapper.cs b/src/Security/CookiePolicy/src/ResponseCookiesWrapper.cs
@@ -161,7 +161,7 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo
                 throw new ArgumentNullException(nameof(options));
             }
 
-            var nonSuppressedValues = new List<KeyValuePair<string, string>>();
+            var nonSuppressedValues = new List<KeyValuePair<string, string>>(keyValuePairs.Length);
 
             foreach (var keyValuePair in keyValuePairs)
             {
diff --git a/src/Security/CookiePolicy/test/CookieChunkingTests.cs b/src/Security/CookiePolicy/test/CookieChunkingTests.cs
@@ -157,14 +157,14 @@ public void DeleteChunkedCookieWithOptionsAndResponseCookies_AllDeleted()
             
             httpContext.Request.Headers["Cookie"] = new[]
             {
-                "TestCookie=chunks-7; domain=foo.com; path=/; secure",
-                "TestCookieC1=abcdefghi; domain=foo.com; path=/; secure",
-                "TestCookieC2=jklmnopqr; domain=foo.com; path=/; secure",
-                "TestCookieC3=stuvwxyz0; domain=foo.com; path=/; secure",
-                "TestCookieC4=123456789; domain=foo.com; path=/; secure",
-                "TestCookieC5=ABCDEFGHI; domain=foo.com; path=/; secure",
-                "TestCookieC6=JKLMNOPQR; domain=foo.com; path=/; secure",
-                "TestCookieC7=STUVWXYZ; domain=foo.com; path=/; secure"
+                "TestCookie=chunks-7",
+                "TestCookieC1=abcdefghi",
+                "TestCookieC2=jklmnopqr",
+                "TestCookieC3=stuvwxyz0",
+                "TestCookieC4=123456789",
+                "TestCookieC5=ABCDEFGHI",
+                "TestCookieC6=JKLMNOPQR",
+                "TestCookieC7=STUVWXYZ"
             };
 
             var cookieOptions = new CookieOptions()
@@ -177,8 +177,8 @@ public void DeleteChunkedCookieWithOptionsAndResponseCookies_AllDeleted()
             httpContext.Response.Headers[HeaderNames.SetCookie] = new[]
             {
                 "TestCookie=chunks-7; domain=foo.com; path=/; secure",
-                "TestCookieC1=abcdefghi; domain=foo.com; path=/; secure",
-                "TestCookieC2=jklmnopqr; domain=foo.com; path=/; secure",
+                "TestCookieC1=STUVWXYZ; domain=foo.com; path=/; secure",
+                "TestCookieC2=123456789; domain=foo.com; path=/; secure",
                 "TestCookieC3=stuvwxyz0; domain=foo.com; path=/; secure",
                 "TestCookieC4=123456789; domain=foo.com; path=/; secure",
                 "TestCookieC5=ABCDEFGHI; domain=foo.com; path=/; secure",
@@ -188,6 +188,17 @@ public void DeleteChunkedCookieWithOptionsAndResponseCookies_AllDeleted()
 
             chunkingCookieManager.DeleteCookie(httpContext, "TestCookie", cookieOptions);
             Assert.Equal(8, httpContext.Response.Headers[HeaderNames.SetCookie].Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure"
+            }, httpContext.Response.Headers[HeaderNames.SetCookie]);
         }
     }
 }
diff --git a/src/Security/CookiePolicy/test/CookiePolicyTests.cs b/src/Security/CookiePolicy/test/CookiePolicyTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Security.Claims;
 using System.Security.Principal;
 using System.Threading.Tasks;
diff --git a/src/Security/perf/Microbenchmarks/ChunkingCookieManagerBenchmark.cs b/src/Security/perf/Microbenchmarks/ChunkingCookieManagerBenchmark.cs
@@ -32,14 +32,14 @@ public void GlobalSetup()
 
             _httpContext.Request.Headers["Cookie"] = new[]
             {
-                "TestCookie=chunks-7; domain=foo.com; path=/; secure",
-                "TestCookieC1=abcdefghi; domain=foo.com; path=/; secure",
-                "TestCookieC2=jklmnopqr; domain=foo.com; path=/; secure",
-                "TestCookieC3=stuvwxyz0; domain=foo.com; path=/; secure",
-                "TestCookieC4=123456789; domain=foo.com; path=/; secure",
-                "TestCookieC5=ABCDEFGHI; domain=foo.com; path=/; secure",
-                "TestCookieC6=JKLMNOPQR; domain=foo.com; path=/; secure",
-                "TestCookieC7=STUVWXYZ; domain=foo.com; path=/; secure"
+                "TestCookie=chunks-7",
+                "TestCookieC1=abcdefghi",
+                "TestCookieC2=jklmnopqr",
+                "TestCookieC3=stuvwxyz0",
+                "TestCookieC4=123456789",
+                "TestCookieC5=ABCDEFGHI",
+                "TestCookieC6=JKLMNOPQR",
+                "TestCookieC7=STUVWXYZ"
             };
 
             _stringToAdd = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
diff --git a/src/Security/perf/Microbenchmarks/Microsoft.AspNetCore.Security.Microbenchmarks.csproj b/src/Security/perf/Microbenchmarks/Microsoft.AspNetCore.Security.Microbenchmarks.csproj
@@ -1,20 +1,20 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
-
+<Project Sdk="Microsoft.NET.Sdk">
+  
   <PropertyGroup>
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <OutputType>Exe</OutputType>
     <ServerGarbageCollection>true</ServerGarbageCollection>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <RootNamespace>Microsoft.AspNetCore.Security</RootNamespace>
   </PropertyGroup>
-
+  
   <ItemGroup>
-    <Reference Include="Microsoft.AspNetCore.Authorization.Policy" />
     <Reference Include="BenchmarkDotNet" />
+    <Reference Include="Microsoft.AspNetCore.Authorization.Policy" />
+    <Reference Include="Microsoft.AspNetCore.Authentication.Cookies" />
     <Reference Include="Microsoft.AspNetCore.Http" />
     <Reference Include="Microsoft.Extensions.DependencyInjection" />
     <Reference Include="Microsoft.Extensions.Logging" />
-    <Reference Include="Microsoft.AspNetCore.Authentication.Cookies" />
 
     <Compile Include="$(SharedSourceRoot)BenchmarkRunner\*.cs" />
   </ItemGroup>

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo`
`101`	`101`	`{`
`102`	`102`	`if (_logger == null)`
`103`	`103`	`{`
`104`		`- var services = _features.Get<Features.IServiceProvidersFeature>()?.RequestServices;`
	`104`	`+ var services = _features.Get<IServiceProvidersFeature>()?.RequestServices;`
`105`	`105`	`_logger = services?.GetService<ILogger<ResponseCookies>>();`
`106`	`106`	`}`
`107`	`107`
`@@ -131,7 +131,8 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo`
`131`	`131`
`132`	`132`	`foreach (var keyValuePair in keyValuePairs)`
`133`	`133`	`{`
`134`		`- cookies[position] = string.Concat(_enableCookieNameEncoding ? Uri.EscapeDataString(keyValuePair.Key) : keyValuePair.Key, "=", Uri.EscapeDataString(keyValuePair.Value), cookierHeaderValue);`
	`134`	`+ var key = _enableCookieNameEncoding ? Uri.EscapeDataString(keyValuePair.Key) : keyValuePair.Key;`
	`135`	`+ cookies[position] = string.Concat(key, "=", Uri.EscapeDataString(keyValuePair.Value), cookierHeaderValue);`
`135`	`136`	`position++;`
`136`	`137`	`}`
`137`	`138`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Copyright (c) .NET Foundation. All rights reserved.`
	`1`	`+// Copyright (c) .NET Foundation. All rights reserved.`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
`4`	`4`	`using System;`
Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,7 @@ public void Append(ReadOnlySpan<KeyValuePair<string, string>> keyValuePairs, Coo`
`161`	`161`	`throw new ArgumentNullException(nameof(options));`
`162`	`162`	`}`
`163`	`163`
`164`		`- var nonSuppressedValues = new List<KeyValuePair<string, string>>();`
	`164`	`+ var nonSuppressedValues = new List<KeyValuePair<string, string>>(keyValuePairs.Length);`
`165`	`165`
`166`	`166`	`foreach (var keyValuePair in keyValuePairs)`
`167`	`167`	`{`