diff --git a/samples/SampleApp/PooledHttpContextFactory.cs b/samples/SampleApp/PooledHttpContextFactory.cs index 67433364..c61e139a 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 @@ public PooledHttpContextFactory(ObjectPoolProvider poolProvider, IHttpContextAcc throw new ArgumentNullException(nameof(poolProvider)); } - _builderPool = poolProvider.CreateStringBuilderPool(); _httpContextAccessor = httpContextAccessor; } @@ -39,9 +37,6 @@ public HttpContext Create(IFeatureCollection featureCollection) 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 620aec35..9b73f97b 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 a62a02f4..0d9444b0 100644 --- a/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs +++ b/src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs @@ -16,9 +16,6 @@ public class ResponseCookiesFeature : IResponseCookiesFeature // 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 @@ public ResponseCookiesFeature(IFeatureCollection features, ObjectPool(features); - _builderPool = builderPool; } private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, _nullResponseFeature); @@ -63,7 +59,7 @@ public IResponseCookies Cookies 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 f2abfa81..80619206 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 @@ public HttpContextFactory(ObjectPoolProvider poolProvider, IOptions throw new ArgumentNullException(nameof(formOptions)); } - _builderPool = poolProvider.CreateStringBuilderPool(); _formOptions = formOptions.Value; _httpContextAccessor = httpContextAccessor; } @@ -43,9 +40,6 @@ public HttpContext Create(IFeatureCollection featureCollection) 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 4a853837..e7f2d120 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 @@ public ResponseCookies(IHeaderDictionary headers, ObjectPool buil } Headers = headers; - _builderPool = builderPool; } private IHeaderDictionary Headers { get; set; } @@ -44,25 +41,7 @@ public void Append(string key, string value) { 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 @@ public void Append(string key, string value, CookieOptions options) 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 7f790d66..ea1b3686 100644 --- a/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs +++ b/src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Text; +using Microsoft.Extensions.Primitives; namespace Microsoft.Net.Http.Headers { @@ -89,10 +90,103 @@ public string Value // 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); + const string equals = "="; + const string separator = "; "; - return header.ToString(); + var length = _name.Length + equals.Length + _value.Length; + + string expires = null; + string maxAge = null; + + if (Expires.HasValue) + { + expires = HeaderUtilities.FormatDate(Expires.Value); + length += separator.Length + ExpiresToken.Length + equals.Length + expires.Length; + } + + if (MaxAge.HasValue) + { + maxAge = HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds); + length += separator.Length + MaxAgeToken.Length + equals.Length + maxAge.Length; + } + + if (Domain != null) + { + length += separator.Length + DomainToken.Length + equals.Length + Domain.Length; + } + + if (Path != null) + { + length += separator.Length + PathToken.Length + equals.Length + Path.Length; + } + + if (Secure) + { + length += separator.Length + SecureToken.Length; + } + + if (HttpOnly) + { + length += separator.Length + HttpOnlyToken.Length; + } + + var sb = new InplaceStringBuilder(length); + + sb.Append(_name); + sb.Append(equals); + sb.Append(_value); + + if (expires != null) + { + sb.Append(separator); + + sb.Append(ExpiresToken); + sb.Append(equals); + sb.Append(expires); + } + + if (maxAge != null) + { + sb.Append(separator); + + sb.Append(MaxAgeToken); + sb.Append(equals); + sb.Append(maxAge); + } + + if (Domain != null) + { + sb.Append(separator); + + sb.Append(DomainToken); + sb.Append(equals); + sb.Append(Domain); + } + + if (Path != null) + { + sb.Append(separator); + + sb.Append(PathToken); + sb.Append(equals); + sb.Append(Path); + } + + if (Secure) + { + sb.Append(separator); + + sb.Append(SecureToken); + } + + if (HttpOnly) + { + sb.Append(separator); + + sb.Append(HttpOnlyToken); + } + + return sb.ToString(); } /// diff --git a/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs b/test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs index f5625f0f..77c7697c 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 @@ public void DeleteCookieShouldSetDefaultPath(ObjectPool builderPo 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 @@ public static TheoryData EscapesKeyValuesBeforeSettingCookieData 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 @@ public static TheoryData EscapesKeyValuesBeforeSettingCookieData 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);