dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.85k stars 4.62k forks source link

SettingsProperty constructor ignores serializeAs parameter if it is not SettingsSerializeAs.Binary #104732

Closed JoachimBickel closed 2 weeks ago

JoachimBickel commented 1 month ago

Description

The SettingsProperty constructor ignores the serializeAs parameter if it is not SettingsSerializeAs.Binary.

Reproduction Steps

using System.Configuration;

var settingsProperty = new SettingsProperty("Name", typeof(string), new LocalFileSettingsProvider(), false, "", SettingsSerializeAs.Xml, new SettingsAttributeDictionary(), false, false);

Console.WriteLine(settingsProperty.SerializeAs);

Expected behavior

Outputs "Xml"

Actual behavior

Outputs "String"

Regression?

Yes, worked before v6.0.0-preview.5.21301.5. Caused by https://github.com/dotnet/runtime/pull/50531 Related to https://github.com/dotnet/runtime/issues/39295

Known Workarounds

Set SettingsProperty.SerializeAs property directly after creating the object.

Configuration

.NET 8

Other information

Easy to fix by inserting the following lines in line 55 of https://github.com/dotnet/runtime/blob/main/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/SettingsProperty.cs

else
{
    SerializeAs = serializeAs;
}
dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @dotnet/area-system-configuration See info in area-owners.md if you want to be subscribed.

ericstj commented 1 month ago

Regression from https://github.com/dotnet/runtime/commit/2fb544c1bffdf7170261ba4e5daab3dc057d7453#diff-c87b78f6c97b860d1dd7bf87e2670d814d34abb4876a373a720f38182d5b4591R43 in 6.0.0

Similar to https://github.com/dotnet/runtime/issues/104914. We should probably audit that change to see if we can spot other breaks introduced.

Fix here is simple, as suggested, but we also need to add tests.