Skip to content

Commit 4c8f48d

Browse files
JamesNKJamesNK
committed
Add reasons for some HTTP/1.1 request body errors
1 parent 0990832 commit 4c8f48d

File tree

10 files changed

+106
-25
lines changed

10 files changed

+106
-25
lines changed

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -812,16 +812,11 @@ private void OnBadRequest(ReadOnlySequence<byte> requestData, BadHttpRequestExce
812812
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestHeaders);
813813
break;
814814
case RequestRejectionReason.TlsOverHttpError:
815-
case RequestRejectionReason.UnexpectedEndOfRequestContent:
816-
case RequestRejectionReason.BadChunkSuffix:
817-
case RequestRejectionReason.BadChunkSizeData:
818-
case RequestRejectionReason.ChunkedRequestIncomplete:
819-
case RequestRejectionReason.RequestBodyTooLarge:
820-
case RequestRejectionReason.RequestHeadersTimeout:
821-
case RequestRejectionReason.RequestBodyTimeout:
822-
case RequestRejectionReason.FinalTransferCodingNotChunked:
823-
case RequestRejectionReason.RequestBodyExceedsContentLength:
815+
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.TlsOverHttp);
816+
break;
824817
default:
818+
// In some scenarios the end reason might already be set to a more specific error
819+
// and attempting to set the reason again has no impact.
825820
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.OtherError);
826821
break;
827822
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.IO.Pipelines;
88
using System.Runtime.CompilerServices;
99
using Microsoft.AspNetCore.Connections;
10+
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
1011

1112
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
1213

@@ -247,6 +248,7 @@ protected override void OnReadStarting()
247248
if (_contentLength > maxRequestBodySize)
248249
{
249250
_context.DisableHttp1KeepAlive();
251+
KestrelMetrics.AddConnectionEndReason(_context.MetricsContext, ConnectionEndReason.MaxRequestBodySizeExceeded);
250252
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge, maxRequestBodySize.GetValueOrDefault().ToString(CultureInfo.InvariantCulture));
251253
}
252254
}
@@ -269,6 +271,7 @@ private void VerifyIsNotReading()
269271
{
270272
if (_readResult.IsCompleted)
271273
{
274+
KestrelMetrics.AddConnectionEndReason(_context.MetricsContext, ConnectionEndReason.UnexpectedEndOfRequestContent);
272275
KestrelBadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent);
273276
}
274277

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics;
5+
using System.Globalization;
56
using System.IO.Pipelines;
67
using Microsoft.AspNetCore.Connections;
78
using Microsoft.AspNetCore.Http;
@@ -114,6 +115,13 @@ protected async Task OnConsumeAsyncAwaited()
114115
}
115116
}
116117

118+
protected override void OnOnbservedBytesExceedMaxRequestBodySize(long? maxRequestBodySize)
119+
{
120+
_context.DisableHttp1KeepAlive();
121+
KestrelMetrics.AddConnectionEndReason(_context.MetricsContext, ConnectionEndReason.MaxRequestBodySizeExceeded);
122+
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge, maxRequestBodySize.GetValueOrDefault().ToString(CultureInfo.InvariantCulture));
123+
}
124+
117125
public static MessageBody For(
118126
HttpVersion httpVersion,
119127
HttpRequestHeaders headers,
@@ -226,6 +234,7 @@ protected void ThrowUnexpectedEndOfRequestContent()
226234
// closing the connection without a response as expected.
227235
((IHttpOutputAborter)_context).OnInputOrOutputCompleted();
228236

237+
KestrelMetrics.AddConnectionEndReason(_context.MetricsContext, ConnectionEndReason.UnexpectedEndOfRequestContent);
229238
KestrelBadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent);
230239
}
231240
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,15 @@ protected void AddAndCheckObservedBytes(long observedBytes)
191191
var maxRequestBodySize = _context.MaxRequestBodySize;
192192
if (_observedBytes > maxRequestBodySize)
193193
{
194-
_context.DisableHttp1KeepAlive();
195-
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge, maxRequestBodySize.GetValueOrDefault().ToString(CultureInfo.InvariantCulture));
194+
OnOnbservedBytesExceedMaxRequestBodySize(maxRequestBodySize);
196195
}
197196
}
198197

198+
protected virtual void OnOnbservedBytesExceedMaxRequestBodySize(long? maxRequestBodySize)
199+
{
200+
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge, maxRequestBodySize.GetValueOrDefault().ToString(CultureInfo.InvariantCulture));
201+
}
202+
199203
protected ValueTask<ReadResult> StartTimingReadAsync(ValueTask<ReadResult> readAwaitable, CancellationToken cancellationToken)
200204
{
201205
if (!readAwaitable.IsCompleted)

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,5 @@ internal enum RequestRejectionReason
3131
ConnectMethodRequired,
3232
MissingHostHeader,
3333
MultipleHostHeaders,
34-
InvalidHostHeader,
35-
RequestBodyExceedsContentLength
34+
InvalidHostHeader
3635
}

