Bug 18171

Summary: [System.Xml.Linq, System.Runtime.Serialization] XElement implements IXmlSerializable, but lacks parameterless constructor, and has incorrect QName in KnownTypeCollection
Product: [Mono] Class Libraries Reporter: Brendan Zagaeski (Xamarin Support) <brendan.zagaeski>
Component: System.XMLAssignee: Bugzilla <bugzilla>
Status: RESOLVED FIXED    
Severity: normal CC: atsushi, mono-bugs+mono
Priority: ---    
Version: master   
Target Milestone: Untriaged   
Hardware: PC   
OS: Mac OS   
Tags: Is this bug a regression?: ---
Last known good build:
Attachments: Tests
Proposed fix
Test case

Description Brendan Zagaeski (Xamarin Support) 2014-03-04 22:11:53 UTC
Created attachment 6221 [details]
Tests

The "Deserialize_System_Xml_Linq_XElement_AnyTypeArray" test from the attached "Tests" currently fails on Mono but succeeds on .NET. The "Proposed fix" includes three changes that allow the test to pass. Note that this patch does _not_ yet fix the case of serializing a plain `XElement`. It only fixes `XElement` used as a `KnownType`/`AnyType`.


## Changes in the "Deserialization" patch

1. Change GetSerializableQName() to allow non-empty qualified names for types with `XmlSchemaProviderAttribute.IsAny == true`. The original version of this line was added to help with bug #11916. Since that bug is also about `XElement`, I suspect this proposed change is OK.


Before this change, the test fails with the following stack trace (abbreviated):

> System.Reflection.ReflectionTypeLoadException : The classes in the module cannot be loaded.
> 
>   at (wrapper managed-to-native) System.Reflection.Assembly:GetTypes (System.Reflection.Assembly,bool)
>   at System.Reflection.Assembly.GetTypes () [0x00000] in /private/tmp/source/bockbuild-mono-3.2.6/profiles/mono-mac-xamarin/build-root/mono-3.2.6/mcs/class/corlib/System.Reflection/Assembly.cs:351 
>   at System.Runtime.Serialization.XmlFormatterDeserializer.GetTypeFromNamePair (System.String name, System.String ns)


2. Add a private parameterless constructor to `XElement`.

As discussed on bug #4507, `XElement` lacks a parameterless constructor. This conflicts with the requirement stated on [1] that any class that implements `IXmlSerializable` "must have a parameterless constructor." On .NET, although `XElement` does not have a _public_ parameterless constructor, it does have a _private_ parameterless constructor.

> [1] http://msdn.microsoft.com/en-us/library/system.xml.serialization.ixmlserializable.aspx


Before this change, the test fails with the following stack trace (abbreviated):

> System.MissingMethodException : Default constructor not found for type System.Xml.Linq.XElement
> 
>   at System.Activator.CreateInstance (System.Type type, Boolean nonPublic) [0x00094] in /private/tmp/source/bockbuild-mono-3.2.6/profiles/mono-mac-xamarin/build-root/mono-3.2.6/mcs/class/corlib/System/Activator.cs:326 
>   at System.Runtime.Serialization.XmlSerializableMap.DeserializeObject (System.Xml.XmlReader reader, System.Runtime.Serialization.XmlFormatterDeserializer deserializer)


3. Switch `XElement.ReadXml()` to use `XElement.LoadCore()` rather than `XNode.ReadFrom()`. I think `XElement.ReadXml()` probably only needs to handle cases where the XML input is an `XmlNodeType.Element`, so this seems like an acceptable change. The `used_parameterless_construtor` check on `reader.Read()` might not be strictly necessary. Removing the check does not break any of the attached tests.


Before this change, the test fails with the following stack trace (abbreviated):
> System.NullReferenceException : Object reference not set to an instance of an object
> 
>   at System.Xml.Linq.XElement.WriteTo (System.Xml.XmlWriter writer)


Additionally, before this change the proposed `ParseVsReadXml` test fails with the following stack trace (abbreviated):

> System.InvalidOperationException : Node type None is not supported
> 
>   at System.Xml.Linq.XNode.ReadFrom (System.Xml.XmlReader r, LoadOptions options)
Comment 1 Brendan Zagaeski (Xamarin Support) 2014-03-04 22:24:19 UTC
Created attachment 6222 [details]
Proposed fix

Note that even with this patch, the `Deserialize_System_Xml_Linq_XElement_AnyTypeArray` test will fail with the "Default constructor" error on Android and iOS platforms. The problem is that `System.Runtime.Serialization.XmlSerializableMap.DeserializeObject()` currently uses the version of `Activator.CreateInstance()` that only allows public constructors. This is likely because the `NET_2_1` constant is defined in these builds [1].

> [1] https://github.com/mono/mono/blob/62bf3c73c589ec994db7f4bcb9bae7f9da51b725/mcs/class/System.Runtime.Serialization/System.Runtime.Serialization/SerializationMap.cs#L417-L421
Comment 2 Atsushi Eno 2015-03-17 16:15:16 UTC
I have been reviewing xml related bug and now I'm trying to build the test attached on the first report post, but for System.Runtime.Serialization_test_net_4_5.dll it doesn't.

You need to add your test files to System.Runtime.Serialization_test.dll.sources. I added two files
- Test/System.Runtime.Serialization/DataContractSerializerTest_FrameworkTypes_System.Xml.Linq.cs
- Test/System.Runtime.Serialization/DataContractSerializerTest_FrameworkTypes.cs (which you seem to depend on. It was somehow not part of the test build.)

But I still get build error.

Test/System.Runtime.Serialization/DataContractSerializerTest_FrameworkTypes_System.Xml.Linq.cs(75,22): error CS0103: The name `Deserialize' does not exist in the current context
Test/System.Runtime.Serialization/DataContractSerializerTest_FrameworkTypes_System.Xml.Linq.cs(79,11): error CS1501: No overload for method `IsTrue' takes `4' arguments

Maybe you have some additional changes?
Comment 3 Brendan Zagaeski (Xamarin Support) 2016-05-05 00:06:58 UTC
Created attachment 15911 [details]
Test case

Whoops. Looking back on it, I'm not sure where I came up with that version of the `Deserialize()` method. I have attached a working test case as a plain console C# app.




## Verification status: verified fixed in Cycle 6 (Mono 4.2)

GOOD: Mono 4.4.0 (81f38a9)
GOOD: Mono 4.2.1 (6dd2d0d)
BAD:  Mono 4.0.5 (1d8d582)

I believe this was fixed by the import of the implementations from https://github.com/microsoft/referencesource in Mono 4.2.