Fixed #799: InjectUnsetProperties now handles exceptions if the gette…

…r throws.
autofac · Sep 22, 2016 · c754726 · c754726
1 parent 41044d7
commit c754726
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 38 deletions.
diff --git a/src/Autofac/Core/Activators/DefaultPropertySelector.cs b/src/Autofac/Core/Activators/DefaultPropertySelector.cs
@@ -10,24 +10,14 @@ namespace Autofac.Core
     /// </summary>
     public class DefaultPropertySelector : IPropertySelector
     {
-        /// <summary>
-        /// Gets an instance of DefaultPropertySelector that will preserve any values already set
-        /// </summary>
-        internal static IPropertySelector PreserveSetValueInstance { get; } = new DefaultPropertySelector(true);
-
-        /// <summary>
-        /// Gets an instance of DefaultPropertySelector that will cause values to be overwritten
-        /// </summary>
-        internal static IPropertySelector OverwriteSetValueInstance { get; } = new DefaultPropertySelector(false);
-
         /// <summary>
         /// Initializes a new instance of the <see cref="DefaultPropertySelector"/> class
         /// that provides default selection criteria.
         /// </summary>
         /// <param name="preserveSetValues">Determines if values should be preserved or not</param>
         public DefaultPropertySelector(bool preserveSetValues)
         {
-            PreserveSetValues = preserveSetValues;
+            this.PreserveSetValues = preserveSetValues;
         }
 
         /// <summary>
@@ -36,6 +26,16 @@ public DefaultPropertySelector(bool preserveSetValues)
         /// </summary>
         public bool PreserveSetValues { get; protected set; }
 
+        /// <summary>
+        /// Gets an instance of DefaultPropertySelector that will cause values to be overwritten
+        /// </summary>
+        internal static IPropertySelector OverwriteSetValueInstance { get; } = new DefaultPropertySelector(false);
+
+        /// <summary>
+        /// Gets an instance of DefaultPropertySelector that will preserve any values already set
+        /// </summary>
+        internal static IPropertySelector PreserveSetValueInstance { get; } = new DefaultPropertySelector(true);
+
         /// <summary>
         /// Provides default filtering to determine if property should be injected by rejecting
         /// non-public settable properties.
@@ -45,12 +45,24 @@ public DefaultPropertySelector(bool preserveSetValues)
         /// <returns>Whether property should be injected</returns>
         public virtual bool InjectProperty(PropertyInfo propertyInfo, object instance)
         {
-            if (propertyInfo.SetMethod?.IsPublic != true)
+            if (!propertyInfo.CanWrite || propertyInfo.SetMethod?.IsPublic != true)
+            {
                 return false;
+            }
 
-            if (PreserveSetValues && propertyInfo.CanRead && propertyInfo.CanWrite &&
-                (propertyInfo.GetValue(instance, null) != null))
-                return false;
+            if (this.PreserveSetValues && propertyInfo.CanRead)
+            {
+                try
+                {
+                    return propertyInfo.GetValue(instance, null) == null;
+                }
+                catch
+                {
+                    // Issue #799: If getting the property value throws an exception
+                    // then assume it's set and skip it.
+                    return false;
+                }
+            }
 
             return true;
         }

diff --git a/test/Autofac.Test/Core/DefaultPropertySelectorTests.cs b/test/Autofac.Test/Core/DefaultPropertySelectorTests.cs
@@ -1,33 +1,12 @@
-using System.Reflection;
+using System;
+using System.Reflection;
 using Autofac.Core;
 using Xunit;
 
 namespace Autofac.Test.Core
 {
     public class DefaultPropertySelectorTests
     {
-        private class Test
-        {
-        }
-
-        private class HasProperties
-        {
-            private Test PrivatePropertyWithSet { get; set; }
-
-            private Test PrivatePropertyWithDefault { get; set; } = new Test();
-
-            public Test PublicPropertyNoDefault { get; set; }
-
-            public Test PublicPropertyWithDefault { get; set; } = new Test();
-
-            public Test PublicPropertyNoGet
-            {
-                set { }
-            }
-
-            public Test PublicPropertyNoSet { get; }
-        }
-
         [InlineData(true, "PrivatePropertyWithSet", false)]
         [InlineData(false, "PrivatePropertyWithSet", false)]
         [InlineData(true, "PublicPropertyNoDefault", true)]
@@ -36,6 +15,8 @@ public Test PublicPropertyNoGet
         [InlineData(false, "PublicPropertyWithDefault", true)]
         [InlineData(true, "PublicPropertyNoGet", true)]
         [InlineData(true, "PublicPropertyNoSet", false)]
+        [InlineData(true, "PublicPropertyThrowsOnGet", false)]
+        [InlineData(false, "PublicPropertyThrowsOnGet", true)]
         [Theory]
         public void DefaultTests(bool preserveSetValue, string propertyName, bool expected)
         {
@@ -46,5 +27,40 @@ public void DefaultTests(bool preserveSetValue, string propertyName, bool expect
 
             Assert.Equal(expected, finder.InjectProperty(property, instance));
         }
+
+        private class HasProperties
+        {
+            public Test PublicPropertyNoDefault { get; set; }
+
+            public Test PublicPropertyNoGet
+            {
+                set { }
+            }
+
+            public Test PublicPropertyNoSet { get; }
+
+            public Test PublicPropertyThrowsOnGet
+            {
+                get
+                {
+                    // Issue #799: InjectUnsetProperties blows up if the getter throws an exception.
+                    throw new InvalidOperationException();
+                }
+
+                set
+                {
+                }
+            }
+
+            public Test PublicPropertyWithDefault { get; set; } = new Test();
+
+            private Test PrivatePropertyWithDefault { get; set; } = new Test();
+
+            private Test PrivatePropertyWithSet { get; set; }
+        }
+
+        private class Test
+        {
+        }
     }
 }