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

NullReferenceException in XmlObjectSerializerReadContext.ReplaceDeserializedObject #41465

Closed
eerhardt opened this issue Aug 27, 2020 · 0 comments · Fixed by #55316
Closed

NullReferenceException in XmlObjectSerializerReadContext.ReplaceDeserializedObject #41465

eerhardt opened this issue Aug 27, 2020 · 0 comments · Fixed by #55316
Assignees
Milestone

Comments

@eerhardt
Copy link
Member

It is possible for null objects to be passed to XmlObjectSerializerReadContext.ReplaceDeserializedObject:

public void ReplaceDeserializedObject(string id, object oldObj, object newObj)
{
if (object.ReferenceEquals(oldObj, newObj))
return;
if (id != Globals.NewObjectId)
{
// In certain cases (IObjectReference, SerializationSurrogate or DataContractSurrogate),
// an object can be replaced with a different object once it is deserialized. If the
// object happens to be referenced from within itself, that reference needs to be updated
// with the new instance. BinaryFormatter supports this by fixing up such references later.
// These XmlObjectSerializer implementations do not currently support fix-ups. Hence we
// throw in such cases to allow us add fix-up support in the future if we need to.
if (DeserializedObjects.IsObjectReferenced(id))
throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(XmlObjectSerializer.CreateSerializationException(SR.Format(SR.FactoryObjectContainsSelfReference, DataContract.GetClrTypeFullName(oldObj.GetType()), DataContract.GetClrTypeFullName(newObj.GetType()), id)));
DeserializedObjects.Remove(id);
DeserializedObjects.Add(id, newObj);

The throw statement de-references the objects to call GetType() on them in order to log their types in the exception message. However, I can't find anything that ensures both of these parameters are non-null. If they are both null, the method exits early, but if one of them is null, it might be possible for this to throw NullReferenceException.

Of the 3 callers of this method, only 1 of them has the potential to pass in null:

private object InternalDeserializeWithSurrogate(XmlReaderDelegator xmlReader, Type declaredType, DataContract surrogateDataContract, string name, string ns)
{
DataContract dataContract = surrogateDataContract ??
GetDataContract(DataContractSurrogateCaller.GetDataContractType(_serializationSurrogateProvider, declaredType));
if (this.IsGetOnlyCollection && dataContract.UnderlyingType != declaredType)
{
throw System.Runtime.Serialization.DiagnosticUtility.ExceptionUtility.ThrowHelperError(new InvalidDataContractException(SR.Format(SR.SurrogatesWithGetOnlyCollectionsNotSupportedSerDeser, DataContract.GetClrTypeFullName(declaredType))));
}
ReadAttributes(xmlReader);
string objectId = GetObjectId();
object oldObj = InternalDeserialize(xmlReader, name, ns, declaredType, ref dataContract);
object obj = DataContractSurrogateCaller.GetDeserializedObject(_serializationSurrogateProvider, oldObj, dataContract.UnderlyingType, declaredType);
ReplaceDeserializedObject(objectId, oldObj, obj);

I can reproduce the NullReferenceException with the following code and .xml file:

using System;
using System.Runtime.Serialization;
using System.Xml;

namespace ConsoleApp101
{
    class Program
    {
        static void Main()
        {
            DataContractSerializer readerSerializer = new DataContractSerializer(typeof(CustomerContainer));
            DataContractSerializerExtensions.SetSerializationSurrogateProvider(readerSerializer, new MySerializationSurrogateProvider());

            using (var xmlreader = XmlReader.Create("temp.xml"))
            {
                readerSerializer.ReadObject(xmlreader);
            }
        }
    }

    class MySerializationSurrogateProvider : ISerializationSurrogateProvider
    {
        private int _customerCounter = 0;
        public object GetDeserializedObject(object obj, Type targetType)
        {
            if (targetType == typeof(Customer))
            {
                if (_customerCounter > 0)
                    return null;
                _customerCounter++;
            }
            return obj;
        }

        public object GetObjectToSerialize(object obj, Type targetType) => obj;
        public Type GetSurrogateType(Type type) => type;
    }

    [DataContract]
    class CustomerContainer
    {
        [DataMember]
        public Customer Customer1 { get; set; }
        [DataMember]
        public Customer Customer2 { get; set; }
    }

    [DataContract]
    class Customer
    {
        [DataMember]
        public string Name { get; set; }
    }
}
<?xml version="1.0" encoding="utf-8"?>
<CustomerContainer xmlns:i="http://www.w3.org/2001/XMLSchema-instance" z:Id="1" xmlns:z="http://schemas.microsoft.com/2003/10/Serialization/" xmlns="http://schemas.datacontract.org/2004/07/ConsoleApp101">
  <Customer1 z:Id="2">
    <Name z:Id="3">Hi</Name>
  </Customer1>
  <Customer2 z:Id="2" z:Ref="2" i:nil="true" />
</CustomerContainer>

Exception

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Object.GetType()
   at System.Runtime.Serialization.XmlObjectSerializerReadContext.ReplaceDeserializedObject(String id, Object oldObj, Object newObj)
   at System.Runtime.Serialization.XmlObjectSerializerReadContextComplex.InternalDeserializeWithSurrogate(XmlReaderDelegator xmlReader, Type declaredType, DataContract surrogateDataContract, String name, String ns)
   at System.Runtime.Serialization.XmlObjectSerializerReadContextComplex.InternalDeserialize(XmlReaderDelegator xmlReader, Int32 declaredTypeID, RuntimeTypeHandle declaredTypeHandle, String name, String ns)
   at ReadCustomerContainerFromXml(XmlReaderDelegator , XmlObjectSerializerReadContext , XmlDictionaryString[] , XmlDictionaryString[] )
   at System.Runtime.Serialization.ClassDataContract.ReadXmlValue(XmlReaderDelegator xmlReader, XmlObjectSerializerReadContext context)
   at System.Runtime.Serialization.XmlObjectSerializerReadContext.ReadDataContractValue(DataContract dataContract, XmlReaderDelegator reader)
   at System.Runtime.Serialization.XmlObjectSerializerReadContext.InternalDeserialize(XmlReaderDelegator reader, String name, String ns, Type declaredType, DataContract& dataContract)
   at System.Runtime.Serialization.XmlObjectSerializerReadContextComplex.InternalDeserializeWithSurrogate(XmlReaderDelegator xmlReader, Type declaredType, DataContract surrogateDataContract, String name, String ns)
   at System.Runtime.Serialization.XmlObjectSerializerReadContextComplex.InternalDeserialize(XmlReaderDelegator xmlReader, Type declaredType, DataContract dataContract, String name, String ns)
   at System.Runtime.Serialization.DataContractSerializer.InternalReadObject(XmlReaderDelegator xmlReader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.ReadObjectHandleExceptions(XmlReaderDelegator reader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.DataContractSerializer.ReadObject(XmlReader reader)
   at ConsoleApp101.Program.Main() in C:\Users\eerhardt\source\repos\ConsoleApp101\Program.cs:line 16
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 27, 2020
eerhardt added a commit to eerhardt/runtime that referenced this issue Aug 27, 2020
krwq pushed a commit that referenced this issue Aug 28, 2020
…Runtime.Serialization.Json (#41476)

* Initial nullable annotations of System.Private.DataContractSerialization

Contributes to #2339

* Mark DataMember.Name as non-nullable.

* Fix a few simple nullable compile errors.

* Assert attributes is non-null in XmlObjectSerializerReadContext

* Ensure XmlObjectSerializerContext.serializer is never null

* Fix a few simple nullable errors

* Remove any checks that DataMember.MemberInfo can be null.

* Mark EnumDataContract.Members as non-nullable.

Fix nullable errors in SchemaExporter.

* Correctly annotate CollectionDataContract.IsCollectionOrTryCreate.

* Assert DataContractResolver is non-null.

* Suppress #41465

* Update System.Runtime.Serialization.Json ref source for nullable annotations.

* Update System.Runtime.Serializaiton.Xml ref source for nullable annotations.

* Update for Xml.ReaderWriter nullable annotations

* Work around compiler issue.

* Fix test failure.

Reference compiler issue in TODO comment.

* Revert nullable suppression now that XmlSchemaAppInfo.Markup is annotated correctly.

* Fix build for latest annotations in master.

* PR feedback round 1

* Address PR feedback round 2
carlossanlop pushed a commit to carlossanlop/runtime that referenced this issue Aug 28, 2020
…Runtime.Serialization.Json (dotnet#41476)

* Initial nullable annotations of System.Private.DataContractSerialization

Contributes to dotnet#2339

* Mark DataMember.Name as non-nullable.

* Fix a few simple nullable compile errors.

* Assert attributes is non-null in XmlObjectSerializerReadContext

* Ensure XmlObjectSerializerContext.serializer is never null

* Fix a few simple nullable errors

* Remove any checks that DataMember.MemberInfo can be null.

* Mark EnumDataContract.Members as non-nullable.

Fix nullable errors in SchemaExporter.

* Correctly annotate CollectionDataContract.IsCollectionOrTryCreate.

* Assert DataContractResolver is non-null.

* Suppress dotnet#41465

* Update System.Runtime.Serialization.Json ref source for nullable annotations.

* Update System.Runtime.Serializaiton.Xml ref source for nullable annotations.

* Update for Xml.ReaderWriter nullable annotations

* Work around compiler issue.

* Fix test failure.

Reference compiler issue in TODO comment.

* Revert nullable suppression now that XmlSchemaAppInfo.Markup is annotated correctly.

* Fix build for latest annotations in master.

* PR feedback round 1

* Address PR feedback round 2
@stephentoub stephentoub added this to the 6.0.0 milestone Sep 23, 2020
@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Sep 23, 2020
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 8, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants