Skip to content

Commit 04988ec

Browse files
CopilotStephenMolloyCopilot
authored
Fix ObsoleteAttribute incorrectly causing XML serialization to ignore properties with AppContext switch and SR resources (#119865)
* Initial plan * Remove ObsoleteAttribute check from XML serialization ignore logic Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> * Add tests for ObsoleteAttribute XML serialization fix Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> * Update ObsoleteAttribute logic to throw exception when IsError=true Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> * Use SR for exception messages and add AppContext switch for ObsoleteAttribute behavior Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> * Make the added 'obsolete' tests work. * Keep [Obsolete] out of pregen serializer assembly. It gives warnings - which surface as errors in our build pipelines. * Can't use runtime-only types in non-runtime tests d'oh. * Update src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Need a more complete approach to testing around all these cached values. * Copy pasta dorfed a curly on the wrong side of the hash --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> Co-authored-by: Steve Molloy <smolloy@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent dc06f93 commit 04988ec

File tree

6 files changed

+233
-2
lines changed

6 files changed

+233
-2
lines changed

‎src/libraries/System.Private.Xml/src/Resources/Strings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,6 +2193,9 @@
21932193
<data name="XmlUnsupportedSoapTypeKind" xml:space="preserve">
21942194
<value>The type {0} may not be serialized with SOAP-encoded messages. Set the Use for your message to Literal.</value>
21952195
</data>
2196+
<data name="XmlObsoleteIsError" xml:space="preserve">
2197+
<value>Cannot serialize member with [Obsolete(IsError=true)]: {0}</value>
2198+
</data>
21962199
<data name="XmlUnsupportedIDictionary" xml:space="preserve">
21972200
<value>The type {0} is not supported because it implements IDictionary.</value>
21982201
</data>

‎src/libraries/System.Private.Xml/src/System/Xml/Core/LocalAppContextSwitches.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,15 @@ public static bool IsNetworkingEnabledByDefault
7979
return SwitchesHelpers.GetCachedSwitchValue("System.Xml.XmlResolver.IsNetworkingEnabledByDefault", ref s_isNetworkingEnabledByDefault);
8080
}
8181
}
82+
83+
private static int s_ignoreObsoleteMembers;
84+
public static bool IgnoreObsoleteMembers
85+
{
86+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
87+
get
88+
{
89+
return SwitchesHelpers.GetCachedSwitchValue("Switch.System.Xml.IgnoreObsoleteMembers", ref s_ignoreObsoleteMembers);
90+
}
91+
}
8292
}
8393
}

