Skip to content

Commit 6893565

Browse files
JamesNKJamesNK
authored
Fix Kestrel psuedo header reuse (#38585)
1 parent 4656f15 commit 6893565

File tree

9 files changed

+95
-13
lines changed

9 files changed

+95
-13
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7102,6 +7102,11 @@ protected override bool CopyToFast(KeyValuePair<string, StringValues>[] array, i
71027102
return true;
71037103
}
71047104

7105+
internal void ClearPseudoRequestHeaders()
7106+
{
7107+
_pseudoBits = _bits & 240;
7108+
_bits &= ~240;
7109+
}
71057110

71067111
[MethodImpl(MethodImplOptions.AggressiveInlining)]
71077112
internal static unsafe ushort ReadUnalignedLittleEndian_ushort(ref byte source)
@@ -17014,4 +17019,4 @@ public bool MoveNext()
1701417019
}
1701517020
}
1701617021
}
17017-
}
17022+
}

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ internal sealed partial class HttpRequestHeaders : HttpHeaders
1919
{
2020
private EnumeratorCache? _enumeratorCache;
2121
private long _previousBits;
22+
private long _pseudoBits;
2223

2324
public bool ReuseHeaderValues { get; set; }
2425
public Func<string, Encoding?> EncodingSelector { get; set; }
@@ -54,16 +55,19 @@ protected override void ClearFast()
5455
if (!ReuseHeaderValues)
5556
{
5657
// If we aren't reusing headers clear them all
57-
Clear(_bits);
58+
Clear(_bits | _pseudoBits);
5859
}
5960
else
6061
{
6162
// If we are reusing headers, store the currently set headers for comparison later
62-
_previousBits = _bits;
63+
// Pseudo header bits were cleared at the start of a request to hide from the user.
64+
// Keep those values for reuse.
65+
_previousBits = _bits | _pseudoBits;
6366
}
6467

6568
// Mark no headers as currently in use
6669
_bits = 0;
70+
_pseudoBits = 0;
6771
// Clear ContentLength and any unknown headers as we will never reuse them
6872
_contentLength = null;
6973
MaybeUnknown?.Clear();

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,10 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
208208
// We don't need any of the parameters because we don't implement BeginRead to actually
209209
// do the reading from a pipeline, nor do we use endConnection to report connection-level errors.
210210
endConnection = !TryValidatePseudoHeaders();
211+
212+
// Suppress pseudo headers from the public headers collection.
213+
HttpRequestHeaders.ClearPseudoRequestHeaders();
214+
211215
return true;
212216
}
213217

