Skip to content

Commit

Permalink
Fix infinite recursion in configuration manager (#41638)
Browse files Browse the repository at this point in the history
* Revert "Remove unused local in System.Configuration.ConfigurationManager (#40583)"

This reverts commit a4eba0a.

* Rename unused variable and add regression test

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
  • Loading branch information
safern and eerhardt authored Sep 1, 2020
1 parent 7fb9d9f commit 52bd85b
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,9 @@ private object GetPropertyValue(string propertyName)
}
}

// 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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -41,6 +42,60 @@ public int IntProperty
}
}

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
{
}
Expand Down Expand Up @@ -237,5 +292,24 @@ public void SettingsChanging_Canceled()
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);
}
}
}

0 comments on commit 52bd85b

Please sign in to comment.