Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 90acd05

Browse files
SteveSandersonMSSteveSandersonMS
committed
Make [FromBody] treat empty request bodies as invalid (#4750)
1 parent 1e7972b commit 90acd05

File tree

24 files changed

+484
-22
lines changed

24 files changed

+484
-22
lines changed

‎src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterContext.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,36 @@ public InputFormatterContext(
3636
ModelStateDictionary modelState,
3737
ModelMetadata metadata,
3838
Func<Stream, Encoding, TextReader> readerFactory)
39+
: this(httpContext, modelName, modelState, metadata, readerFactory, treatEmptyInputAsDefaultValue: false)
40+
{
41+
}
42+
43+
/// <summary>
44+
/// Creates a new instance of <see cref="InputFormatterContext"/>.
45+
/// </summary>
46+
/// <param name="httpContext">
47+
/// The <see cref="Http.HttpContext"/> for the current operation.
48+
/// </param>
49+
/// <param name="modelName">The name of the model.</param>
50+
/// <param name="modelState">
51+
/// The <see cref="ModelStateDictionary"/> for recording errors.
52+
/// </param>
53+
/// <param name="metadata">
54+
/// The <see cref="ModelMetadata"/> of the model to deserialize.
55+
/// </param>
56+
/// <param name="readerFactory">
57+
/// A delegate which can create a <see cref="TextReader"/> for the request body.
58+
/// </param>
59+
/// <param name="treatEmptyInputAsDefaultValue">
60+
/// A value for the <see cref="TreatEmptyInputAsDefaultValue"/> property.
61+
/// </param>
62+
public InputFormatterContext(
63+
HttpContext httpContext,
64+
string modelName,
65+
ModelStateDictionary modelState,
66+
ModelMetadata metadata,
67+
Func<Stream, Encoding, TextReader> readerFactory,
68+
bool treatEmptyInputAsDefaultValue)
3969
{
4070
if (httpContext == null)
4171
{
@@ -67,9 +97,19 @@ public InputFormatterContext(
6797
ModelState = modelState;
6898
Metadata = metadata;
6999
ReaderFactory = readerFactory;
100+
TreatEmptyInputAsDefaultValue = treatEmptyInputAsDefaultValue;
70101
ModelType = metadata.ModelType;
71102
}
72103

104+
/// <summary>
105+
/// Gets a flag to indicate whether the input formatter should allow no value to be provided.
106+
/// If <see langword="false"/>, the input formatter should handle empty input by returning
107+
/// <see cref="InputFormatterResult.NoValueAsync()"/>. If <see langword="true"/>, the input
108+
/// formatter should handle empty input by returning the default value for the type
109+
/// <see cref="ModelType"/>.
110+
/// </summary>
111+
public bool TreatEmptyInputAsDefaultValue { get; }
112+
73113
/// <summary>
74114
/// Gets the <see cref="Http.HttpContext"/> associated with the current operation.
75115
/// </summary>

‎src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterResult.cs

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,32 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
1010
/// </summary>
1111
public class InputFormatterResult
1212
{
13-
private static readonly InputFormatterResult _failure = new InputFormatterResult();
13+
private static readonly InputFormatterResult _failure = new InputFormatterResult(hasError: true);
14+
private static readonly InputFormatterResult _noValue = new InputFormatterResult(hasError: false);
1415
private static readonly Task<InputFormatterResult> _failureAsync = Task.FromResult(_failure);
16+
private static readonly Task<InputFormatterResult> _noValueAsync = Task.FromResult(_noValue);
1517

16-
private InputFormatterResult()
18+
private InputFormatterResult(bool hasError)
1719
{
18-
HasError = true;
20+
HasError = hasError;
1921
}
2022

2123
private InputFormatterResult(object model)
2224
{
2325
Model = model;
26+
IsModelSet = true;
2427
}
2528

2629
/// <summary>
2730
/// Gets an indication whether the <see cref="IInputFormatter.ReadAsync"/> operation had an error.
2831
/// </summary>
2932
public bool HasError { get; }
3033

34+
/// <summary>
35+
/// Gets an indication whether a value for the <see cref="Model"/> property was supplied.
36+
/// </summary>
37+
public bool IsModelSet { get; }
38+
3139
/// <summary>
3240
/// Gets the deserialized <see cref="object"/>.
3341
/// </summary>
@@ -89,5 +97,31 @@ public static Task<InputFormatterResult> SuccessAsync(object model)
8997
{
9098
return Task.FromResult(Success(model));
9199
}
100+
101+
/// <summary>
102+
/// Returns an <see cref="InputFormatterResult"/> indicating the <see cref="IInputFormatter.ReadAsync"/>
103+
/// operation produced no value.
104+
/// </summary>
105+
/// <returns>
106+
/// An <see cref="InputFormatterResult"/> indicating the <see cref="IInputFormatter.ReadAsync"/>
107+
/// operation produced no value.
108+
/// </returns>
109+
public static InputFormatterResult NoValue()
110+
{
111+
return _noValue;
112+
}
113+
114+
/// <summary>
115+
/// Returns a <see cref="Task"/> that on completion provides an <see cref="InputFormatterResult"/> indicating
116+
/// the <see cref="IInputFormatter.ReadAsync"/> operation produced no value.
117+
/// </summary>
118+
/// <returns>
119+
/// A <see cref="Task"/> that on completion provides an <see cref="InputFormatterResult"/> indicating the
120+
/// <see cref="IInputFormatter.ReadAsync"/> operation produced no value.
121+
/// </returns>
122+
public static Task<InputFormatterResult> NoValueAsync()
123+
{
124+
return _noValueAsync;
125+
}
92126
}
93127
}

‎src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/IModelBindingMessageProvider.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ public interface IModelBindingMessageProvider
2424
/// <value>Default <see cref="string"/> is "A value is required.".</value>
2525
Func<string> MissingKeyOrValueAccessor { get; }
2626

27+
/// <summary>
28+
/// Error message the model binding system adds when no value is provided for the request body,
29+
/// but a value is required.
30+
/// </summary>
31+
/// <value>Default <see cref="string"/> is "A non-empty request body is required.".</value>
32+
Func<string> MissingRequestBodyRequiredValueAccessor { get; }
33+
2734
/// <summary>
2835
/// Error message the model binding system adds when a <c>null</c> value is bound to a
2936
/// non-<see cref="Nullable"/> property.

‎src/Microsoft.AspNetCore.Mvc.Abstractions/exceptions.net45.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[
2+
{
3+
"OldTypeId": "public interface Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.IModelBindingMessageProvider",
4+
"NewTypeId": "public interface Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.IModelBindingMessageProvider",
5+
"NewMemberId": "System.Func<System.String> get_MissingRequestBodyRequiredValueAccessor()",
6+
"Kind": "Addition"
7+
}
8+
]

‎src/Microsoft.AspNetCore.Mvc.Abstractions/exceptions.netcore.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[
2+
{
3+
"OldTypeId": "public interface Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.IModelBindingMessageProvider",
4+
"NewTypeId": "public interface Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.IModelBindingMessageProvider",
5+
"NewMemberId": "System.Func<System.String> get_MissingRequestBodyRequiredValueAccessor()",
6+
"Kind": "Addition"
7+
}
8+
]

‎src/Microsoft.AspNetCore.Mvc.Core/Formatters/InputFormatter.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,12 @@ public virtual Task<InputFormatterResult> ReadAsync(InputFormatterContext contex
105105
var request = context.HttpContext.Request;
106106
if (request.ContentLength == 0)
107107
{
108-
return InputFormatterResult.SuccessAsync(GetDefaultValueForType(context.ModelType));
108+
if (context.TreatEmptyInputAsDefaultValue)
109+
{
110+
return InputFormatterResult.SuccessAsync(GetDefaultValueForType(context.ModelType));
111+
}
112+
113+
return InputFormatterResult.NoValueAsync();
109114
}
110115

111116
return ReadRequestBodyAsync(context);

‎src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public void Configure(MvcOptions options)
4343
// Set up ModelBinding
4444
options.ModelBinderProviders.Add(new BinderTypeModelBinderProvider());
4545
options.ModelBinderProviders.Add(new ServicesModelBinderProvider());
46-
options.ModelBinderProviders.Add(new BodyModelBinderProvider(options.InputFormatters, _readerFactory, _loggerFactory));
46+
options.ModelBinderProviders.Add(new BodyModelBinderProvider(options.InputFormatters, _readerFactory, _loggerFactory, options));
4747
options.ModelBinderProviders.Add(new HeaderModelBinderProvider());
4848
options.ModelBinderProviders.Add(new SimpleTypeModelBinderProvider());
4949
options.ModelBinderProviders.Add(new CancellationTokenModelBinderProvider());

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public class BodyModelBinder : IModelBinder
2222
private readonly IList<IInputFormatter> _formatters;
2323
private readonly Func<Stream, Encoding, TextReader> _readerFactory;
2424
private readonly ILogger _logger;
25+
private readonly MvcOptions _options;
2526

2627
/// <summary>
2728
/// Creates a new <see cref="BodyModelBinder"/>.
@@ -45,7 +46,29 @@ public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamRead
4546
/// instances for reading the request body.
4647
/// </param>
4748
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
48-
public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory loggerFactory)
49+
public BodyModelBinder(
50+
IList<IInputFormatter> formatters,
51+
IHttpRequestStreamReaderFactory readerFactory,
52+
ILoggerFactory loggerFactory)
53+
: this(formatters, readerFactory, loggerFactory, options: null)
54+
{
55+
}
56+
57+
/// <summary>
58+
/// Creates a new <see cref="BodyModelBinder"/>.
59+
/// </summary>
60+
/// <param name="formatters">The list of <see cref="IInputFormatter"/>.</param>
61+
/// <param name="readerFactory">
62+
/// The <see cref="IHttpRequestStreamReaderFactory"/>, used to create <see cref="System.IO.TextReader"/>
63+
/// instances for reading the request body.
64+
/// </param>
65+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
66+
/// <param name="options">The <see cref="MvcOptions"/>.</param>
67+
public BodyModelBinder(
68+
IList<IInputFormatter> formatters,
69+
IHttpRequestStreamReaderFactory readerFactory,
70+
ILoggerFactory loggerFactory,
71+
MvcOptions options)
4972
{
5073
if (formatters == null)
5174
{
@@ -64,6 +87,8 @@ public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamRead
6487
{
6588
_logger = loggerFactory.CreateLogger<BodyModelBinder>();
6689
}
90+
91+
_options = options;
6792
}
6893

6994
/// <inheritdoc />
@@ -89,12 +114,15 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
89114

90115
var httpContext = bindingContext.HttpContext;
91116

117+
var allowEmptyInputInModelBinding = _options?.AllowEmptyInputInBodyModelBinding == true;
118+
92119
var formatterContext = new InputFormatterContext(
93120
httpContext,
94121
modelBindingKey,
95122
bindingContext.ModelState,
96123
bindingContext.ModelMetadata,
97-
_readerFactory);
124+
_readerFactory,
125+
allowEmptyInputInModelBinding);
98126

99127
var formatter = (IInputFormatter)null;
100128
for (var i = 0; i < _formatters.Count; i++)
@@ -132,7 +160,24 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
132160
return;
133161
}
134162

135-
bindingContext.Result = ModelBindingResult.Success(model);
163+
if (result.IsModelSet)
164+
{
165+
bindingContext.Result = ModelBindingResult.Success(model);
166+
}
167+
else
168+
{
169+
// If the input formatter gives a "no value" result, that's always a model state error,
170+
// because BodyModelBinder implicitly regards input as being required for model binding.
171+
// If instead the input formatter wants to treat the input as optional, it must do so by
172+
// returning InputFormatterResult.Success(defaultForModelType), because input formatters
173+
// are responsible for choosing a default value for the model type.
174+
var message = bindingContext
175+
.ModelMetadata
176+
.ModelBindingMessageProvider
177+
.MissingRequestBodyRequiredValueAccessor();
178+
bindingContext.ModelState.AddModelError(modelBindingKey, message);
179+
}
180+
136181
return;
137182
}
138183
catch (Exception ex)

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class BodyModelBinderProvider : IModelBinderProvider
1818
private readonly IList<IInputFormatter> _formatters;
1919
private readonly IHttpRequestStreamReaderFactory _readerFactory;
2020
private readonly ILoggerFactory _loggerFactory;
21+
private readonly MvcOptions _options;
2122

