Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Commit 4569653

Browse files
benaadamspakrym
benaadams
authored and
pakrym
committed
Don't allocate for ResponseCookiesFeature
1 parent 063d6ec commit 4569653

File tree

7 files changed

+107
-93
lines changed

7 files changed

+107
-93
lines changed

‎samples/SampleApp/PooledHttpContextFactory.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ namespace SampleApp
1212
{
1313
public class PooledHttpContextFactory : IHttpContextFactory
1414
{
15-
private readonly ObjectPool<StringBuilder> _builderPool;
1615
private readonly IHttpContextAccessor _httpContextAccessor;
1716
private readonly Stack<PooledHttpContext> _pool = new Stack<PooledHttpContext>();
1817

@@ -28,7 +27,6 @@ public PooledHttpContextFactory(ObjectPoolProvider poolProvider, IHttpContextAcc
2827
throw new ArgumentNullException(nameof(poolProvider));
2928
}
3029

31-
_builderPool = poolProvider.CreateStringBuilderPool();
3230
_httpContextAccessor = httpContextAccessor;
3331
}
3432

@@ -39,9 +37,6 @@ public HttpContext Create(IFeatureCollection featureCollection)
3937
throw new ArgumentNullException(nameof(featureCollection));
4038
}
4139

42-
var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
43-
featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
44-
4540
PooledHttpContext httpContext = null;
4641
lock (_pool)
4742
{

‎src/Microsoft.AspNetCore.Http.Abstractions/project.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
"type": "build"
3030
},
3131
"NETStandard.Library": "1.6.1-*",
32-
"System.Text.Encodings.Web": "4.3.0-*"
32+
"System.Text.Encodings.Web": "4.3.0-*",
33+
"Microsoft.Extensions.Primitives": "1.1.0-*"
3334
},
3435
"frameworks": {
3536
"net451": {

‎src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ public class ResponseCookiesFeature : IResponseCookiesFeature
1616
// Lambda hoisted to static readonly field to improve inlining https://github.com/dotnet/roslyn/issues/13624
1717
private readonly static Func<IFeatureCollection, IHttpResponseFeature> _nullResponseFeature = f => null;
1818

19-
// Object pool will be null only in test scenarios e.g. if code news up a DefaultHttpContext.
20-
private readonly ObjectPool<StringBuilder> _builderPool;
21-
2219
private FeatureReferences<IHttpResponseFeature> _features;
2320
private IResponseCookies _cookiesCollection;
2421

@@ -50,7 +47,6 @@ public ResponseCookiesFeature(IFeatureCollection features, ObjectPool<StringBuil
5047
}
5148

5249
_features = new FeatureReferences<IHttpResponseFeature>(features);
53-
_builderPool = builderPool;
5450
}
5551

5652
private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, _nullResponseFeature);
@@ -63,7 +59,7 @@ public IResponseCookies Cookies
6359
if (_cookiesCollection == null)
6460
{
6561
var headers = HttpResponseFeature.Headers;
66-
_cookiesCollection = new ResponseCookies(headers, _builderPool);
62+
_cookiesCollection = new ResponseCookies(headers, null);
6763
}
6864

6965
return _cookiesCollection;

‎src/Microsoft.AspNetCore.Http/HttpContextFactory.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Text;
65
using Microsoft.AspNetCore.Http.Features;
76
using Microsoft.Extensions.ObjectPool;
87
using Microsoft.Extensions.Options;
@@ -11,7 +10,6 @@ namespace Microsoft.AspNetCore.Http
1110
{
1211
public class HttpContextFactory : IHttpContextFactory
1312
{
14-
private readonly ObjectPool<StringBuilder> _builderPool;
1513
private readonly IHttpContextAccessor _httpContextAccessor;
1614
private readonly FormOptions _formOptions;
1715

@@ -31,7 +29,6 @@ public HttpContextFactory(ObjectPoolProvider poolProvider, IOptions<FormOptions>
3129
throw new ArgumentNullException(nameof(formOptions));
3230
}
3331

34-
_builderPool = poolProvider.CreateStringBuilderPool();
3532
_formOptions = formOptions.Value;
3633
_httpContextAccessor = httpContextAccessor;
3734
}
@@ -43,9 +40,6 @@ public HttpContext Create(IFeatureCollection featureCollection)
4340
throw new ArgumentNullException(nameof(featureCollection));
4441
}
4542

