Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BinaryFormatter deprecation for System.Configuration.ConfigurationManager #50531

Merged
merged 3 commits into from
May 13, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ public enum SettingsSerializeAs
{
String = 0,
Xml = 1,
[System.ObsoleteAttribute(System.Obsoletions.BinaryFormatterMessage + @". Consider using Xml instead.", false)]
Binary = 2,
ProviderSpecific = 3,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
</PropertyGroup>
<ItemGroup>
<Compile Include="System.Configuration.ConfigurationManager.cs" />
<Compile Include="$(CommonPath)System\Obsoletions.cs" Link="Common\System\Obsoletions.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\System.Security.Permissions\ref\System.Security.Permissions.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@
<Compile Include="System\UriIdnScope.cs" />
<Compile Include="$(CommonPath)System\IO\TempFileCollection.cs"
Link="Common\System\IO\TempFileCollection.cs" />
<Compile Include="$(CommonPath)System\Obsoletions.cs" Link="Common\System\Obsoletions.cs" />
pgovind marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
<ItemGroup Condition="'$(IsPartialFacadeAssembly)' != 'true'">
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Cryptography.ProtectedData\src\System.Security.Cryptography.ProtectedData.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,9 @@ private XmlNode SerializeToXmlElement(SettingsProperty setting, SettingsProperty

string serializedValue = value.SerializedValue as string;

#pragma warning disable CS0618 // Type or member is obsolete
if (serializedValue == null && setting.SerializeAs == SettingsSerializeAs.Binary)
#pragma warning restore CS0618 // Type or member is obsolete
{
// SettingsPropertyValue returns a byte[] in the binary serialization case. We need to
// encode this - we use base64 since SettingsPropertyValue understands it and we won't have
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ namespace System.Configuration
{
public class SettingsProperty
{
internal static bool EnableUnsafeBinaryFormatterInPropertyValueSerialization { get; } = AppContext.TryGetSwitch("System.Configuration.ConfigurationManager.EnableUnsafeBinaryFormatterInPropertyValueSerialization", out bool isEnabled) ? isEnabled : false;

public virtual string Name { get; set; }
public virtual bool IsReadOnly { get; set; }
public virtual object DefaultValue { get; set; }
Expand Down Expand Up @@ -37,7 +39,19 @@ public SettingsProperty(
Provider = provider;
IsReadOnly = isReadOnly;
DefaultValue = defaultValue;
SerializeAs = serializeAs;
#pragma warning disable CS0618 // Type or member is obsolete
if (serializeAs == SettingsSerializeAs.Binary)
#pragma warning restore CS0618 // Type or member is obsolete
{
if (EnableUnsafeBinaryFormatterInPropertyValueSerialization)
{
SerializeAs = serializeAs;
}
else
{
throw new NotSupportedException(Obsoletions.BinaryFormatterMessage);
}
}
Attributes = attributes;
ThrowOnErrorDeserializing = throwOnErrorDeserializing;
ThrowOnErrorSerializing = throwOnErrorSerializing;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ private object Deserialize()
// Attempt 1: Try creating from SerializedValue
if (SerializedValue != null)
{
bool throwBinaryFormatterDeprecationException = false;
try
{
if (SerializedValue is string)
Expand All @@ -96,10 +97,17 @@ private object Deserialize()
}
else
{
using (MemoryStream ms = new MemoryStream((byte[])SerializedValue))
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
if (SettingsProperty.EnableUnsafeBinaryFormatterInPropertyValueSerialization)
{
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
pgovind marked this conversation as resolved.
Show resolved Hide resolved
value = (new BinaryFormatter()).Deserialize(ms);
using (MemoryStream ms = new MemoryStream((byte[])SerializedValue))
{
value = (new BinaryFormatter()).Deserialize(ms);
}
}
else
{
throwBinaryFormatterDeprecationException = true;
}
}
}
Expand All @@ -124,6 +132,11 @@ private object Deserialize()
}
}

if (throwBinaryFormatterDeprecationException)
{
throw new NotSupportedException(Obsoletions.BinaryFormatterMessage);
}

if (value != null && !Property.PropertyType.IsAssignableFrom(value.GetType())) // is it the correct type
value = null;
}
Expand Down Expand Up @@ -192,12 +205,21 @@ private static object GetObjectFromString(Type type, SettingsSerializeAs seriali
// Convert based on the serialized type
switch (serializeAs)
{
#pragma warning disable CS0618 // Type or member is obsolete
case SettingsSerializeAs.Binary:
byte[] buffer = Convert.FromBase64String(serializedValue);
using (MemoryStream ms = new MemoryStream(buffer))
#pragma warning restore CS0618 // Type or member is obsolete
if (SettingsProperty.EnableUnsafeBinaryFormatterInPropertyValueSerialization)
{
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
pgovind marked this conversation as resolved.
Show resolved Hide resolved
return (new BinaryFormatter()).Deserialize(ms);
byte[] buffer = Convert.FromBase64String(serializedValue);
using (MemoryStream ms = new MemoryStream(buffer))
{
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
return (new BinaryFormatter()).Deserialize(ms);
}
}
else
{
throw new NotSupportedException(Obsoletions.BinaryFormatterMessage);
}
case SettingsSerializeAs.Xml:
StringReader sr = new StringReader(serializedValue);
Expand All @@ -218,15 +240,26 @@ private object SerializePropertyValue()
if (_value == null)
return null;

#pragma warning disable CS0618 // Type or member is obsolete
if (Property.SerializeAs != SettingsSerializeAs.Binary)
#pragma warning restore CS0618 // Type or member is obsolete
{
return ConvertObjectToString(_value, Property.PropertyType, Property.SerializeAs, Property.ThrowOnErrorSerializing);
}

using (MemoryStream ms = new MemoryStream())
if (SettingsProperty.EnableUnsafeBinaryFormatterInPropertyValueSerialization)
{
using (MemoryStream ms = new MemoryStream())
{
BinaryFormatter bf = new BinaryFormatter();
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
pgovind marked this conversation as resolved.
Show resolved Hide resolved
bf.Serialize(ms, _value);
return ms.ToArray();
}
}
else
{
// Issue https://github.com/dotnet/runtime/issues/39295 tracks finding an alternative to BinaryFormatter
BinaryFormatter bf = new BinaryFormatter();
bf.Serialize(ms, _value);
return ms.ToArray();
throw new NotSupportedException(Obsoletions.BinaryFormatterMessage);
}
}

Expand Down Expand Up @@ -255,7 +288,9 @@ private static string ConvertObjectToString(object propertyValue, Type type, Set

xs.Serialize(sw, propertyValue);
return sw.ToString();
#pragma warning disable CS0618 // Type or member is obsolete
case SettingsSerializeAs.Binary:
#pragma warning restore CS0618 // Type or member is obsolete
Debug.Fail("Should not have gotten here with Binary formatting");
break;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace System.Configuration
{
public enum SettingsSerializeAs
{
String = 0,
Xml = 1,
[Obsolete(Obsoletions.BinaryFormatterMessage + @". Consider using Xml instead.", false)]
pgovind marked this conversation as resolved.
Show resolved Hide resolved
Binary = 2,
ProviderSpecific = 3
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
<Compile Include="System\Configuration\ApplicationSettingsBaseTests.cs" />
<Compile Include="System\Configuration\AppSettingsReaderTests.cs" />
<Compile Include="System\Configuration\AppSettingsTests.cs" />
<Compile Include="System\Configuration\BinaryFormatterDeprecationTests.cs" />
<Compile Include="System\Configuration\CallBackValidatorAttributeTests.cs" />
<Compile Include="System\Configuration\ConfigPathUtilityTests.cs" />
<Compile Include="System\Configuration\ConfigurationElementCollectionTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ public void Reload_SimpleSettings_Ok()
[ReadOnly(false)]
[SettingsGroupName("TestGroup")]
[SettingsProvider(typeof(TestProvider))]
#pragma warning disable CS0618 // Type or member is obsolete
[SettingsSerializeAs(SettingsSerializeAs.Binary)]
#pragma warning restore CS0618 // Type or member is obsolete
private class SettingsWithAttributes : ApplicationSettingsBase
{
[ApplicationScopedSetting]
Expand Down Expand Up @@ -243,7 +245,9 @@ public void SettingsProperty_SettingsWithAttributes_Ok()
Assert.Equal(1, settings.Properties.Count);
SettingsProperty property = settings.Properties["StringProperty"];
Assert.Equal(typeof(TestProvider), property.Provider.GetType());
#pragma warning disable CS0618 // Type or member is obsolete
Assert.Equal(SettingsSerializeAs.Binary, property.SerializeAs);
#pragma warning restore CS0618 // Type or member is obsolete
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also care about adding a test case using reflection as well?

cc: @GrabYourPitchforks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in general we don't care about binary formatter usage if it is called explicitly via reflection by a user.

// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Configuration;
using Microsoft.DotNet.RemoteExecutor;
using Xunit;

namespace System.ConfigurationTests
{
public class BinaryFormatterDeprecationTests
{
private static bool AreBinaryFormatterAndRemoteExecutorSupportedOnThisPlatform => PlatformDetection.IsBinaryFormatterSupported && RemoteExecutor.IsSupported;

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "SettingsSerializeAs.Binary is deprecated only on Core")]
[Fact]
public void ThrowOnSettingsPropertyConstructorWithSettingsSerializeAsBinary()
{
#pragma warning disable CS0618 // Type or member is obsolete
Assert.Throws<NotSupportedException>(() => new SettingsProperty("Binary", typeof(byte[]), null, false,"AString", SettingsSerializeAs.Binary, new SettingsAttributeDictionary(), true, true));
#pragma warning restore CS0618 // Type or member is obsolete
}

[ConditionalFact(nameof(AreBinaryFormatterAndRemoteExecutorSupportedOnThisPlatform))]
public void SerializeAndDeserializeWithSettingsSerializeAsBinary()
{
RemoteInvokeOptions options = new RemoteInvokeOptions();
options.RuntimeConfigurationOptions.Add("System.Configuration.ConfigurationManager.EnableUnsafeBinaryFormatterInPropertyValueSerialization", bool.TrueString);
RemoteExecutor.Invoke(() =>
{
#pragma warning disable CS0618 // Type or member is obsolete
SettingsProperty property = new SettingsProperty("Binary", typeof(string), null, false, "AString", SettingsSerializeAs.Binary, new SettingsAttributeDictionary(), true, true);
#pragma warning restore CS0618 // Type or member is obsolete
SettingsPropertyValue value = new SettingsPropertyValue(property);
value.PropertyValue = "AString"; // To force _changedSinceLastSerialized to true to allow for serialization in the next call
object serializedValue = value.SerializedValue;
Assert.NotNull(serializedValue);
value.Deserialized = false;
object deserializedValue = value.PropertyValue;
Assert.Equal("AString", deserializedValue);
}, options).Dispose();
}
}
}