diff --git a/samples/SampleApp/PooledHttpContextFactory.cs b/samples/SampleApp/PooledHttpContextFactory.cs index 6743336466..c61e139ac3 100644 --- a/samples/SampleApp/PooledHttpContextFactory.cs +++ b/samples/SampleApp/PooledHttpContextFactory.cs @@ -12,7 +12,6 @@ namespace SampleApp { public class PooledHttpContextFactory : IHttpContextFactory { - private readonly ObjectPool _builderPool; private readonly IHttpContextAccessor _httpContextAccessor; private readonly Stack _pool = new Stack(); @@ -28,7 +27,6 @@ namespace SampleApp throw new ArgumentNullException(nameof(poolProvider)); } - _builderPool = poolProvider.CreateStringBuilderPool(); _httpContextAccessor = httpContextAccessor; } @@ -39,9 +37,6 @@ namespace SampleApp throw new ArgumentNullException(nameof(featureCollection)); } - var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool); - featureCollection.Set(responseCookiesFeature); - PooledHttpContext httpContext = null; lock (_pool) { diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/project.json b/src/Microsoft.AspNetCore.Http.Abstractions/project.json index 620aec35eb..9b73f97bab 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/project.json +++ b/src/Microsoft.AspNetCore.Http.Abstractions/project.json @@ -29,7 +29,8 @@ "type": "build" }, "NETStandard.Library": "1.6.1-*", - "System.Text.Encodings.Web": "4.3.0-*" + "System.Text.Encodings.Web": "4.3.0-*", + "Microsoft.Extensions.Primitives": "1.1.0-*" }, "frameworks": { "net451": { diff --git a/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs b/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs index a62a02f4dc..0d9444b0f5 100644 --- a/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs +++ b/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs @@ -16,9 +16,6 @@ namespace Microsoft.AspNetCore.Http.Features // Lambda hoisted to static readonly field to improve inlining https://github.com/dotnet/roslyn/issues/13624 private readonly static Func _nullResponseFeature = f => null; - // Object pool will be null only in test scenarios e.g. if code news up a DefaultHttpContext. - private readonly ObjectPool _builderPool; - private FeatureReferences _features; private IResponseCookies _cookiesCollection; @@ -50,7 +47,6 @@ namespace Microsoft.AspNetCore.Http.Features } _features = new FeatureReferences(features); - _builderPool = builderPool; } private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, _nullResponseFeature); @@ -63,7 +59,7 @@ namespace Microsoft.AspNetCore.Http.Features if (_cookiesCollection == null) { var headers = HttpResponseFeature.Headers; - _cookiesCollection = new ResponseCookies(headers, _builderPool); + _cookiesCollection = new ResponseCookies(headers, null); } return _cookiesCollection; diff --git a/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs b/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs index f2abfa8188..80619206a3 100644 --- a/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs +++ b/src/Microsoft.AspNetCore.Http/HttpContextFactory.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.Text; using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; @@ -11,7 +10,6 @@ namespace Microsoft.AspNetCore.Http { public class HttpContextFactory : IHttpContextFactory { - private readonly ObjectPool _builderPool; private readonly IHttpContextAccessor _httpContextAccessor; private readonly FormOptions _formOptions; @@ -31,7 +29,6 @@ namespace Microsoft.AspNetCore.Http throw new ArgumentNullException(nameof(formOptions)); } - _builderPool = poolProvider.CreateStringBuilderPool(); _formOptions = formOptions.Value; _httpContextAccessor = httpContextAccessor; } @@ -43,9 +40,6 @@ namespace Microsoft.AspNetCore.Http throw new ArgumentNullException(nameof(featureCollection)); } - var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool); - featureCollection.Set(responseCookiesFeature); - var httpContext = new DefaultHttpContext(featureCollection); if (_httpContextAccessor != null) { diff --git a/src/Microsoft.AspNetCore.Http/Internal/ResponseCookies.cs b/src/Microsoft.AspNetCore.Http/Internal/ResponseCookies.cs index 4a8538371b..e7f2d12039 100644 --- a/src/Microsoft.AspNetCore.Http/Internal/ResponseCookies.cs +++ b/src/Microsoft.AspNetCore.Http/Internal/ResponseCookies.cs @@ -15,8 +15,6 @@ namespace Microsoft.AspNetCore.Http.Internal /// public class ResponseCookies : IResponseCookies { - private readonly ObjectPool _builderPool; - /// /// Create a new wrapper. /// @@ -30,7 +28,6 @@ namespace Microsoft.AspNetCore.Http.Internal } Headers = headers; - _builderPool = builderPool; } private IHeaderDictionary Headers { get; set; } @@ -44,25 +41,7 @@ namespace Microsoft.AspNetCore.Http.Internal { Path = "/" }; - - string cookieValue; - if (_builderPool == null) - { - cookieValue = setCookieHeaderValue.ToString(); - } - else - { - var stringBuilder = _builderPool.Get(); - try - { - setCookieHeaderValue.AppendToStringBuilder(stringBuilder); - cookieValue = stringBuilder.ToString(); - } - finally - { - _builderPool.Return(stringBuilder); - } - } + var cookieValue = setCookieHeaderValue.ToString(); Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue); } @@ -86,24 +65,7 @@ namespace Microsoft.AspNetCore.Http.Internal HttpOnly = options.HttpOnly, }; - string cookieValue; - if (_builderPool == null) - { - cookieValue = setCookieHeaderValue.ToString(); - } - else - { - var stringBuilder = _builderPool.Get(); - try - { - setCookieHeaderValue.AppendToStringBuilder(stringBuilder); - cookieValue = stringBuilder.ToString(); - } - finally - { - _builderPool.Return(stringBuilder); - } - } + var cookieValue = setCookieHeaderValue.ToString(); Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue); } diff --git a/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs b/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs index 7f790d6617..7fda7571a3 100644 --- a/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs +++ b/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Text; +using Microsoft.Extensions.Primitives; namespace Microsoft.Net.Http.Headers { @@ -17,6 +18,8 @@ namespace Microsoft.Net.Http.Headers private const string PathToken = "path"; private const string SecureToken = "secure"; private const string HttpOnlyToken = "httponly"; + private const string SeparatorToken = "; "; + private const string EqualsToken = "="; private const string DefaultPath = "/"; // TODO: Used? private static readonly HttpHeaderParser SingleValueParser @@ -89,10 +92,91 @@ namespace Microsoft.Net.Http.Headers // name="val ue"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; httponly public override string ToString() { - StringBuilder header = new StringBuilder(); - AppendToStringBuilder(header); + var length = _name.Length + EqualsToken.Length + _value.Length; - return header.ToString(); + string expires = null; + string maxAge = null; + + if (Expires.HasValue) + { + expires = HeaderUtilities.FormatDate(Expires.Value); + length += SeparatorToken.Length + ExpiresToken.Length + EqualsToken.Length + expires.Length; + } + + if (MaxAge.HasValue) + { + maxAge = HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds); + length += SeparatorToken.Length + MaxAgeToken.Length + EqualsToken.Length + maxAge.Length; + } + + if (Domain != null) + { + length += SeparatorToken.Length + DomainToken.Length + EqualsToken.Length + Domain.Length; + } + + if (Path != null) + { + length += SeparatorToken.Length + PathToken.Length + EqualsToken.Length + Path.Length; + } + + if (Secure) + { + length += SeparatorToken.Length + SecureToken.Length; + } + + if (HttpOnly) + { + length += SeparatorToken.Length + HttpOnlyToken.Length; + } + + var sb = new InplaceStringBuilder(length); + + sb.Append(_name); + sb.Append(EqualsToken); + sb.Append(_value); + + if (expires != null) + { + AppendSegment(ref sb, ExpiresToken, expires); + } + + if (maxAge != null) + { + AppendSegment(ref sb, MaxAgeToken, maxAge); + } + + if (Domain != null) + { + AppendSegment(ref sb, DomainToken, Domain); + } + + if (Path != null) + { + AppendSegment(ref sb, PathToken, Path); + } + + if (Secure) + { + AppendSegment(ref sb, SecureToken, null); + } + + if (HttpOnly) + { + AppendSegment(ref sb, HttpOnlyToken, null); + } + + return sb.ToString(); + } + + private static void AppendSegment(ref InplaceStringBuilder builder, string name, string value) + { + builder.Append(SeparatorToken); + builder.Append(name); + if (value != null) + { + builder.Append(EqualsToken); + builder.Append(value); + } } /// diff --git a/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs b/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs index f5625f0fc6..77c7697c82 100644 --- a/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs +++ b/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs @@ -11,27 +11,11 @@ namespace Microsoft.AspNetCore.Http.Tests { public class ResponseCookiesTest { - private static readonly ObjectPool _builderPool = - new DefaultObjectPoolProvider().Create(new StringBuilderPooledObjectPolicy()); - - public static TheoryData BuilderPoolData - { - get - { - return new TheoryData> - { - null, - _builderPool, - }; - } - } - - [Theory] - [MemberData(nameof(BuilderPoolData))] - public void DeleteCookieShouldSetDefaultPath(ObjectPool builderPool) + [Fact] + public void DeleteCookieShouldSetDefaultPath() { var headers = new HeaderDictionary(); - var cookies = new ResponseCookies(headers, builderPool); + var cookies = new ResponseCookies(headers, null); var testcookie = "TestCookie"; cookies.Delete(testcookie); @@ -43,12 +27,11 @@ namespace Microsoft.AspNetCore.Http.Tests Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]); } - [Theory] - [MemberData(nameof(BuilderPoolData))] - public void NoParamsDeleteRemovesCookieCreatedByAdd(ObjectPool builderPool) + [Fact] + public void NoParamsDeleteRemovesCookieCreatedByAdd() { var headers = new HeaderDictionary(); - var cookies = new ResponseCookies(headers, builderPool); + var cookies = new ResponseCookies(headers, null); var testcookie = "TestCookie"; cookies.Append(testcookie, testcookie); @@ -66,15 +49,15 @@ namespace Microsoft.AspNetCore.Http.Tests get { // key, value, object pool, expected - return new TheoryData, string> + return new TheoryData { - { "key", "value", null, "key=value" }, - { "key,", "!value", null, "key%2C=%21value" }, - { "ke#y,", "val^ue", null, "ke%23y%2C=val%5Eue" }, - { "key", "value", _builderPool, "key=value" }, - { "key,", "!value", _builderPool, "key%2C=%21value" }, - { "ke#y,", "val^ue", _builderPool, "ke%23y%2C=val%5Eue" }, - { "base64", "QUI+REU/Rw==", _builderPool, "base64=QUI%2BREU%2FRw%3D%3D" }, + { "key", "value", "key=value" }, + { "key,", "!value", "key%2C=%21value" }, + { "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" }, + { "key", "value", "key=value" }, + { "key,", "!value", "key%2C=%21value" }, + { "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" }, + { "base64", "QUI+REU/Rw==", "base64=QUI%2BREU%2FRw%3D%3D" }, }; } } @@ -84,11 +67,10 @@ namespace Microsoft.AspNetCore.Http.Tests public void EscapesKeyValuesBeforeSettingCookie( string key, string value, - ObjectPool builderPool, string expected) { var headers = new HeaderDictionary(); - var cookies = new ResponseCookies(headers, builderPool); + var cookies = new ResponseCookies(headers, null); cookies.Append(key, value);