Skip to content

Properly reject non-json FromBody parameter binding #35976

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.

Sign up for GitHub

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 4 commits into from
Sep 2, 2021
Merged

Properly reject non-json FromBody parameter binding #35976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64b3f6d
Properly reject non-json FromBody parameter binding
BrennanConroy Aug 31, 2021
a330d0b
Update src/Http/Http.Extensions/src/RequestDelegateFactory.cs
BrennanConroy Aug 31, 2021
6112761
Update src/Http/Http.Extensions/src/RequestDelegateFactory.cs
BrennanConroy Aug 31, 2021
5f18b0e
test
BrennanConroy Sep 2, 2021
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
22 changes: 21 additions & 1 deletion src/Http/Http.Extensions/src/RequestDelegateFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,12 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
if (feature?.CanHaveBody == true)
{
if (!httpContext.Request.HasJsonContentType())
{
Log.UnexpectedContentType(httpContext, httpContext.Request.ContentType);
httpContext.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
return;
}
try
{
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
Expand Down Expand Up @@ -590,6 +596,12 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
if (feature?.CanHaveBody == true)
{
if (!httpContext.Request.HasJsonContentType())
{
Log.UnexpectedContentType(httpContext, httpContext.Request.ContentType);
httpContext.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
return;
}
try
{
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
Expand All @@ -603,7 +615,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
{

Log.RequestBodyInvalidDataException(httpContext, ex, factoryContext.ThrowOnBadRequest);
httpContext.Response.StatusCode = 400;
httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
return;
}
}
Expand Down Expand Up @@ -1204,6 +1216,14 @@ 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)
=> UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");

[LoggerMessage(6, LogLevel.Debug,
"Expected a supported JSON media type but got \"{ContentType}\".",
EventName = "UnexpectedContentType")]
private static partial void UnexpectedContentType(ILogger logger, string contentType);

private static ILogger GetLogger(HttpContext httpContext)
{
var loggerFactory = httpContext.RequestServices.GetRequiredService<ILoggerFactory>();
Expand Down
69 changes: 69 additions & 0 deletions src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2360,7 +2360,65 @@ public async Task CanExecuteRequestDelegateWithResultsExtension()
Assert.False(httpContext.RequestAborted.IsCancellationRequested);
var decodedResponseBody = Encoding.UTF8.GetString(responseBodyStream.ToArray());
Assert.Equal(@"""Hello Tester. This is from an extension method.""", decodedResponseBody);
}

[Fact]
public async Task RequestDelegateRejectsNonJsonContent()
{
var httpContext = new DefaultHttpContext();
httpContext.Request.Headers["Content-Type"] = "application/xml";
httpContext.Request.Headers["Content-Length"] = "1";
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));

var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(LoggerFactory);
httpContext.RequestServices = serviceCollection.BuildServiceProvider();

var factoryResult = RequestDelegateFactory.Create((HttpContext context, Todo todo) =>
{
});
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);

Assert.Equal(415, httpContext.Response.StatusCode);
var logMessage = Assert.Single(TestSink.Writes);
Assert.Equal(new EventId(6, "UnexpectedContentType"), logMessage.EventId);
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
}

[Fact]
public async Task RequestDelegateWithBindAndImplicitBodyRejectsNonJsonContent()
{
Todo originalTodo = new()
{
Name = "Write more tests!"
};

var httpContext = new DefaultHttpContext();

var requestBodyBytes = JsonSerializer.SerializeToUtf8Bytes(originalTodo);
var stream = new MemoryStream(requestBodyBytes);
httpContext.Request.Body = stream;
httpContext.Request.Headers["Content-Type"] = "application/xml";
httpContext.Request.Headers["Content-Length"] = stream.Length.ToString(CultureInfo.InvariantCulture);
httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));

var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(LoggerFactory);
httpContext.RequestServices = serviceCollection.BuildServiceProvider();

var factoryResult = RequestDelegateFactory.Create((HttpContext context, JsonTodo customTodo, Todo todo) =>
{
});
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);

Assert.Equal(415, httpContext.Response.StatusCode);
var logMessage = Assert.Single(TestSink.Writes);
Assert.Equal(new EventId(6, "UnexpectedContentType"), logMessage.EventId);
Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
}

private DefaultHttpContext CreateHttpContext()
Expand Down Expand Up @@ -2399,6 +2457,17 @@ private class CustomTodo : Todo
}
}

private class JsonTodo : Todo
{
public static async ValueTask<JsonTodo?> BindAsync(HttpContext context, ParameterInfo parameter)
{
// manually call deserialize so we don't check content type
var body = await JsonSerializer.DeserializeAsync<JsonTodo>(context.Request.Body);
context.Request.Body.Position = 0;
return body;
}
}

private record struct TodoStruct(int Id, string? Name, bool IsComplete) : ITodo;

private interface ITodo
Expand Down