From 8d2a1c47e5e620deb6ab3f7ed3c806582bb8639e Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Wed, 22 Oct 2014 22:13:03 -0700 Subject: [PATCH] Change `ViewDataDictionary` copy constructor to ensure `ModelMetadata` is never that for `object` - `ViewDataDictionary.ModelMetadata` was for `object` after base copy constructor got value from `ViewDataDictionary` - problem led to #1426 symptoms - with copy constructor leaving `base.ModelMetadata==null` more often, `ViewDataDictionary.ModelMetadata` usually tracks `TModel` if `Model==null` nit: - fix existing comment in main `ViewDataDictionary` copy constructor --- .../ViewDataDictionary.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs index 6b940b6e94..eca162da51 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs @@ -33,7 +33,8 @@ namespace Microsoft.AspNet.Mvc } /// - /// copy constructor for use when model type does not change. + /// copy constructor for use when model type does not change or caller will + /// immediately set the property. /// public ViewDataDictionary([NotNull] ViewDataDictionary source) : this(source, source.Model) @@ -50,9 +51,15 @@ namespace Microsoft.AspNet.Mvc new CopyOnWriteDictionary(source, StringComparer.OrdinalIgnoreCase), new TemplateInfo(source.TemplateInfo)) { - _modelMetadata = source.ModelMetadata; - // If we're constructing a derived ViewDataDictionary (or any other value type), - // SetModel will throw if we try to set it to null. We should not throw in that case. + // Avoid copying information about the object type. To do so when model==null would confuse the + // ViewDataDictionary.ModelMetadata getter. + if (source.ModelMetadata?.ModelType != typeof(object)) + { + _modelMetadata = source.ModelMetadata; + } + + // If we're constructing a derived ViewDataDictionary where TModel is a non-Nullable value type, + // SetModel will throw if we try to call it with null. We should not throw in that case. if (model != null) { SetModel(model);