46-
var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
47-
featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
48-
4943
var httpContext = new DefaultHttpContext(featureCollection);
5044
if (_httpContextAccessor != null)
5145
{

‎src/Microsoft.AspNetCore.Http/Internal/ResponseCookies.cs

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ namespace Microsoft.AspNetCore.Http.Internal
1515
/// </summary>
1616
public class ResponseCookies : IResponseCookies
1717
{
18-
private readonly ObjectPool<StringBuilder> _builderPool;
19-
2018
/// <summary>
2119
/// Create a new wrapper.
2220
/// </summary>
@@ -30,7 +28,6 @@ public ResponseCookies(IHeaderDictionary headers, ObjectPool<StringBuilder> buil
3028
}
3129

3230
Headers = headers;
33-
_builderPool = builderPool;
3431
}
3532

3633
private IHeaderDictionary Headers { get; set; }
@@ -44,25 +41,7 @@ public void Append(string key, string value)
4441
{
4542
Path = "/"
4643
};
47-
48-
string cookieValue;
49-
if (_builderPool == null)
50-
{
51-
cookieValue = setCookieHeaderValue.ToString();
52-
}
53-
else
54-
{
55-
var stringBuilder = _builderPool.Get();
56-
try
57-
{
58-
setCookieHeaderValue.AppendToStringBuilder(stringBuilder);
59-
cookieValue = stringBuilder.ToString();
60-
}
61-
finally
62-
{
63-
_builderPool.Return(stringBuilder);
64-
}
65-
}
44+
var cookieValue = setCookieHeaderValue.ToString();
6645

6746
Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
6847
}
@@ -86,24 +65,7 @@ public void Append(string key, string value, CookieOptions options)
8665
HttpOnly = options.HttpOnly,
8766
};
8867

89-
string cookieValue;
90-
if (_builderPool == null)
91-
{
92-
cookieValue = setCookieHeaderValue.ToString();
93-
}
94-
else
95-
{
96-
var stringBuilder = _builderPool.Get();
97-
try
98-
{
99-
setCookieHeaderValue.AppendToStringBuilder(stringBuilder);
100-
cookieValue = stringBuilder.ToString();
101-
}
102-
finally
103-
{
104-
_builderPool.Return(stringBuilder);
105-
}
106-
}
68+
var cookieValue = setCookieHeaderValue.ToString();
10769

10870
Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
10971
}

‎src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs

Lines changed: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics.Contracts;
77
using System.Text;
8+
using Microsoft.Extensions.Primitives;
89

910
namespace Microsoft.Net.Http.Headers
1011
{
@@ -17,6 +18,8 @@ public class SetCookieHeaderValue
1718
private const string PathToken = "path";
1819
private const string SecureToken = "secure";
1920
private const string HttpOnlyToken = "httponly";
21+
private const string SeparatorToken = "; ";
22+
private const string EqualsToken = "=";
2023
private const string DefaultPath = "/"; // TODO: Used?
2124

2225
private static readonly HttpHeaderParser<SetCookieHeaderValue> SingleValueParser
@@ -89,10 +92,91 @@ public string Value
8992
// name="val ue"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; httponly
9093
public override string ToString()
9194
{
92-
StringBuilder header = new StringBuilder();
93-
AppendToStringBuilder(header);
95+
var length = _name.Length + EqualsToken.Length + _value.Length;
9496

95-
return header.ToString();
97+
string expires = null;
98+
string maxAge = null;
99+
100+
if (Expires.HasValue)
101+
{
102+
expires = HeaderUtilities.FormatDate(Expires.Value);
103+
length += SeparatorToken.Length + ExpiresToken.Length + EqualsToken.Length + expires.Length;
104+
}
105+
106+
if (MaxAge.HasValue)
107+
{
108+
maxAge = HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds);
109+
length += SeparatorToken.Length + MaxAgeToken.Length + EqualsToken.Length + maxAge.Length;
110+
}
111+
112+
if (Domain != null)
113+
{
114+
length += SeparatorToken.Length + DomainToken.Length + EqualsToken.Length + Domain.Length;
115+
}
116+
117+
if (Path != null)
118+
{
119+
length += SeparatorToken.Length + PathToken.Length + EqualsToken.Length + Path.Length;
120+
}
121+
122+
if (Secure)
123+
{
124+
length += SeparatorToken.Length + SecureToken.Length;
125+
}
126+
127+
if (HttpOnly)
128+
{
129+
length += SeparatorToken.Length + HttpOnlyToken.Length;
130+
}
131+
132+
var sb = new InplaceStringBuilder(length);
133+
134+
sb.Append(_name);
135+
sb.Append(EqualsToken);
136+
sb.Append(_value);
137+
138+
if (expires != null)
139+
{
140+
AppendSegment(ref sb, ExpiresToken, expires);
141+
}
142+
143+
if (maxAge != null)
144+
{
145+
AppendSegment(ref sb, MaxAgeToken, maxAge);
146+
}
147+
148+
if (Domain != null)
149+
{
150+
AppendSegment(ref sb, DomainToken, Domain);
151+
}
152+
153+
if (Path != null)
154+
{
155+
AppendSegment(ref sb, PathToken, Path);
156+
}
157+
158+
if (Secure)
159+
{
160+
AppendSegment(ref sb, SecureToken, null);
161+
}
162+
163+
if (HttpOnly)
164+
{
165+
AppendSegment(ref sb, HttpOnlyToken, null);
166+
}
167+
168+
return sb.ToString();
169+
}
170+
171+
private static void AppendSegment(ref InplaceStringBuilder builder, string name, string value)
172+
{
173+
builder.Append(SeparatorToken);
174+
builder.Append(name);
175+
if (value != null)
176+
{
177+
builder.Append(EqualsToken);
178+
builder.Append(value);
179+
}
96180
}
97181