2223
/// <summary>
2324
/// Creates a new <see cref="BodyModelBinderProvider"/>.
@@ -36,6 +37,22 @@ public BodyModelBinderProvider(IList<IInputFormatter> formatters, IHttpRequestSt
3637
/// <param name="readerFactory">The <see cref="IHttpRequestStreamReaderFactory"/>.</param>
3738
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
3839
public BodyModelBinderProvider(IList<IInputFormatter> formatters, IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory loggerFactory)
40+
: this(formatters, readerFactory, loggerFactory, options: null)
41+
{
42+
}
43+
44+
/// <summary>
45+
/// Creates a new <see cref="BodyModelBinderProvider"/>.
46+
/// </summary>
47+
/// <param name="formatters">The list of <see cref="IInputFormatter"/>.</param>
48+
/// <param name="readerFactory">The <see cref="IHttpRequestStreamReaderFactory"/>.</param>
49+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
50+
/// <param name="options">The <see cref="MvcOptions"/>.</param>
51+
public BodyModelBinderProvider(
52+
IList<IInputFormatter> formatters,
53+
IHttpRequestStreamReaderFactory readerFactory,
54+
ILoggerFactory loggerFactory,
55+
MvcOptions options)
3956
{
4057
if (formatters == null)
4158
{
@@ -50,6 +67,7 @@ public BodyModelBinderProvider(IList<IInputFormatter> formatters, IHttpRequestSt
5067
_formatters = formatters;
5168
_readerFactory = readerFactory;
5269
_loggerFactory = loggerFactory;
70+
_options = options;
5371
}
5472

5573
/// <inheritdoc />
@@ -71,7 +89,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
7189
typeof(IInputFormatter).FullName));
7290
}
7391

