Skip to content

Commit 3177ab3

Browse files
halter73halter73
committed
Handle JsonExceptions like InvalidDataExceptions in RequestDelegateFactory
1 parent 068797e commit 3177ab3

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
}
@@ -879,11 +885,14 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa
879885

880886
private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, FactoryContext factoryContext)
881887
{
882-
if (factoryContext.JsonRequestBodyType is not null)
888+
if (factoryContext.JsonRequestBodyParameter is not null)
883889
{
884890
factoryContext.HasMultipleBodyParameters = true;
885891
var parameterName = parameter.Name;
886-
if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName))
892+
893+
Debug.Assert(parameterName is not null, "CreateArgument() should throw if parameter.Name is null.");
894+
895+
if (factoryContext.TrackedParameters.ContainsKey(parameterName))
887896
{
888897
factoryContext.TrackedParameters.Remove(parameterName);
889898
factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN");
@@ -892,7 +901,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al
892901

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

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

@@ -1164,7 +1173,7 @@ private class FactoryContext
11641173
public bool DisableInferredFromBody { get; init; }
11651174

11661175
// Temporary State
1167-
public Type? JsonRequestBodyType { get; set; }
1176+
public ParameterInfo? JsonRequestBodyParameter { get; set; }
11681177
public bool AllowEmptyRequestBody { get; set; }
11691178

11701179
public bool UsingTempSourceString { get; set; }
@@ -1197,7 +1206,8 @@ private static class RequestDelegateFactoryConstants
11971206

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

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

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

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

1221-
public static void RequestBodyInvalidDataException(HttpContext httpContext, InvalidDataException exception, bool shouldThrow)
1232+
public static void InvalidJsonRequestBody(HttpContext httpContext, string parameterTypeName, string parameterName, Exception exception, bool shouldThrow)
12221233
{
12231234
if (shouldThrow)
12241235
{
1225-
throw new BadHttpRequestException(RequestBodyInvalidDataExceptionMessage, exception);
1236+
var message = string.Format(CultureInfo.InvariantCulture, InvalidJsonRequestBodyExceptionMessage, parameterTypeName, parameterName);
1237+
throw new BadHttpRequestException(message, exception);
12261238
}
12271239

1228-
RequestBodyInvalidDataException(GetLogger(httpContext), exception);
1240+
InvalidJsonRequestBody(GetLogger(httpContext), parameterTypeName, parameterName, exception);
12291241
}
12301242

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

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

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

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

10861086
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
1087-
var stream = new MemoryStream(requestBodyBytes); ;
1087+
var stream = new MemoryStream(requestBodyBytes);
10881088
httpContext.Request.Body = stream;
10891089

10901090
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1140,7 +1140,7 @@ public async Task BindAsyncRunsBeforeBodyBinding()
11401140
var httpContext = CreateHttpContext();
11411141

11421142
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
1143-
var stream = new MemoryStream(requestBodyBytes); ;
1143+
var stream = new MemoryStream(requestBodyBytes);
11441144
httpContext.Request.Body = stream;
11451145

11461146
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1302,7 +1302,7 @@ public async Task RequestDelegatePopulatesFromBodyParameter(Delegate action)
13021302
var httpContext = CreateHttpContext();
13031303

13041304
var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
1305-
var stream = new MemoryStream(requestBodyBytes); ;
1305+
var stream = new MemoryStream(requestBodyBytes);
13061306
httpContext.Request.Body = stream;
13071307

13081308
httpContext.Request.Headers["Content-Type"] = "application/json";
@@ -1482,12 +1482,45 @@ void TestAction([FromBody] Todo todo)
14821482
Assert.False(httpContext.Response.HasStarted);
14831483

14841484
var logMessage = Assert.Single(TestSink.Writes);
1485-
Assert.Equal(new EventId(2, "RequestBodyInvalidDataException"), logMessage.EventId);
1485+
Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
14861486
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
1487-
Assert.Equal("Reading the request body failed with an InvalidDataException.", logMessage.Message);
1487+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
14881488
Assert.Same(invalidDataException, logMessage.Exception);
14891489
}
14901490

1491+
[Fact]
1492+
public async Task RequestDelegateLogsFromBodyJsonExceptionAsDebugAndSets400Response()
1493+
{
1494+
var invoked = false;
1495+
1496+
void TestAction([FromBody] Todo todo)
1497+
{
1498+
invoked = true;
1499+
}
1500+
1501+
var httpContext = CreateHttpContext();
1502+
httpContext.Request.Headers["Content-Type"] = "application/json";
1503+
httpContext.Request.Headers["Content-Length"] = "1";
1504+
httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
1505+
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
1506+
1507+
var factoryResult = RequestDelegateFactory.Create(TestAction);
1508+
var requestDelegate = factoryResult.RequestDelegate;
1509+
1510+
await requestDelegate(httpContext);
1511+
1512+
Assert.False(invoked);
1513+
Assert.False(httpContext.RequestAborted.IsCancellationRequested);
1514+
Assert.Equal(400, httpContext.Response.StatusCode);
1515+
Assert.False(httpContext.Response.HasStarted);
1516+
1517+
var logMessage = Assert.Single(TestSink.Writes);
1518+
Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
1519+
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
1520+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
1521+
Assert.IsType<JsonException>(logMessage.Exception);
1522+
}
1523+
14911524
[Fact]
14921525
public async Task RequestDelegateLogsFromBodyInvalidDataExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
14931526
{
@@ -1521,11 +1554,49 @@ void TestAction([FromBody] Todo todo)
15211554
// We don't log bad requests when we throw.
15221555
Assert.Empty(TestSink.Writes);
15231556

1524-
Assert.Equal("Reading the request body failed with an InvalidDataException.", badHttpRequestException.Message);
1557+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
15251558
Assert.Equal(400, badHttpRequestException.StatusCode);
15261559
Assert.Same(invalidDataException, badHttpRequestException.InnerException);
15271560
}
15281561

1562+
[Fact]
1563+
public async Task RequestDelegateLogsFromBodyJsonExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
1564+
{
1565+
var invoked = false;
1566+
1567+
void TestAction([FromBody] Todo todo)
1568+
{
1569+
invoked = true;
1570+
}
1571+
1572+
var invalidDataException = new InvalidDataException();
1573+
1574+
var httpContext = CreateHttpContext();
1575+
httpContext.Request.Headers["Content-Type"] = "application/json";
1576+
httpContext.Request.Headers["Content-Length"] = "1";
1577+
httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
1578+
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
1579+
1580+
var factoryResult = RequestDelegateFactory.Create(TestAction, new() { ThrowOnBadRequest = true });
1581+
var requestDelegate = factoryResult.RequestDelegate;
1582+
1583+
var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => requestDelegate(httpContext));
1584+
1585+
Assert.False(invoked);
1586+
1587+
// The httpContext should be untouched.
1588+
Assert.False(httpContext.RequestAborted.IsCancellationRequested);
1589+
Assert.Equal(200, httpContext.Response.StatusCode);
1590+
Assert.False(httpContext.Response.HasStarted);
1591+
1592+
// We don't log bad requests when we throw.
1593+
Assert.Empty(TestSink.Writes);
1594+
1595+
Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
1596+
Assert.Equal(400, badHttpRequestException.StatusCode);
1597+
Assert.IsType<JsonException>(badHttpRequestException.InnerException);
1598+
}
1599+
15291600
[Fact]
15301601
public void BuildRequestDelegateThrowsInvalidOperationExceptionGivenFromBodyOnMultipleParameters()
15311602
{

0 commit comments

Comments
 (0)