Fix infinite recursion in configuration manager (#41638)
authorSantiago Fernandez Madero <safern@microsoft.com>
Tue, 1 Sep 2020 02:45:04 +0000 (19:45 -0700)
committerGitHub <noreply@github.com>
Tue, 1 Sep 2020 02:45:04 +0000 (19:45 -0700)
* Revert "Remove unused local in System.Configuration.ConfigurationManager (#40583)"

This reverts commit a4eba0a0e1ecb31b7cb3482509bd461c8e9f5ccf.

* Rename unused variable and add regression test

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ApplicationSettingsBase.cs
src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ApplicationSettingsBaseTests.cs

index b23e49c..2c5cd0e 100644 (file)
@@ -725,6 +725,9 @@ namespace System.Configuration
                     }
                 }
 
+                // we query the value first so that we initialize all values from value providers and so that we don't end up
+                // on an infinite recursion when calling Properties[propertyName] as that calls this.
+                _ = base[propertyName];
                 SettingsProperty setting = Properties[propertyName];
                 SettingsProvider provider = setting != null ? setting.Provider : null;
 
index 9b5db48..5ad3251 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Specialized;
 using System.ComponentModel;
 using System.Configuration;
 using Xunit;
@@ -41,6 +42,60 @@ namespace System.ConfigurationTests
             }
         }
 
+        public class SettingsWithProvider : ApplicationSettingsBase
+        {
+            [Setting]
+            [SettingsProvider(typeof(CustomProvider))]
+            public string StringPropertyWithProvider
+            {
+                get
+                {
+                    return (string)this[nameof(StringPropertyWithProvider)];
+                }
+                set
+                {
+                    this[nameof(StringPropertyWithProvider)] = value;
+                }
+            }
+
+            [UserScopedSetting]
+            public string StringProperty
+            {
+                get
+                {
+                    return (string)this[nameof(StringProperty)];
+                }
+                set
+                {
+                    this[nameof(StringProperty)] = value;
+                }
+            }
+
+            public class CustomProvider : SettingsProvider
+            {
+                public const string DefaultStringPropertyValue = "stringPropertySet";
+                public override string ApplicationName { get; set; }
+
+                public override SettingsPropertyValueCollection GetPropertyValues(SettingsContext context, SettingsPropertyCollection collection)
+                {
+                    SettingsPropertyValueCollection result = new SettingsPropertyValueCollection();
+                    SettingsProperty property = new SettingsProperty("StringPropertyWithProvider", typeof(string), this, false, DefaultStringPropertyValue, SettingsSerializeAs.String, new SettingsAttributeDictionary(), false, false);
+                    result.Add(new SettingsPropertyValue(new SettingsProperty(property)));
+                    return result;
+                }
+
+                public override void SetPropertyValues(SettingsContext context, SettingsPropertyValueCollection collection)
+                {
+                }
+
+                public override void Initialize(string name, NameValueCollection config)
+                {
+                    base.Initialize(name ?? "CustomProvider", config ?? new NameValueCollection());
+                }
+            }
+
+        }
+
         private class PersistedSimpleSettings : SimpleSettings
         {
         }
@@ -237,5 +292,24 @@ namespace System.ConfigurationTests
             Assert.True(changingFired);
             Assert.Equal(oldValue, settings.IntProperty);
         }
+
+        [Fact]
+        public void OnSettingsLoaded_QueryProperty()
+        {
+            SettingsWithProvider settings = new SettingsWithProvider();
+            bool loadedFired = false;
+            string newStringPropertyValue = nameof(SettingsWithProvider.StringProperty);
+            settings.SettingsLoaded += (object s, SettingsLoadedEventArgs e)
+                =>
+            {
+                loadedFired = true;
+                Assert.Equal(SettingsWithProvider.CustomProvider.DefaultStringPropertyValue, settings.StringPropertyWithProvider);
+                if (string.IsNullOrEmpty(settings.StringProperty))
+                    settings.StringProperty = newStringPropertyValue;
+            };
+
+            Assert.Equal(newStringPropertyValue, settings.StringProperty);
+            Assert.True(loadedFired);
+        }
     }
 }