74-
return new BodyModelBinder(_formatters, _readerFactory, _loggerFactory);
92+
return new BodyModelBinder(_formatters, _readerFactory, _loggerFactory, _options);
7593
}
7694

7795
return null;

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelBindingMessageProvider.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public class ModelBindingMessageProvider : IModelBindingMessageProvider
1313
{
1414
private Func<string, string> _missingBindRequiredValueAccessor;
1515
private Func<string> _missingKeyOrValueAccessor;
16+
private Func<string> _missingRequestBodyRequiredValueAccessor;
1617
private Func<string, string> _valueMustNotBeNullAccessor;
1718
private Func<string, string, string> _attemptedValueIsInvalidAccessor;
1819
private Func<string, string> _unknownValueIsInvalidAccessor;
@@ -26,6 +27,7 @@ public ModelBindingMessageProvider()
2627
{
2728
MissingBindRequiredValueAccessor = Resources.FormatModelBinding_MissingBindRequiredMember;
2829
MissingKeyOrValueAccessor = Resources.FormatKeyValuePair_BothKeyAndValueMustBePresent;
30+
MissingRequestBodyRequiredValueAccessor = Resources.FormatModelBinding_MissingRequestBodyRequiredMember;
2931
ValueMustNotBeNullAccessor = Resources.FormatModelBinding_NullValueNotValid;
3032
AttemptedValueIsInvalidAccessor = Resources.FormatModelState_AttemptedValueIsInvalid;
3133
UnknownValueIsInvalidAccessor = Resources.FormatModelState_UnknownValueIsInvalid;
@@ -47,6 +49,7 @@ public ModelBindingMessageProvider(ModelBindingMessageProvider originalProvider)
4749

4850
MissingBindRequiredValueAccessor = originalProvider.MissingBindRequiredValueAccessor;
4951
MissingKeyOrValueAccessor = originalProvider.MissingKeyOrValueAccessor;
52+
MissingRequestBodyRequiredValueAccessor = originalProvider.MissingRequestBodyRequiredValueAccessor;
5053
ValueMustNotBeNullAccessor = originalProvider.ValueMustNotBeNullAccessor;
5154
AttemptedValueIsInvalidAccessor = originalProvider.AttemptedValueIsInvalidAccessor;
5255
UnknownValueIsInvalidAccessor = originalProvider.UnknownValueIsInvalidAccessor;
@@ -90,6 +93,24 @@ public Func<string> MissingKeyOrValueAccessor
9093
}
9194
}
9295

