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

Commit 42cea41

Browse files
dougbudougbu
committed
Fail more gracefully when option collections cleared
- #4690 - move `ModelBindingMessageProvider` init from `DefaultBindingMetadataProvider` to `DefaultModelMetadata` - in addition to avoiding error cases, this removes some boilerplate - add specific errors to `BodyModelBinderProvider`, `CompilerCache`, `CompositeViewEngine`, `ModelBinderFactory`, and `ObjectResultExecutor` - `DefaultRazorViewEngineFileProviderAccessor.FileProvider` now a `NullFileProvider` in empty case
1 parent a852352 commit 42cea41

File tree

29 files changed

+515
-306
lines changed

29 files changed

+515
-306
lines changed

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
1515
/// </summary>
1616
public class DefaultBindingMetadataProvider : IBindingMetadataProvider
1717
{
18-
private readonly ModelBindingMessageProvider _messageProvider;
19-
20-
public DefaultBindingMetadataProvider(ModelBindingMessageProvider messageProvider)
21-
{
22-
if (messageProvider == null)
23-
{
24-
throw new ArgumentNullException(nameof(messageProvider));
25-
}
26-
27-
_messageProvider = messageProvider;
28-
}
29-
30-
/// <inheritdoc />
3118
public void CreateBindingMetadata(BindingMetadataProviderContext context)
3219
{
3320
if (context == null)
@@ -65,10 +52,6 @@ public void CreateBindingMetadata(BindingMetadataProviderContext context)
6552
}
6653
}
6754

68-
// ModelBindingMessageProvider
69-
// Provide a unique instance based on one passed to the constructor.
70-
context.BindingMetadata.ModelBindingMessageProvider = new ModelBindingMessageProvider(_messageProvider);
71-
7255
// PropertyFilterProvider
7356
var propertyFilterProviders = context.Attributes.OfType<IPropertyFilterProvider>().ToArray();
7457
if (propertyFilterProviders.Length == 0)

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44
using System;
55
using System.Threading;
66
using Microsoft.AspNetCore.Http;
7-
using Microsoft.AspNetCore.Mvc.Core;
87
using Microsoft.AspNetCore.Mvc.Formatters;
98
using Microsoft.AspNetCore.Mvc.ModelBinding;
109
using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
1110
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
12-
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
1311
using Microsoft.Extensions.Options;
1412

1513
namespace Microsoft.AspNetCore.Mvc.Internal
@@ -33,16 +31,6 @@ public MvcCoreMvcOptionsSetup(IHttpRequestStreamReaderFactory readerFactory)
3331

3432
public void Configure(MvcOptions options)
3533
{
36-
// Set up default error messages
37-
var messageProvider = options.ModelBindingMessageProvider;
38-
messageProvider.MissingBindRequiredValueAccessor = Resources.FormatModelBinding_MissingBindRequiredMember;
39-
messageProvider.MissingKeyOrValueAccessor = Resources.FormatKeyValuePair_BothKeyAndValueMustBePresent;
40-
messageProvider.ValueMustNotBeNullAccessor = Resources.FormatModelBinding_NullValueNotValid;
41-
messageProvider.AttemptedValueIsInvalidAccessor = Resources.FormatModelState_AttemptedValueIsInvalid;
42-
messageProvider.UnknownValueIsInvalidAccessor = Resources.FormatModelState_UnknownValueIsInvalid;
43-
messageProvider.ValueIsInvalidAccessor = Resources.FormatHtmlGeneration_ValueIsInvalid;
44-
messageProvider.ValueMustBeANumberAccessor = Resources.FormatHtmlGeneration_ValueMustBeNumber;
45-
4634
// Set up ModelBinding
4735
options.ModelBinderProviders.Add(new BinderTypeModelBinderProvider());
4836
options.ModelBinderProviders.Add(new ServicesModelBinderProvider());
@@ -79,7 +67,7 @@ public void Configure(MvcOptions options)
7967
// by altering the collection of providers.
8068
options.ModelMetadataDetailsProviders.Add(new ExcludeBindingMetadataProvider(typeof(Type)));
8169

82-
options.ModelMetadataDetailsProviders.Add(new DefaultBindingMetadataProvider(messageProvider));
70+
options.ModelMetadataDetailsProviders.Add(new DefaultBindingMetadataProvider());
8371
options.ModelMetadataDetailsProviders.Add(new DefaultValidationMetadataProvider());
8472

8573
// Set up validators

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.IO;
78
using System.Text;
89
using System.Threading.Tasks;
@@ -117,6 +118,16 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result)
117118
if (formatters == null || formatters.Count == 0)
118119
{
119120
formatters = OptionsFormatters;
121+
122+
// Complain about MvcOptions.OutputFormatters only if the result has an empty Formatters.
123+
Debug.Assert(formatters != null, "MvcOptions.OutputFormatters cannot be null.");
124+
if (formatters.Count == 0)
125+
{
126+
throw new InvalidOperationException(Resources.FormatOutputFormattersAreRequired(
127+
typeof(MvcOptions).FullName,
128+
nameof(MvcOptions.OutputFormatters),
129+
typeof(IOutputFormatter).FullName));
130+
}
120131
}
121132