@@ -249,7 +253,6 @@ private bool TryValidatePseudoHeaders()
249253
// enabling the use of HTTP to interact with non - HTTP services.
250254
// A common example is TLS termination.
251255
var headerScheme = HttpRequestHeaders.HeaderScheme.ToString();
252-
HttpRequestHeaders.HeaderScheme = default; // Suppress pseduo headers from the public headers collection.
253256
if (!ReferenceEquals(headerScheme, Scheme) &&
254257
!string.Equals(headerScheme, Scheme, StringComparison.OrdinalIgnoreCase))
255258
{
@@ -266,7 +269,6 @@ private bool TryValidatePseudoHeaders()
266269
// :path (and query) - Required
267270
// Must start with / except may be * for OPTIONS
268271
var path = HttpRequestHeaders.HeaderPath.ToString();
269-
HttpRequestHeaders.HeaderPath = default; // Suppress pseduo headers from the public headers collection.
270272
RawTarget = path;
271273

272274
// OPTIONS - https://tools.ietf.org/html/rfc7540#section-8.1.2.3
@@ -304,7 +306,6 @@ private bool TryValidateMethod()
304306
{
305307
// :method
306308
_methodText = HttpRequestHeaders.HeaderMethod.ToString();
307-
HttpRequestHeaders.HeaderMethod = default; // Suppress pseduo headers from the public headers collection.
308309
Method = HttpUtilities.GetKnownMethod(_methodText);
309310

310311
if (Method == HttpMethod.None)
@@ -331,7 +332,6 @@ private bool TryValidateAuthorityAndHost(out string hostText)
331332
// Prefer this over Host
332333

333334
var authority = HttpRequestHeaders.HeaderAuthority;
334-
HttpRequestHeaders.HeaderAuthority = default; // Suppress pseduo headers from the public headers collection.
335335
var host = HttpRequestHeaders.HeaderHost;
336336
if (!StringValues.IsNullOrEmpty(authority))
337337
{

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,10 @@ protected override MessageBody CreateMessageBody()
755755
protected override bool TryParseRequest(ReadResult result, out bool endConnection)
756756
{
757757
endConnection = !TryValidatePseudoHeaders();
758+
759+
// Suppress pseudo headers from the public headers collection.
760+
HttpRequestHeaders.ClearPseudoRequestHeaders();
761+
758762
return true;
759763
}
760764

@@ -791,7 +795,6 @@ private bool TryValidatePseudoHeaders()
791795
// proxy or gateway can translate requests for non - HTTP schemes,
792796
// enabling the use of HTTP to interact with non - HTTP services.
793797
var headerScheme = HttpRequestHeaders.HeaderScheme.ToString();
794-
HttpRequestHeaders.HeaderScheme = default; // Suppress pseduo headers from the public headers collection.
795798
if (!ReferenceEquals(headerScheme, Scheme) &&
796799
!string.Equals(headerScheme, Scheme, StringComparison.OrdinalIgnoreCase))
797800
{
@@ -808,7 +811,6 @@ private bool TryValidatePseudoHeaders()
808811
// :path (and query) - Required
809812
// Must start with / except may be * for OPTIONS
810813
var path = HttpRequestHeaders.HeaderPath.ToString();
811-
HttpRequestHeaders.HeaderPath = default; // Suppress pseduo headers from the public headers collection.
812814
RawTarget = path;
813815

814816
// OPTIONS - https://tools.ietf.org/html/rfc7540#section-8.1.2.3
@@ -847,7 +849,6 @@ private bool TryValidateMethod()
847849
{
848850
// :method
849851
_methodText = HttpRequestHeaders.HeaderMethod.ToString();
850-
HttpRequestHeaders.HeaderMethod = default; // Suppress pseduo headers from the public headers collection.
851852
Method = HttpUtilities.GetKnownMethod(_methodText);
852853

853854
if (Method == Http.HttpMethod.None)
@@ -874,7 +875,6 @@ private bool TryValidateAuthorityAndHost(out string hostText)
874875
// Prefer this over Host
875876

876877
var authority = HttpRequestHeaders.HeaderAuthority;
877-
HttpRequestHeaders.HeaderAuthority = default; // Suppress pseduo headers from the public headers collection.
878878
var host = HttpRequestHeaders.HeaderHost;
879879
if (!StringValues.IsNullOrEmpty(authority))
880880
{

‎src/Servers/Kestrel/Core/test/HttpRequestHeadersTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Linq;
7+
using System.Reflection;
78
using System.Text;
89
using Microsoft.AspNetCore.Http;
910
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
@@ -112,6 +113,23 @@ public void EntriesCanBeEnumeratedAfterResets()
112113
EnumerateEntries((IDictionary<string, StringValues>)headers);
113114
}
114115

116+
[Fact]
117+
public void ClearPseudoRequestHeadersPlusResetClearsHeaderReferenceValue()
118+
{
119+
const BindingFlags privateFlags = BindingFlags.NonPublic | BindingFlags.Instance;
120+
121+
HttpRequestHeaders headers = new HttpRequestHeaders(reuseHeaderValues: false);
122+
headers.HeaderMethod = "GET";
123+
headers.ClearPseudoRequestHeaders();
124+
headers.Reset();
125+
126+
// Hacky but required because header references is private.
127+
var headerReferences = typeof(HttpRequestHeaders).GetField("_headers", privateFlags).GetValue(headers);
128+
var methodValue = (StringValues)headerReferences.GetType().GetField("_Method").GetValue(headerReferences);
129+
130+
Assert.Equal(StringValues.Empty, methodValue);
131+
}
132+
115133
[Fact]
116134
public void EnumeratorNotReusedBeforeReset()
117135
{

‎src/Servers/Kestrel/shared/KnownHeaders.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class KnownHeaders
3939
HeaderNames.DNT,
4040
};
4141

42-
public static readonly string[] PsuedoHeaderNames = new[]
42+
public static readonly string[] PseudoHeaderNames = new[]
4343
{
4444
"Authority", // :authority
4545
"Method", // :method
@@ -50,7 +50,7 @@ public class KnownHeaders
5050

5151
public static readonly string[] NonApiHeaders =
5252
ObsoleteHeaderNames
53-
.Concat(PsuedoHeaderNames)
53+
.Concat(PseudoHeaderNames)
5454
.ToArray();
5555

5656
public static readonly string[] ApiHeaderNames =
@@ -59,6 +59,7 @@ public class KnownHeaders
5959
.ToArray();
6060

6161
public static readonly long InvalidH2H3ResponseHeadersBits;
62+
public static readonly long PseudoRequestHeadersBits;
6263

6364
static KnownHeaders()
6465
{
@@ -263,6 +264,11 @@ static KnownHeaders()
263264
.Where(header => invalidH2H3ResponseHeaders.Contains(header.Name))
264265
.Select(header => 1L << header.Index)
265266
.Aggregate((a, b) => a | b);
267+
268+
PseudoRequestHeadersBits = RequestHeaders
269+
.Where(header => PseudoHeaderNames.Contains(header.Identifier))
270+
.Select(header => 1L << header.Index)
271+
.Aggregate((a, b) => a | b);
266272
}
267273

268274
static string Each<T>(IEnumerable<T> values, Func<T, string> formatter)
@@ -1249,6 +1255,11 @@ internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
12491255
}}
12501256
}} while (tempBits != 0);
12511257
}}" : "")}{(loop.ClassName == "HttpRequestHeaders" ? $@"
1258+
internal void ClearPseudoRequestHeaders()
1259+
{{
1260+
_pseudoBits = _bits & {PseudoRequestHeadersBits};
1261+
_bits &= ~{PseudoRequestHeadersBits};
1262+
}}
12521263
{Each(new string[] { "ushort", "uint", "ulong" }, type => $@"
12531264
[MethodImpl(MethodImplOptions.AggressiveInlining)]
12541265
internal static unsafe {type} ReadUnalignedLittleEndian_{type}(ref byte source)

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ await ExpectAsync(Http2FrameType.HEADERS,
179179
withStreamId: 1);
180180

181181
var contentType1 = _receivedHeaders["Content-Type"];
182+
var authority1 = _receivedRequestFields.Authority;
183+
var path1 = _receivedRequestFields.Path;
182184

183185
// TriggerTick will trigger the stream to be returned to the pool so we can assert it
184186
TriggerTick();
@@ -194,8 +196,12 @@ await ExpectAsync(Http2FrameType.HEADERS,
194196
withStreamId: 3);
195197

196198
var contentType2 = _receivedHeaders["Content-Type"];
199+
var authority2 = _receivedRequestFields.Authority;
200+
var path2 = _receivedRequestFields.Path;
197201

198202
Assert.Same(contentType1, contentType2);
203+
Assert.Same(authority1, authority2);
204+
Assert.Same(path1, path2);
199205

200206
await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
201207
}

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ public Http2TestBase()
182182
_receivedRequestFields.Scheme = context.Request.Scheme;
183183
_receivedRequestFields.Path = context.Request.Path.Value;
184184
_receivedRequestFields.RawTarget = context.Features.Get<IHttpRequestFeature>().RawTarget;
185+
_receivedRequestFields.Authority = context.Request.Host.Value;
185186
foreach (var header in context.Request.Headers)
186187
{
187188
_receivedHeaders[header.Key] = header.Value.ToString();
@@ -1413,5 +1414,6 @@ public class RequestFields
14131414
public string Scheme { get; set; }
14141415
public string Path { get; set; }
14151416
public string RawTarget { get; set; }
1417+
public string Authority { get; set; }
14161418
}
14171419
}

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,42 @@ public async Task StreamPool_MultipleStreamsInSequence_PooledStreamReused()
336336
Assert.Same(streamContext1, streamContext2);
337337
}
338338