‎src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,9 @@ internal static bool TryGetErrorType(ConnectionEndReason reason, [NotNullWhen(tr
504504
ConnectionEndReason.AppShutdown => "app_shutdown",
505505
ConnectionEndReason.TlsHandshakeFailed => "tls_handshake_failed",
506506
ConnectionEndReason.InvalidRequestLine => "invalid_request_line",
507+
ConnectionEndReason.TlsOverHttp => "tls_over_http",
508+
ConnectionEndReason.MaxRequestBodySizeExceeded => "max_request_body_size_exceeded",
509+
ConnectionEndReason.UnexpectedEndOfRequestContent => "unexpected_end_of_request_content",
507510
_ => throw new InvalidOperationException($"Unable to calculate whether {reason} resolves to error.type value.")
508511
};
509512

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedRequestTests.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,17 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Buffers;
6-
using System.Collections.Generic;
75
using System.Globalization;
8-
using System.IO;
9-
using System.Linq;
106
using System.Text;
11-
using System.Threading.Tasks;
127
using Microsoft.AspNetCore.Http;
8+
using Microsoft.AspNetCore.InternalTesting;
9+
using Microsoft.AspNetCore.Server.Kestrel.Core;
1310
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
11+
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
1412
using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport;
15-
using Microsoft.AspNetCore.InternalTesting;
13+
using Microsoft.Extensions.Diagnostics.Metrics.Testing;
1614
using Microsoft.Extensions.Logging;
17-
using Xunit;
1815
using BadHttpRequestException = Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException;
1916

2017
namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests;
@@ -852,7 +849,10 @@ await connection.ReceiveEnd(
852849
[Fact]
853850
public async Task ClosingConnectionMidChunkPrefixThrows()
854851
{
855-
var testContext = new TestServiceContext(LoggerFactory);
852+
var testMeterFactory = new TestMeterFactory();
853+
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
854+
855+
var testContext = new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory));
856856
var readStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
857857
#pragma warning disable CS0618 // Type or member is obsolete
858858
var exTcs = new TaskCompletionSource<BadHttpRequestException>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -898,6 +898,11 @@ await connection.SendAll(
898898
Assert.Equal(RequestRejectionReason.UnexpectedEndOfRequestContent, badReqEx.Reason);
899899
}
900900
}
901+
902+
Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m =>
903+
{
904+
Assert.Equal(KestrelMetrics.GetErrorType(ConnectionEndReason.UnexpectedEndOfRequestContent), m.Tags[KestrelMetrics.ErrorType]);
905+
});
901906
}
902907

903908
[Fact]

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/MaxRequestBodySizeTests.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
using Microsoft.AspNetCore.InternalTesting;
1313
using Xunit;
1414
using BadHttpRequestException = Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException;
15+
using Microsoft.Extensions.Diagnostics.Metrics.Testing;
16+
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
1517

1618
namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests;
1719

@@ -107,6 +109,9 @@ await connection.ReceiveEnd(
107109
[Fact]
108110
public async Task RejectsRequestWithChunckedBodySizeExceedingPerRequestLimitAndExceptionWasCaughtByApplication()
109111
{
112+
var testMeterFactory = new TestMeterFactory();
113+
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
114+
110115
var maxRequestBodySize = 3;
111116
var customApplicationResponse = "custom";
112117
var chunkedPayload = $"5;random chunk extension\r\nHello\r\n6\r\n World\r\n0\r\n";
@@ -127,7 +132,7 @@ public async Task RejectsRequestWithChunckedBodySizeExceedingPerRequestLimitAndE
127132
await context.Response.WriteAsync(customApplicationResponse);
128133
throw requestRejectedEx;
129134
},
130-
new TestServiceContext(LoggerFactory) { ServerOptions = { Limits = { MaxRequestBodySize = maxRequestBodySize } } }))
135+
new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory)) { ServerOptions = { Limits = { MaxRequestBodySize = maxRequestBodySize } } }))
131136
{
132137
using var connection = server.CreateConnection();
133138
await connection.Send(
@@ -146,6 +151,11 @@ await connection.ReceiveEnd(
146151
customApplicationResponse,
147152
"");
148153
}
154+
155+
Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m =>
156+
{
157+
Assert.Equal(KestrelMetrics.GetErrorType(ConnectionEndReason.MaxRequestBodySizeExceeded), m.Tags[KestrelMetrics.ErrorType]);
158+
});
149159
}
150160