98182
/// <summary>

‎test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,11 @@ namespace Microsoft.AspNetCore.Http.Tests
1111
{
1212
public class ResponseCookiesTest
1313
{
14-
private static readonly ObjectPool<StringBuilder> _builderPool =
15-
new DefaultObjectPoolProvider().Create<StringBuilder>(new StringBuilderPooledObjectPolicy());
16-
17-
public static TheoryData BuilderPoolData
18-
{
19-
get
20-
{
21-
return new TheoryData<ObjectPool<StringBuilder>>
22-
{
23-
null,
24-
_builderPool,
25-
};
26-
}
27-
}
28-
29-
[Theory]
30-
[MemberData(nameof(BuilderPoolData))]
31-
public void DeleteCookieShouldSetDefaultPath(ObjectPool<StringBuilder> builderPool)
14+
[Fact]
15+
public void DeleteCookieShouldSetDefaultPath()
3216
{
3317
var headers = new HeaderDictionary();
34-
var cookies = new ResponseCookies(headers, builderPool);
18+
var cookies = new ResponseCookies(headers, null);
3519
var testcookie = "TestCookie";
3620

3721
cookies.Delete(testcookie);
@@ -43,12 +27,11 @@ public void DeleteCookieShouldSetDefaultPath(ObjectPool<StringBuilder> builderPo
4327
Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]);
4428
}
4529

46-
[Theory]
47-
[MemberData(nameof(BuilderPoolData))]
48-
public void NoParamsDeleteRemovesCookieCreatedByAdd(ObjectPool<StringBuilder> builderPool)
30+
[Fact]
31+
public void NoParamsDeleteRemovesCookieCreatedByAdd()
4932
{
5033
var headers = new HeaderDictionary();
51-
var cookies = new ResponseCookies(headers, builderPool);
34+
var cookies = new ResponseCookies(headers, null);
5235
var testcookie = "TestCookie";
5336

5437
cookies.Append(testcookie, testcookie);
@@ -66,15 +49,15 @@ public static TheoryData EscapesKeyValuesBeforeSettingCookieData
6649
get
6750
{
6851
// key, value, object pool, expected
69-
return new TheoryData<string, string, ObjectPool<StringBuilder>, string>
52+
return new TheoryData<string, string, string>
7053
{
71-
{ "key", "value", null, "key=value" },
72-
{ "key,", "!value", null, "key%2C=%21value" },
73-
{ "ke#y,", "val^ue", null, "ke%23y%2C=val%5Eue" },
74-
{ "key", "value", _builderPool, "key=value" },
75-
{ "key,", "!value", _builderPool, "key%2C=%21value" },
76-
{ "ke#y,", "val^ue", _builderPool, "ke%23y%2C=val%5Eue" },
77-
{ "base64", "QUI+REU/Rw==", _builderPool, "base64=QUI%2BREU%2FRw%3D%3D" },
54+
{ "key", "value", "key=value" },
55+
{ "key,", "!value", "key%2C=%21value" },
56+
{ "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" },
57+
{ "key", "value", "key=value" },
58+
{ "key,", "!value", "key%2C=%21value" },
59+
{ "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" },
60+
{ "base64", "QUI+REU/Rw==", "base64=QUI%2BREU%2FRw%3D%3D" },
7861
};
7962
}
8063
}
@@ -84,11 +67,10 @@ public static TheoryData EscapesKeyValuesBeforeSettingCookieData
8467
public void EscapesKeyValuesBeforeSettingCookie(
8568
string key,
8669
string value,
87-
ObjectPool<StringBuilder> builderPool,
8870
string expected)
8971
{
9072
var headers = new HeaderDictionary();
91-
var cookies = new ResponseCookies(headers, builderPool);
73+
var cookies = new ResponseCookies(headers, null);
9274

9375
cookies.Append(key, value);
9476

0 commit comments

Comments
 (0)