Skip to content

[release/6.0-rc2] Handle JsonExceptions in RequestDelegateFactory #36627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 39 additions & 29 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Reflection;
using System.Security.Claims;
using System.Text;
using System.Text.Json;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -504,7 +505,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,

private static Func<object?, HttpContext, Task> HandleRequestBodyAndCompileRequestDelegate(Expression responseWritingMethodCall, FactoryContext factoryContext)
{
if (factoryContext.JsonRequestBodyType is null)
if (factoryContext.JsonRequestBodyParameter is null)
{
if (factoryContext.ParameterBinders.Count > 0)
{
Expand Down Expand Up @@ -533,7 +534,12 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
responseWritingMethodCall, TargetExpr, HttpContextExpr).Compile();
}

var bodyType = factoryContext.JsonRequestBodyType;
var bodyType = factoryContext.JsonRequestBodyParameter.ParameterType;
var parameterTypeName = TypeNameHelper.GetTypeDisplayName(factoryContext.JsonRequestBodyParameter.ParameterType, fullName: false);
var parameterName = factoryContext.JsonRequestBodyParameter.Name;

Debug.Assert(parameterName is not null, "CreateArgument() should throw if parameter.Name is null.");

object? defaultBodyValue = null;

if (factoryContext.AllowEmptyRequestBody && bodyType.IsValueType)
Expand Down Expand Up @@ -580,10 +586,10 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
Log.RequestBodyIOException(httpContext, ex);
return;
}
catch (InvalidDataException ex)
catch (JsonException ex)
{
Log.RequestBodyInvalidDataException(httpContext, ex, factoryContext.ThrowOnBadRequest);
httpContext.Response.StatusCode = 400;
Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
return;
}
}
Expand Down Expand Up @@ -618,10 +624,10 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
Log.RequestBodyIOException(httpContext, ex);
return;
}
catch (InvalidDataException ex)
catch (JsonException ex)
{

Log.RequestBodyInvalidDataException(httpContext, ex, factoryContext.ThrowOnBadRequest);
Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
return;
}
Expand Down Expand Up @@ -878,11 +884,14 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa

private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, FactoryContext factoryContext)
{
if (factoryContext.JsonRequestBodyType is not null)
if (factoryContext.JsonRequestBodyParameter is not null)
{
factoryContext.HasMultipleBodyParameters = true;
var parameterName = parameter.Name;
if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName))

Debug.Assert(parameterName is not null, "CreateArgument() should throw if parameter.Name is null.");

if (factoryContext.TrackedParameters.ContainsKey(parameterName))
{
factoryContext.TrackedParameters.Remove(parameterName);
factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN");
Expand All @@ -891,7 +900,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al

var isOptional = IsOptionalParameter(parameter, factoryContext);

factoryContext.JsonRequestBodyType = parameter.ParameterType;
factoryContext.JsonRequestBodyParameter = parameter;
factoryContext.AllowEmptyRequestBody = allowEmpty || isOptional;
factoryContext.Metadata.Add(new AcceptsMetadata(parameter.ParameterType, factoryContext.AllowEmptyRequestBody, DefaultContentType));

Expand Down Expand Up @@ -1163,7 +1172,7 @@ private class FactoryContext
public bool DisableInferredFromBody { get; init; }

// Temporary State
public Type? JsonRequestBodyType { get; set; }
public ParameterInfo? JsonRequestBodyParameter { get; set; }
public bool AllowEmptyRequestBody { get; set; }

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

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

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

private const string UnexpectedContentTypeLogMessage = @"Expected a supported JSON media type but got ""{ContentType}"".";
private const string UnexpectedContentTypeExceptionMessage = @"Expected a supported JSON media type but got ""{0}"".";

private const string ImplicitBodyNotProvidedLogMessage = @"Implicit body inferred for parameter ""{ParameterName}"" but no body was provided. Did you mean to use a Service instead?";
private const string ImplicitBodyNotProvidedExceptionMessage = @"Implicit body inferred for parameter ""{0}"" but no body was provided. Did you mean to use a Service instead?";

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

public static void RequestBodyInvalidDataException(HttpContext httpContext, InvalidDataException exception, bool shouldThrow)
public static void InvalidJsonRequestBody(HttpContext httpContext, string parameterTypeName, string parameterName, Exception exception, bool shouldThrow)
{
if (shouldThrow)
{
throw new BadHttpRequestException(RequestBodyInvalidDataExceptionMessage, exception);
var message = string.Format(CultureInfo.InvariantCulture, InvalidJsonRequestBodyExceptionMessage, parameterTypeName, parameterName);
throw new BadHttpRequestException(message, exception);
}

RequestBodyInvalidDataException(GetLogger(httpContext), exception);
InvalidJsonRequestBody(GetLogger(httpContext), parameterTypeName, parameterName, exception);
}

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

public static void ParameterBindingFailed(HttpContext httpContext, string parameterTypeName, string parameterName, string sourceValue, bool shouldThrow)
{
Expand Down Expand Up @@ -1258,16 +1270,6 @@ public static void RequiredParameterNotProvided(HttpContext httpContext, string
[LoggerMessage(4, LogLevel.Debug, RequiredParameterNotProvidedLogMessage, EventName = "RequiredParameterNotProvided")]
private static partial void RequiredParameterNotProvided(ILogger logger, string parameterType, string parameterName, string source);

public static void UnexpectedContentType(HttpContext httpContext, string? contentType, bool shouldThrow)
{
if (shouldThrow)
{
var message = string.Format(CultureInfo.InvariantCulture, UnexpectedContentTypeExceptionMessage, contentType);
throw new BadHttpRequestException(message, StatusCodes.Status415UnsupportedMediaType);
}

UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
}
public static void ImplicitBodyNotProvided(HttpContext httpContext, string parameterName, bool shouldThrow)
{
if (shouldThrow)
Expand All @@ -1282,8 +1284,16 @@ public static void ImplicitBodyNotProvided(HttpContext httpContext, string param
[LoggerMessage(5, LogLevel.Debug, ImplicitBodyNotProvidedLogMessage, EventName = "ImplicitBodyNotProvided")]
private static partial void ImplicitBodyNotProvided(ILogger logger, string parameterName);

public static void UnexpectedContentType(HttpContext httpContext, string? contentType)
=> UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
public static void UnexpectedContentType(HttpContext httpContext, string? contentType, bool shouldThrow)
{
if (shouldThrow)
{
var message = string.Format(CultureInfo.InvariantCulture, UnexpectedContentTypeExceptionMessage, contentType);
throw new BadHttpRequestException(message, StatusCodes.Status415UnsupportedMediaType);
}

UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
}

[LoggerMessage(6, LogLevel.Debug, UnexpectedContentTypeLogMessage, EventName = "UnexpectedContentType")]
private static partial void UnexpectedContentType(ILogger logger, string contentType);
Expand Down
Loading