339+
[Fact]
340+
public async Task StreamPool_MultipleStreamsInSequence_KnownHeaderReused()
341+
{
342+
var headers = new[]
343+
{
344+
new KeyValuePair<string, string>(HeaderNames.Method, "Custom"),
345+
new KeyValuePair<string, string>(HeaderNames.Path, "/"),
346+
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
347+
new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
348+
new KeyValuePair<string, string>(HeaderNames.ContentType, "application/json"),
349+
};
350+
351+
string contentType = null;
352+
string authority = null;
353+
await Http3Api.InitializeConnectionAsync(async context =>
354+
{
355+
contentType = context.Request.ContentType;
356+
authority = context.Request.Host.Value;
357+
await _echoApplication(context);
358+
});
359+
360+
var streamContext1 = await MakeRequestAsync(0, headers, sendData: true, waitForServerDispose: true);
361+
var contentType1 = contentType;
362+
var authority1 = authority;
363+
364+
var streamContext2 = await MakeRequestAsync(1, headers, sendData: true, waitForServerDispose: true);
365+
var contentType2 = contentType;
366+
var authority2 = authority;
367+
368+
Assert.NotNull(contentType1);
369+
Assert.NotNull(authority1);
370+
371+
Assert.Same(contentType1, contentType2);
372+
Assert.Same(authority1, authority2);
373+
}
374+
339375
[Theory]
340376
[InlineData(10)]
341377
[InlineData(100)]

0 commit comments

Comments
 (0)