rikimaru0345 / Ceras

Universal binary serializer for a wide variety of scenarios https://discord.gg/FGaCX4c
MIT License
484 stars 53 forks source link

Bug: Deserializing object with multiple private fields with the same name fails. #91

Open SJMakin opened 3 years ago

SJMakin commented 3 years ago

Describe the bug Deserialization fails when processing an object with multiple private fields with the same name.

    public class Base
    {
        private bool _dirty;
    }

    public class Child : Base
    {
        private bool _dirty;
    }

    [Test]
    public void InhitanceWorks()
    {
        var config = new SerializerConfig {DefaultTargets = TargetMember.AllFields, VersionTolerance = {Mode = VersionToleranceMode.Standard}};
        var ceras = new CerasSerializer(config);
        var data = ceras.Serialize(new Child());
        var result = ceras.Deserialize<Child>(data);
    }

Additional info

Message: 
    System.ArgumentException : An item with the same key has already been added.
  Stack Trace: 
    ThrowHelper.ThrowArgumentException(ExceptionResource resource)
    Dictionary`2.Insert(TKey key, TValue value, Boolean add)
    Dictionary`2.Add(TKey key, TValue value)
    DynamicFormatter`1.GenerateDeserializer(CerasSerializer ceras, Schema schema, Boolean isSchemaFormatter, Boolean isStatic) line 251
    SchemaDynamicFormatter`1.ActivateSchema(Schema schema) line 150
    ISchemaTaintedFormatter.OnSchemaChanged(TypeMetaData meta) line 83
    CerasSerializer.ActivateSchemaOverride(Type type, Schema schema) line 1029
    SchemaDynamicFormatter`1.Deserialize(Byte[] buffer, Int32& offset, T& value) line 67
    ReferenceFormatter`1.Deserialize(Byte[] buffer, Int32& offset, T& value) line 288
    CerasSerializer.Deserialize[T](T& value, Byte[] buffer, Int32& offset, Int32 expectedReadLength) line 524
    CerasSerializer.Deserialize[T](Byte[] buffer) line 478
    SerializationTests.InhitanceWorks() line 47

Platform

rikimaru0345 commented 3 years ago

IIRC this bug comes from an oversight in the design of Ceras. (read: I just didn't think of that case :P) But it is possible to fix it in multiple ways. You could rename the field by using attributes. But I imagine that would become a hassle pretty fast.

It would probably be better to use config.OnConfigNewType, then go through each member, and if you ever encounter a member named _dirty, you'd rename it to something like "ContainingClass._dirty", for example: Base._dirty or Child._dirty.

SJMakin commented 3 years ago

It's a pretty rare edge case and so far have only seen it a couple of times in projects I control and can change. Happy to close this as a workaround is available should I come across it again in code I can't modify. Appreciate your help. S

On Sun, 11 Apr 2021, 10:24 Moritz Staudinger, @.***> wrote:

IIRC this bug comes from an oversight in the design of Ceras. (read: I just didn't think of that case :P) But it is possible to fix it in multiple ways. You could rename the field by using attributes. But I imagine that would become a hassle pretty fast.

It would probably be better to use config.OnConfigNewType, then go through each member, and if you ever encounter a member named _dirty, you'd rename it to something like "ContainingClass._dirty", for example: Base._dirty or Child._dirty.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rikimaru0345/Ceras/issues/91#issuecomment-817277010, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADGXYEZXUQENSILXR4IT7DDTIFTD3ANCNFSM4WYXUQHA .