Skip to content

Commit 7de673f

Browse files
halter73halter73
committed
Handle JsonExceptions like InvalidDataExceptions in RequestDelegateFactory
1 parent 3f93126 commit 7de673f

File tree

2 files changed

+106
-23
lines changed

2 files changed

+106
-23
lines changed

‎src/Http/Http.Extensions/src/RequestDelegateFactory.cs

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Reflection;
99
using System.Security.Claims;
1010
using System.Text;
11+
using System.Text.Json;
1112
using Microsoft.AspNetCore.Http.Features;
1213
using Microsoft.AspNetCore.Http.Metadata;
1314
using Microsoft.Extensions.DependencyInjection;
@@ -504,7 +505,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
504505

505506
private static Func<object?, HttpContext, Task> HandleRequestBodyAndCompileRequestDelegate(Expression responseWritingMethodCall, FactoryContext factoryContext)
506507
{
507-
if (factoryContext.JsonRequestBodyType is null)
508+
if (factoryContext.JsonRequestBodyParameter is null)
508509
{
509510
if (factoryContext.ParameterBinders.Count > 0)
510511
{
@@ -533,7 +534,12 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
533534
responseWritingMethodCall, TargetExpr, HttpContextExpr).Compile();
534535
}
535536

536-
var bodyType = factoryContext.JsonRequestBodyType;
537+
var bodyType = factoryContext.JsonRequestBodyParameter.ParameterType;
538+
var parameterTypeName = TypeNameHelper.GetTypeDisplayName(factoryContext.JsonRequestBodyParameter.ParameterType, fullName: false);
539+
var parameterName = factoryContext.JsonRequestBodyParameter.Name;
540+
541+
Debug.Assert(parameterName is not null, "CreateArgument() should throw if parameter.Name is null.");
542+
537543
object? defaultBodyValue = null;
538544

539545
if (factoryContext.AllowEmptyRequestBody && bodyType.IsValueType)
@@ -580,10 +586,10 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
580586
Log.RequestBodyIOException(httpContext, ex);
581587
return;
582588
}
583-
catch (InvalidDataException ex)
589+
catch (Exception ex) when (ex is InvalidDataException || ex is JsonException)
584590
{
585-
Log.RequestBodyInvalidDataException(httpContext, ex, factoryContext.ThrowOnBadRequest);
586-
httpContext.Response.StatusCode = 400;
591+
Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
592+
httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
587593
return;
588594
}
589595
}
@@ -618,10 +624,10 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
618624
Log.RequestBodyIOException(httpContext, ex);
619625
return;
620626
}
621-
catch (InvalidDataException ex)
627+
catch (Exception ex) when (ex is InvalidDataException || ex is JsonException)
622628
{
623629

624-
Log.RequestBodyInvalidDataException(httpContext, ex, factoryContext.ThrowOnBadRequest);
630+
Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
625631
httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
626632
return;
627633
}
@@ -878,11 +884,14 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa
878884