122133
var objectType = result.DeclaredType;
@@ -136,7 +147,7 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result)
136147
{
137148
// No formatter supports this.
138149
Logger.NoFormatter(formatterContext);
139-
150+
140151
context.HttpContext.Response.StatusCode = StatusCodes.Status406NotAcceptable;
141152
return TaskCache.CompletedTask;
142153
}
@@ -189,7 +200,7 @@ protected virtual IOutputFormatter SelectFormatter(
189200
if (acceptableMediaTypes.Count == 0)
190201
{
191202
// There is either no Accept header value, or it contained */* and we
192-
// are not currently respecting the 'browser accept header'.
203+
// are not currently respecting the 'browser accept header'.
193204
Logger.NoAcceptForNegotiation();
194205

195206
selectFormatterWithoutRegardingAcceptHeader = true;
@@ -399,7 +410,7 @@ protected virtual IOutputFormatter SelectFormatterUsingAnyAcceptableContentType(
399410

400411
return null;
401412
}
402-
413+
403414
/// <summary>
404415
/// Selects the <see cref="IOutputFormatter"/> to write the response based on the content type values
405416
/// present in <paramref name="sortedAcceptableContentTypes"/> and <paramref name="possibleOutputContentTypes"/>.
@@ -423,27 +434,27 @@ protected virtual IOutputFormatter SelectFormatterUsingSortedAcceptHeadersAndCon
423434
IList<MediaTypeSegmentWithQuality> sortedAcceptableContentTypes,
424435
MediaTypeCollection possibleOutputContentTypes)
425436
{
426-
for (var i = 0; i < sortedAcceptableContentTypes.Count; i++)
437+
for (var i = 0; i < sortedAcceptableContentTypes.Count; i++)
427438
{
428439
var acceptableContentType = new MediaType(sortedAcceptableContentTypes[i].MediaType);
429-
for (var j = 0; j < possibleOutputContentTypes.Count; j++)
440+
for (var j = 0; j < possibleOutputContentTypes.Count; j++)
430441
{
431442
var candidateContentType = new MediaType(possibleOutputContentTypes[j]);
432-
if (candidateContentType.IsSubsetOf(acceptableContentType))
443+
if (candidateContentType.IsSubsetOf(acceptableContentType))
433444
{
434-
for (var k = 0; k < formatters.Count; k++)
445+
for (var k = 0; k < formatters.Count; k++)
435446
{
436447
var formatter = formatters[k];
437448
formatterContext.ContentType = new StringSegment(possibleOutputContentTypes[j]);
438-
if (formatter.CanWriteResult(formatterContext))
449+
if (formatter.CanWriteResult(formatterContext))
439450
{
440451
return formatter;
441452
}
442453
}
443454
}
444455
}
445456
}
446-
457+
447458
return null;
448459
}
449460

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using Microsoft.AspNetCore.Mvc.Core;
67
using Microsoft.AspNetCore.Mvc.Formatters;
78
using Microsoft.AspNetCore.Mvc.Internal;
89

@@ -48,6 +49,14 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
4849
if (context.BindingInfo.BindingSource != null &&
4950
context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Body))
5051
{
52+
if (_formatters.Count == 0)
53+
{
54+
throw new InvalidOperationException(Resources.FormatInputFormattersAreRequired(
55+
typeof(MvcOptions).FullName,
56+
nameof(MvcOptions.InputFormatters),
57+
typeof(IInputFormatter).FullName));
58+
}
59+
5160
return new BodyModelBinder(_formatters, _readerFactory);
5261
}
5362

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/EmptyModelMetadataProvider.cs

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,25 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
5-
using Microsoft.AspNetCore.Mvc.Core;
4+
using System.Collections.Generic;
65
using Microsoft.AspNetCore.Mvc.Internal;
76
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
7+
using Microsoft.Extensions.Options;
88