96+
/// <inheritdoc/>
97+
public Func<string> MissingRequestBodyRequiredValueAccessor
98+
{
99+
get
100+
{
101+
return _missingRequestBodyRequiredValueAccessor;
102+
}
103+
set
104+
{
105+
if (value == null)
106+
{
107+
throw new ArgumentNullException(nameof(value));
108+
}
109+
110+
_missingRequestBodyRequiredValueAccessor = value;
111+
}
112+
}
113+
93114
/// <inheritdoc/>
94115
public Func<string, string> ValueMustNotBeNullAccessor
95116
{

‎src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ public MvcOptions()
3434
ValueProviderFactories = new List<IValueProviderFactory>();
3535
}
3636

37+
/// <summary>
38+
/// Gets or sets the flag which decides whether body model binding (for example, on an
39+
/// action method parameter with <see cref="FromBodyAttribute"/>) should treat empty
40+
/// input as valid. <see langword="false"/> by default.
41+
/// </summary>
42+
/// <example>
43+
/// When <see langword="false"/>, actions that model bind the request body (for example,
44+
/// using <see cref="FromBodyAttribute"/>) will register an error in the
45+
/// <see cref="ModelStateDictionary"/> if the incoming request body is empty.
46+
/// </example>
47+
public bool AllowEmptyInputInBodyModelBinding { get; set; }
48+
3749
/// <summary>
3850
/// Gets a Dictionary of CacheProfile Names, <see cref="CacheProfile"/> which are pre-defined settings for
3951
/// response caching.

0 commit comments

Comments
 (0)