879885
private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, FactoryContext factoryContext)
880886
{
881-
if (factoryContext.JsonRequestBodyType is not null)
887+
if (factoryContext.JsonRequestBodyParameter is not null)
882888
{
883889
factoryContext.HasMultipleBodyParameters = true;
884890
var parameterName = parameter.Name;
885-
if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName))
891+
892+
Debug.Assert(parameterName is not null, "CreateArgument() should throw if parameter.Name is null.");
893+
894+
if (factoryContext.TrackedParameters.ContainsKey(parameterName))
886895
{
887896
factoryContext.TrackedParameters.Remove(parameterName);
888897
factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN");
@@ -891,7 +900,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al
891900

892901
var isOptional = IsOptionalParameter(parameter, factoryContext);
893902

894-
factoryContext.JsonRequestBodyType = parameter.ParameterType;
903+
factoryContext.JsonRequestBodyParameter = parameter;
895904
factoryContext.AllowEmptyRequestBody = allowEmpty || isOptional;
896905
factoryContext.Metadata.Add(new AcceptsMetadata(parameter.ParameterType, factoryContext.AllowEmptyRequestBody, DefaultAcceptsContentType));
897906

@@ -1163,7 +1172,7 @@ private class FactoryContext
11631172
public bool DisableInferredFromBody { get; init; }
11641173

11651174
// Temporary State
1166-
public Type? JsonRequestBodyType { get; set; }
1175+
public ParameterInfo? JsonRequestBodyParameter { get; set; }
11671176
public bool AllowEmptyRequestBody { get; set; }
11681177

11691178
public bool UsingTempSourceString { get; set; }
@@ -1196,7 +1205,8 @@ private static class RequestDelegateFactoryConstants
11961205

11971206
private static partial class Log
11981207
{
1199-
private const string RequestBodyInvalidDataExceptionMessage = "Reading the request body failed with an InvalidDataException.";
1208+
private const string InvalidJsonRequestBodyMessage = @"Failed to read parameter ""{ParameterType} {ParameterName}"" from the request body as JSON.";
1209+
private const string InvalidJsonRequestBodyExceptionMessage = @"Failed to read parameter ""{0} {1}"" from the request body as JSON.";
12001210

12011211
private const string ParameterBindingFailedLogMessage = @"Failed to bind parameter ""{ParameterType} {ParameterName}"" from ""{SourceValue}"".";
12021212
private const string ParameterBindingFailedExceptionMessage = @"Failed to bind parameter ""{0} {1}"" from ""{2}"".";
@@ -1206,6 +1216,7 @@ private static partial class Log
12061216

12071217
private const string UnexpectedContentTypeLogMessage = @"Expected a supported JSON media type but got ""{ContentType}"".";
12081218
private const string UnexpectedContentTypeExceptionMessage = @"Expected a supported JSON media type but got ""{0}"".";
1219+
12091220
private const string ImplicitBodyNotProvidedLogMessage = @"Implicit body inferred for parameter ""{ParameterName}"" but no body was provided. Did you mean to use a Service instead?";
12101221
private const string ImplicitBodyNotProvidedExceptionMessage = @"Implicit body inferred for parameter ""{0}"" but no body was provided. Did you mean to use a Service instead?";
12111222

@@ -1217,18 +1228,19 @@ public static void RequestBodyIOException(HttpContext httpContext, IOException e
12171228
[LoggerMessage(1, LogLevel.Debug, "Reading the request body failed with an IOException.", EventName = "RequestBodyIOException")]
12181229
private static partial void RequestBodyIOException(ILogger logger, IOException exception);
12191230

1220-
public static void RequestBodyInvalidDataException(HttpContext httpContext, InvalidDataException exception, bool shouldThrow)
1231+
public static void InvalidJsonRequestBody(HttpContext httpContext, string parameterTypeName, string parameterName, Exception exception, bool shouldThrow)
12211232
{
12221233
if (shouldThrow)
12231234
{
1224-
throw new BadHttpRequestException(RequestBodyInvalidDataExceptionMessage, exception);
1235+
var message = string.Format(CultureInfo.InvariantCulture, InvalidJsonRequestBodyExceptionMessage, parameterTypeName, parameterName);
1236+
throw new BadHttpRequestException(message, exception);
12251237
}
12261238

1227-
RequestBodyInvalidDataException(GetLogger(httpContext), exception);
1239+
InvalidJsonRequestBody(GetLogger(httpContext), parameterTypeName, parameterName, exception);
12281240
}
12291241

1230-
[LoggerMessage(2, LogLevel.Debug, RequestBodyInvalidDataExceptionMessage, EventName = "RequestBodyInvalidDataException")]
1231-
private static partial void RequestBodyInvalidDataException(ILogger logger, InvalidDataException exception);
1242+
[LoggerMessage(2, LogLevel.Debug, InvalidJsonRequestBodyMessage, EventName = "InvalidJsonRequestBody")]
1243+
private static partial void InvalidJsonRequestBody(ILogger logger, string parameterType, string parameterName, Exception exception);
12321244

12331245
public static void ParameterBindingFailed(HttpContext httpContext, string parameterTypeName, string parameterName, string sourceValue, bool shouldThrow)
12341246
{

‎src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ public async Task BindAsyncWithBodyArgument()
956956
var httpContext = CreateHttpContext();
957957

958958
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
959-
var stream = new MemoryStream(requestBodyBytes); ;
959+
var stream = new MemoryStream(requestBodyBytes);
960960
httpContext.Request.Body = stream;
961961

962962
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1012,7 +1012,7 @@ public async Task BindAsyncRunsBeforeBodyBinding()
10121012
var httpContext = CreateHttpContext();
10131013

10141014
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
1015-
var stream = new MemoryStream(requestBodyBytes); ;
1015+
var stream = new MemoryStream(requestBodyBytes);
10161016
httpContext.Request.Body = stream;
10171017

10181018
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1174,7 +1174,7 @@ public async Task RequestDelegatePopulatesFromBodyParameter(Delegate action)
11741174
var httpContext = CreateHttpContext();
11751175

11761176
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
1177-
var stream = new MemoryStream(requestBodyBytes); ;
1177+
var stream = new MemoryStream(requestBodyBytes);
11781178
httpContext.Request.Body = stream;
11791179

11801180
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1354,12 +1354,45 @@ void TestAction([FromBody] Todo todo)
13541354
Assert.False(httpContext.Response.HasStarted);
13551355

13561356
var logMessage = Assert.Single(TestSink.Writes);
1357-
Assert.Equal(new EventId(2, "RequestBodyInvalidDataException"), logMessage.EventId);
1357+
Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
13581358
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
1359-
Assert.Equal("Reading the request body failed with an InvalidDataException.", logMessage.Message);
1359+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
13601360
Assert.Same(invalidDataException, logMessage.Exception);
13611361
}
13621362

1363+
[Fact]
1364+
public async Task RequestDelegateLogsFromBodyJsonExceptionAsDebugAndSets400Response()
1365+
{
1366+
var invoked = false;
1367+
1368+
void TestAction([FromBody] Todo todo)
1369+
{
1370+
invoked = true;
1371+
}
1372+
1373+
var httpContext = CreateHttpContext();
1374+
httpContext.Request.Headers["Content-Type"] = "application/json";
1375+
httpContext.Request.Headers["Content-Length"] = "1";
1376+
httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
1377+
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
1378+
1379+
var factoryResult = RequestDelegateFactory.Create(TestAction);
1380+
var requestDelegate = factoryResult.RequestDelegate;
1381+
1382+
await requestDelegate(httpContext);
1383+
1384+
Assert.False(invoked);
1385+
Assert.False(httpContext.RequestAborted.IsCancellationRequested);
1386+
Assert.Equal(400, httpContext.Response.StatusCode);
1387+
Assert.False(httpContext.Response.HasStarted);
1388+
1389+
var logMessage = Assert.Single(TestSink.Writes);
1390+
Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
1391+
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
1392+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
1393+
Assert.IsType<JsonException>(logMessage.Exception);
1394+
}
1395+
13631396
[Fact]
13641397
public async Task RequestDelegateLogsFromBodyInvalidDataExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
13651398
{
@@ -1393,11 +1426,49 @@ void TestAction([FromBody] Todo todo)
13931426
// We don't log bad requests when we throw.
13941427
Assert.Empty(TestSink.Writes);
13951428

1396-
Assert.Equal("Reading the request body failed with an InvalidDataException.", badHttpRequestException.Message);
1429+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
13971430
Assert.Equal(400, badHttpRequestException.StatusCode);
13981431
Assert.Same(invalidDataException, badHttpRequestException.InnerException);
13991432
}
14001433

1434+
[Fact]
1435+
public async Task RequestDelegateLogsFromBodyJsonExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
1436+
{
1437+
var invoked = false;
1438+
1439+
void TestAction([FromBody] Todo todo)
1440+
{
1441+
invoked = true;
1442+
}
1443+
1444+
var invalidDataException = new InvalidDataException();
1445+
1446+
var httpContext = CreateHttpContext();
1447+
httpContext.Request.Headers["Content-Type"] = "application/json";
1448+
httpContext.Request.Headers["Content-Length"] = "1";
1449+
httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
1450+
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
1451+
1452+
var factoryResult = RequestDelegateFactory.Create(TestAction, new() { ThrowOnBadRequest = true });
1453+
var requestDelegate = factoryResult.RequestDelegate;
1454+
1455+
var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => requestDelegate(httpContext));
1456+
1457+
Assert.False(invoked);
1458+
1459+
// The httpContext should be untouched.
1460+
Assert.False(httpContext.RequestAborted.IsCancellationRequested);
1461+
Assert.Equal(200, httpContext.Response.StatusCode);
1462+
Assert.False(httpContext.Response.HasStarted);
1463+
1464+
// We don't log bad requests when we throw.
1465+
Assert.Empty(TestSink.Writes);
1466+
1467+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
1468+
Assert.Equal(400, badHttpRequestException.StatusCode);
1469+
Assert.IsType<JsonException>(badHttpRequestException.InnerException);
1470+
}
1471+
14011472
[Fact]
14021473
public void BuildRequestDelegateThrowsInvalidOperationExceptionGivenFromBodyOnMultipleParameters()
14031474
{

0 commit comments

Comments
 (0)