‎src/libraries/System.Private.Xml/src/System/Xml/Serialization/SoapAttributes.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,28 @@ public SoapAttributes(ICustomAttributeProvider provider)
3232
object[] attrs = provider.GetCustomAttributes(false);
3333
for (int i = 0; i < attrs.Length; i++)
3434
{
35-
if (attrs[i] is SoapIgnoreAttribute || attrs[i] is ObsoleteAttribute)
35+
if (attrs[i] is SoapIgnoreAttribute)
3636
{
3737
_soapIgnore = true;
3838
break;
3939
}
40+
else if (attrs[i] is ObsoleteAttribute obsoleteAttr)
41+
{
42+
if (!System.Xml.LocalAppContextSwitches.IgnoreObsoleteMembers)
43+
{
44+
if (obsoleteAttr.IsError)
45+
{
46+
throw new InvalidOperationException(SR.Format(SR.XmlObsoleteIsError, obsoleteAttr.Message));
47+
}
48+
// If IsError is false, continue processing normally (don't ignore)
49+
}
50+
else
51+
{
52+
// Old behavior: ignore obsolete members when switch is enabled
53+
_soapIgnore = true;
54+
break;
55+
}
56+
}
4057
else if (attrs[i] is SoapElementAttribute)
4158
{
4259
_soapElement = (SoapElementAttribute)attrs[i];

‎src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlAttributes.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,28 @@ public XmlAttributes(ICustomAttributeProvider provider)
8383
XmlAnyElementAttribute? wildcard = null;
8484
for (int i = 0; i < attrs.Length; i++)
8585
{
86-
if (attrs[i] is XmlIgnoreAttribute || attrs[i] is ObsoleteAttribute)
86+
if (attrs[i] is XmlIgnoreAttribute)
8787
{
8888
_xmlIgnore = true;
8989
break;
9090
}
91+
else if (attrs[i] is ObsoleteAttribute obsoleteAttr)
92+
{
93+
if (!System.Xml.LocalAppContextSwitches.IgnoreObsoleteMembers)
94+
{
95+
if (obsoleteAttr.IsError)
96+
{
97+
throw new InvalidOperationException(SR.Format(SR.XmlObsoleteIsError, obsoleteAttr.Message));
98+
}
99+
// If IsError is false, continue processing normally (don't ignore)
100+
}
101+
else
102+
{
103+
// Old behavior: ignore obsolete members when switch is enabled
104+
_xmlIgnore = true;
105+
break;
106+
}
107+
}
91108
else if (attrs[i] is XmlElementAttribute)
92109
{
93110
_xmlElements.Add((XmlElementAttribute)attrs[i]);

‎src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,6 +2625,112 @@ public static void ValidateXElementArray()
26252625
Assert.Equal("Member", retarray.xelements[1].Name);
26262626
}
26272627

2628+
#if !XMLSERIALIZERGENERATORTESTS
2629+
[Fact]
2630+
public static void ObsoleteAttribute_DoesNotAffectSerialization()
2631+
{
2632+
// Test that properties marked with [Obsolete(IsError=false)] are still serialized (not ignored like [XmlIgnore])
2633+
var testObject = new TypeWithObsoleteProperty
2634+
{
2635+
NormalProperty = "normal",
2636+
#pragma warning disable CS0618 // Type or member is obsolete
2637+
ObsoleteProperty = "obsolete",
2638+
#pragma warning restore CS0618 // Type or member is obsolete
2639+
IgnoredProperty = "ignored"
2640+
};
2641+
2642+
var result = SerializeAndDeserialize(testObject, $"""
2643+
<?xml version="1.0" encoding="utf-8"?>
2644+
<TypeWithObsoleteProperty xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
2645+
<NormalProperty>normal</NormalProperty>
2646+
<ObsoleteProperty>obsolete</ObsoleteProperty>
2647+
</TypeWithObsoleteProperty>
2648+
""");
2649+
2650+
// Verify that the obsolete property was correctly roundtripped
2651+
Assert.Equal("normal", result.NormalProperty);
2652+
#pragma warning disable CS0618 // Type or member is obsolete
2653+
Assert.Equal("obsolete", result.ObsoleteProperty);
2654+
#pragma warning restore CS0618 // Type or member is obsolete
2655+
// IgnoredProperty should remain null as it has [XmlIgnore]
2656+
Assert.Null(result.IgnoredProperty);
2657+
}
2658+
2659+
[Fact]
2660+
public static void ObsoleteAttribute_IsError_ThrowsException()
2661+
{
2662+
// Test that properties marked with [Obsolete(IsError=true)] throw an exception during serializer creation
2663+
var testObject = new TypeWithObsoleteErrorProperty
2664+
{
2665+
NormalProperty = "normal",
2666+
#pragma warning disable CS0618, CS0619 // Type or member is obsolete
2667+
ObsoleteProperty = "obsolete",
2668+
// This line would cause a compile-time error due to IsError=true. So we set this in the class definition instead.
2669+
// ObsoletePropertyWithError = "error",
2670+
#pragma warning restore CS0618, CS0619 // Type or member is obsolete
2671+
IgnoredProperty = "ignored"
2672+
};
2673+
2674+
// We need to create a type with just the error property to test the exception
2675+
// Using reflection to create an XmlSerializer for a type with ObsoletePropertyWithError
2676+
Assert.Throws<InvalidOperationException>(() =>
2677+
{
2678+
var serializer = new XmlSerializer(typeof(TypeWithObsoleteErrorProperty));
2679+
var xml = Serialize(testObject, null, () => serializer, true);
2680+
});
2681+
}
2682+
2683+
[Fact]
2684+
public static void ObsoleteAttribute_WithAppContextSwitch_IgnoresObsoleteMembers()
2685+
{
2686+
// Enable compat switch
2687+
using (var compatSwitch = new XmlSerializerAppContextSwitchScope("Switch.System.Xml.IgnoreObsoleteMembers", true))
2688+
{
2689+
Assert.True(compatSwitch.CurrentValue);
2690+
2691+
// Even if we just flipped the appContext switch, previous tests might have already created a serializer
2692+
// for this type. To avoid using a cached serializer, use a different namespace.
2693+
var cacheBustingNamespace = "http://tempuri.org/DoNotUseCachedSerializer";
2694+
var expectedXml = $"""
2695+
<?xml version="1.0" encoding="utf-8"?>
2696+
<TypeWithObsoleteProperty xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns="{cacheBustingNamespace}">
2697+
<NormalProperty>normal</NormalProperty>
2698+
</TypeWithObsoleteProperty>
2699+
""";
2700+
2701+
// Test that when the switch is enabled, obsolete members are ignored (old behavior)
2702+
var testObject = new TypeWithObsoleteProperty
2703+
{
2704+
NormalProperty = "normal",
2705+
#pragma warning disable CS0618 // Type or member is obsolete
2706+
ObsoleteProperty = "obsolete",
2707+
#pragma warning restore CS0618 // Type or member is obsolete
2708+
IgnoredProperty = "ignored"
2709+
};
2710+
2711+
// With the switch enabled, obsolete properties should be ignored
2712+
var serializerFactory = () => new XmlSerializer(typeof(TypeWithObsoleteProperty), cacheBustingNamespace);
2713+
var result = SerializeAndDeserialize(testObject, expectedXml, serializerFactory);
2714+
2715+
// Try with the type that has Obsolete(IsError=true) property. It should still get ignored without throwing.
2716+
var testObjectWithError = new TypeWithObsoleteErrorProperty
2717+
{
2718+
NormalProperty = "normal",
2719+
#pragma warning disable CS0618, CS0619 // Type or member is obsolete
2720+
ObsoleteProperty = "obsolete",
2721+
// This line would cause a compile-time error due to IsError=true. So we set this in the class definition instead.
2722+
// ObsoletePropertyWithError = "error",
2723+
#pragma warning restore CS0618, CS0619 // Type or member is obsolete
2724+
IgnoredProperty = "ignored"
2725+
};
2726+
2727+
serializerFactory = () => new XmlSerializer(typeof(TypeWithObsoleteErrorProperty), cacheBustingNamespace);
2728+
var resultWithError = SerializeAndDeserialize(testObjectWithError,
2729+
expectedXml.Replace("TypeWithObsoleteProperty", "TypeWithObsoleteErrorProperty"), serializerFactory);
2730+
}
2731+
}
2732+
#endif
2733+
26282734
[MethodImpl(MethodImplOptions.NoInlining)]
26292735
private static void ExecuteAndUnload(string assemblyfile, string typename, out WeakReference wref)
26302736
{
@@ -2985,3 +3091,56 @@ private static string GuessCachedName(string name)
29853091
throw new ArgumentException($"Cannot guess cached field name from switch name '{name}'");
29863092
}
29873093
}
3094+
3095+
internal sealed class XmlSerializerAppContextSwitchScope : IDisposable
3096+
{
3097+
private readonly string _name;
3098+
private readonly string _cachedName;
3099+
private readonly bool _hadValue;
3100+
private readonly bool _originalValue;
3101+
3102+
public bool OriginalValue => _originalValue;
3103+
public bool CurrentValue => AppContext.TryGetSwitch(_name, out bool value) && value;
3104+
3105+
public XmlSerializerAppContextSwitchScope(string name, bool value, string cachedName = null)
3106+
{
3107+
_name = name;
3108+
_cachedName = cachedName ?? GuessCachedName(name);
3109+
_hadValue = AppContext.TryGetSwitch(name, out _originalValue);
3110+
AppContext.SetSwitch(name, value);
3111+
ClearCachedSwitch(_cachedName);
3112+
}
3113+
3114+
public void Dispose()
3115+
{
3116+
if (_hadValue)
3117+
AppContext.SetSwitch(_name, _originalValue);
3118+
else
3119+
// There's no "unset", so pick a default or false
3120+
AppContext.SetSwitch(_name, false);
3121+
ClearCachedSwitch(_cachedName);
3122+
}
3123+
3124+
private static void ClearCachedSwitch(string name)
3125+
{
3126+
Type t = Type.GetType("System.Xml.LocalAppContextSwitches, System.Private.Xml");
3127+
Assert.NotNull(t);
3128+
3129+
FieldInfo fi = t.GetField(name, BindingFlags.NonPublic | BindingFlags.Static);
3130+
Assert.NotNull(fi);
3131+
fi.SetValue(null, 0);
3132+
}
3133+
3134+
private static string GuessCachedName(string name)
3135+
{
3136+
// Switch names are typically of the form "Switch.System.Xml.SomeFeature"
3137+
// The cached field is typically "s_someFeature"
3138+
int idx = name.LastIndexOf('.');
3139+
if (idx > 0 && idx < name.Length - 1)
3140+
{
3141+
string feature = name.Substring(idx + 1);
3142+
return "s_" + char.ToLowerInvariant(feature[0]) + feature.Substring(1);
3143+
}
3144+
throw new ArgumentException($"Cannot guess cached field name from switch name '{name}'");
3145+
}
3146+
}

‎src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.RuntimeOnly.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,31 @@ public override int GetHashCode()
12571257
}
12581258
}
12591259

1260+
public class TypeWithObsoleteProperty
1261+
{
1262+
public string NormalProperty { get; set; }
1263+
1264+
[Obsolete("This property is obsolete but should still be serialized")]
1265+
public string ObsoleteProperty { get; set; }
1266+
1267+
[XmlIgnore]
1268+
public string IgnoredProperty { get; set; }
1269+
}
1270+
1271+
public class TypeWithObsoleteErrorProperty
1272+
{
1273+
public string NormalProperty { get; set; }
1274+
1275+
[Obsolete("This property is obsolete but should still be serialized")]
1276+
public string ObsoleteProperty { get; set; }
1277+
1278+
[Obsolete("This property is obsolete with error", true)]
1279+
public string ObsoletePropertyWithError { get; set; } = "error";
1280+
1281+
[XmlIgnore]
1282+
public string IgnoredProperty { get; set; }
1283+
}
1284+
12601285
public class BaseClassForInvalidDerivedClass
12611286
{
12621287
public int Id;

0 commit comments

Comments
 (0)