99
namespace Microsoft.AspNetCore.Mvc.ModelBinding
1010
{
1111
public class EmptyModelMetadataProvider : DefaultModelMetadataProvider
1212
{
1313
public EmptyModelMetadataProvider()
14-
: base(new DefaultCompositeMetadataDetailsProvider(new IMetadataDetailsProvider[]
15-
{
16-
new MessageOnlyBindingProvider()
17-
}))
14+
: base(
15+
new DefaultCompositeMetadataDetailsProvider(new List<IMetadataDetailsProvider>()),
16+
new OptionsAccessor())
1817
{
1918
}
2019

21-
private class MessageOnlyBindingProvider : IBindingMetadataProvider
20+
private class OptionsAccessor : IOptions<MvcOptions>
2221
{
23-
private readonly ModelBindingMessageProvider _messageProvider = CreateMessageProvider();
24-
25-
public void CreateBindingMetadata(BindingMetadataProviderContext context)
26-
{
27-
if (context == null)
28-
{
29-
throw new ArgumentNullException(nameof(context));
30-
}
31-
32-
// Don't bother with ModelBindingMessageProvider copy constructor. No other provider can change the
33-
// delegates.
34-
context.BindingMetadata.ModelBindingMessageProvider = _messageProvider;
35-
}
36-
37-
private static ModelBindingMessageProvider CreateMessageProvider()
38-
{
39-
return new ModelBindingMessageProvider
40-
{
41-
MissingBindRequiredValueAccessor = Resources.FormatModelBinding_MissingBindRequiredMember,
42-
MissingKeyOrValueAccessor = Resources.FormatKeyValuePair_BothKeyAndValueMustBePresent,
43-
ValueMustNotBeNullAccessor = Resources.FormatModelBinding_NullValueNotValid,
44-
AttemptedValueIsInvalidAccessor = Resources.FormatModelState_AttemptedValueIsInvalid,
45-
UnknownValueIsInvalidAccessor = Resources.FormatModelState_UnknownValueIsInvalid,
46-
ValueIsInvalidAccessor = Resources.FormatHtmlGeneration_ValueIsInvalid,
47-
ValueMustBeANumberAccessor = Resources.FormatHtmlGeneration_ValueMustBeNumber,
48-
};
49-
}
22+
public MvcOptions Value { get; } = new MvcOptions();
5023
}
5124
}
5225
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public class DefaultModelMetadata : ModelMetadata
1717
private readonly ICompositeMetadataDetailsProvider _detailsProvider;
1818
private readonly DefaultMetadataDetails _details;
1919

20+
// Default message provider for all DefaultModelMetadata instances; cloned before exposing to
21+
// IBindingMetadataProvider instances to ensure customizations are not accidentally shared.
22+
private readonly ModelBindingMessageProvider _modelBindingMessageProvider;
23+
2024
private ReadOnlyDictionary<object, object> _additionalValues;
2125
private ModelMetadata _elementMetadata;
2226
private bool? _isBindingRequired;
@@ -36,6 +40,22 @@ public DefaultModelMetadata(
3640
IModelMetadataProvider provider,
3741
ICompositeMetadataDetailsProvider detailsProvider,
3842
DefaultMetadataDetails details)
43+
: this(provider, detailsProvider, details, new ModelBindingMessageProvider())
44+
{
45+
}
46+
47+
/// <summary>
48+
/// Creates a new <see cref="DefaultModelMetadata"/>.
49+
/// </summary>
50+
/// <param name="provider">The <see cref="IModelMetadataProvider"/>.</param>
51+
/// <param name="detailsProvider">The <see cref="ICompositeMetadataDetailsProvider"/>.</param>
52+
/// <param name="details">The <see cref="DefaultMetadataDetails"/>.</param>
53+
/// <param name="modelBindingMessageProvider">The <see cref="Metadata.ModelBindingMessageProvider"/>.</param>
54+
public DefaultModelMetadata(
55+
IModelMetadataProvider provider,
56+
ICompositeMetadataDetailsProvider detailsProvider,
57+
DefaultMetadataDetails details,
58+
ModelBindingMessageProvider modelBindingMessageProvider)
3959
: base(details.Key)
4060
{
4161
if (provider == null)
@@ -53,9 +73,15 @@ public DefaultModelMetadata(
5373
throw new ArgumentNullException(nameof(details));
5474
}
5575

76+
if (modelBindingMessageProvider == null)
77+
{
78+
throw new ArgumentNullException(nameof(modelBindingMessageProvider));
79+
}
80+
5681
_provider = provider;
5782
_detailsProvider = detailsProvider;
5883
_details = details;
84+
_modelBindingMessageProvider = modelBindingMessageProvider;
5985
}
6086

6187
/// <summary>
@@ -82,6 +108,11 @@ public BindingMetadata BindingMetadata
82108
if (_details.BindingMetadata == null)
83109
{
84110
var context = new BindingMetadataProviderContext(Identity, _details.ModelAttributes);
111+
112+
// Provide a unique ModelBindingMessageProvider instance so providers' customizations are per-type.
113+
context.BindingMetadata.ModelBindingMessageProvider =
114+
new ModelBindingMessageProvider(_modelBindingMessageProvider);
115+
85116
_detailsProvider.CreateBindingMetadata(context);
86117
_details.BindingMetadata = context.BindingMetadata;
87118
}

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

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.Generic;
77
using System.Reflection;
88
using Microsoft.Extensions.Internal;
9+
using Microsoft.Extensions.Options;
910

1011
namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata
1112
{
@@ -23,11 +24,35 @@ public class DefaultModelMetadataProvider : IModelMetadataProvider
2324
/// </summary>
2425
/// <param name="detailsProvider">The <see cref="ICompositeMetadataDetailsProvider"/>.</param>
2526
public DefaultModelMetadataProvider(ICompositeMetadataDetailsProvider detailsProvider)
27+
: this(detailsProvider, new ModelBindingMessageProvider())
2628
{
29+
}
30+
31+
/// <summary>
32+
/// Creates a new <see cref="DefaultModelMetadataProvider"/>.
33+
/// </summary>
34+
/// <param name="detailsProvider">The <see cref="ICompositeMetadataDetailsProvider"/>.</param>
35+
/// <param name="optionsAccessor">The accessor for <see cref="MvcOptions"/>.</param>
36+
public DefaultModelMetadataProvider(
37+
ICompositeMetadataDetailsProvider detailsProvider,
38+
IOptions<MvcOptions> optionsAccessor)
39+
: this(detailsProvider, GetMessageProvider(optionsAccessor))
40+
{
41+
}
42+
43+
private DefaultModelMetadataProvider(
44+
ICompositeMetadataDetailsProvider detailsProvider,
45+
ModelBindingMessageProvider modelBindingMessageProvider)
46+
{
47+
if (detailsProvider == null)
48+
{
49+
throw new ArgumentNullException(nameof(detailsProvider));
50+
}
51+
2752
DetailsProvider = detailsProvider;
53+
ModelBindingMessageProvider = modelBindingMessageProvider;
2854

2955
_cacheEntryFactory = CreateCacheEntry;
30-
3156
_metadataCacheEntryForObjectType = GetMetadataCacheEntryForObjectType();
3257
}
3358

@@ -36,6 +61,12 @@ public DefaultModelMetadataProvider(ICompositeMetadataDetailsProvider detailsPro
3661
/// </summary>
3762
protected ICompositeMetadataDetailsProvider DetailsProvider { get; }
3863

64+
/// <summary>
65+
/// Gets the <see cref="Metadata.ModelBindingMessageProvider"/>.
66+
/// </summary>
67+
/// <value>Same as <see cref="MvcOptions.ModelBindingMessageProvider"/> in all production scenarios.</value>
68+
protected ModelBindingMessageProvider ModelBindingMessageProvider { get; }
69+
3970
/// <inheritdoc />
4071
public virtual IEnumerable<ModelMetadata> GetMetadataForProperties(Type modelType)
4172
{
@@ -78,6 +109,16 @@ public virtual ModelMetadata GetMetadataForType(Type modelType)
78109
return cacheEntry.Metadata;
79110
}
80111

112+
private static ModelBindingMessageProvider GetMessageProvider(IOptions<MvcOptions> optionsAccessor)
113+
{
114+
if (optionsAccessor == null)
115+
{
116+
throw new ArgumentNullException(nameof(optionsAccessor));
117+
}
118+
119+
return optionsAccessor.Value.ModelBindingMessageProvider;
120+
}
121+
81122
private ModelMetadataCacheEntry GetCacheEntry(Type modelType)
82123
{
83124
ModelMetadataCacheEntry cacheEntry;
@@ -123,7 +164,7 @@ private ModelMetadataCacheEntry GetMetadataCacheEntryForObjectType()
123164
/// </remarks>
124165
protected virtual ModelMetadata CreateModelMetadata(DefaultMetadataDetails entry)
125166
{
126-
return new DefaultModelMetadata(this, DetailsProvider, entry);
167+
return new DefaultModelMetadata(this, DetailsProvider, entry, ModelBindingMessageProvider);
127168
}
128169

129170
/// <summary>

0 commit comments

Comments
 (0)