151161
[Fact]
@@ -355,6 +365,9 @@ await connection.Receive("HTTP/1.1 101 Switching Protocols",
355365
[Fact]
356366
public async Task EveryReadFailsWhenContentLengthHeaderExceedsGlobalLimit()
357367
{
368+
var testMeterFactory = new TestMeterFactory();
369+
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
370+
358371
#pragma warning disable CS0618 // Type or member is obsolete
359372
BadHttpRequestException requestRejectedEx1 = null;
360373
BadHttpRequestException requestRejectedEx2 = null;
@@ -371,7 +384,7 @@ public async Task EveryReadFailsWhenContentLengthHeaderExceedsGlobalLimit()
371384
#pragma warning restore CS0618 // Type or member is obsolete
372385
throw requestRejectedEx2;
373386
},
374-
new TestServiceContext(LoggerFactory) { ServerOptions = { Limits = { MaxRequestBodySize = 0 } } }))
387+
new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory)) { ServerOptions = { Limits = { MaxRequestBodySize = 0 } } }))
375388
{
376389
using (var connection = server.CreateConnection())
377390
{
@@ -395,6 +408,11 @@ await connection.ReceiveEnd(
395408
Assert.NotNull(requestRejectedEx2);
396409
Assert.Equal(CoreStrings.FormatBadRequest_RequestBodyTooLarge(0), requestRejectedEx1.Message);
397410
Assert.Equal(CoreStrings.FormatBadRequest_RequestBodyTooLarge(0), requestRejectedEx2.Message);
411+
412+
Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m =>
413+
{
414+
Assert.Equal(KestrelMetrics.GetErrorType(ConnectionEndReason.MaxRequestBodySizeExceeded), m.Tags[KestrelMetrics.ErrorType]);
415+
});
398416
}
399417

400418
[Fact]

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using Microsoft.AspNetCore.InternalTesting;
1616
using Microsoft.Extensions.Logging;
1717
using Moq;
18+
using Microsoft.Extensions.Diagnostics.Metrics.Testing;
1819

1920
namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests;
2021

@@ -1152,7 +1153,10 @@ await connection.Receive(
11521153
[Fact]
11531154
public async Task ContentLengthReadAsyncSingleBytesAtATime()
11541155
{
1155-
var testContext = new TestServiceContext(LoggerFactory);
1156+
var testMeterFactory = new TestMeterFactory();
1157+
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
1158+
1159+
var testContext = new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory));
11561160
var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
11571161
var tcs2 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
11581162

@@ -1221,6 +1225,11 @@ await connection.ReceiveEnd(
12211225
"");
12221226
}
12231227
}
1228+
1229+
Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m =>
1230+
{
1231+
Assert.Equal(KestrelMetrics.GetErrorType(ConnectionEndReason.UnexpectedEndOfRequestContent), m.Tags[KestrelMetrics.ErrorType]);
1232+
});
12241233
}
12251234

12261235
[Fact]
@@ -2246,6 +2255,39 @@ await connection.ReceiveEnd(
22462255
}
22472256
}
22482257

2258+
[Fact]
2259+
public async Task TlsOverHttp()
2260+
{
2261+
var testMeterFactory = new TestMeterFactory();
2262+
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
2263+
2264+
var testContext = new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory));
2265+
2266+
await using (var server = new TestServer(context =>
2267+
{
2268+
return Task.CompletedTask;
2269+
}, testContext))
2270+
{
2271+
using (var connection = server.CreateConnection())
2272+
{
2273+
await connection.Stream.WriteAsync(new byte[] { 0x16, 0x03, 0x01, 0x02, 0x00, 0x01, 0x00, 0xfc, 0x03, 0x03, 0x03, 0xca, 0xe0, 0xfd, 0x0a }).DefaultTimeout();
2274+
2275+
await connection.ReceiveEnd(
2276+
"HTTP/1.1 400 Bad Request",
2277+
"Content-Length: 0",
2278+
"Connection: close",
2279+
$"Date: {testContext.DateHeaderValue}",
2280+
"",
2281+
"");
2282+
}
2283+
}
2284+
2285+
Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m =>
2286+
{
2287+
Assert.Equal(KestrelMetrics.GetErrorType(ConnectionEndReason.TlsOverHttp), m.Tags[KestrelMetrics.ErrorType]);
2288+
});
2289+
}
2290+
22492291
[Fact]
22502292
public async Task CustomRequestHeaderEncodingSelectorCanBeConfigured()
22512293
{

‎src/Shared/ServerInfrastructure/Http2/ConnectionEndReason.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,8 @@ internal enum ConnectionEndReason
4646
AppShutdown,
4747
GracefulAppShutdown,
4848
TransportCompleted,
49-
TlsHandshakeFailed
49+
TlsHandshakeFailed,
50+
TlsOverHttp,
51+
MaxRequestBodySizeExceeded,
52+
UnexpectedEndOfRequestContent
5053
}

0 commit comments

Comments
 (0)