fullserializer icon indicating copy to clipboard operation
fullserializer copied to clipboard

fsIEnumerableConverter fails on objects with custom indexers

Open lazlo-bonin opened this issue 8 years ago • 1 comments

fsIEnumerableConverter tries to deserialize into existing elements using the get_Item and set_Item methods, assuming without any type check that the first available property indexer has a single int parameter, while that may not be the case.

For example, any class inheriting from KeyedCollection<TKey, TValue> will fail, because get_Item and set_Item use a TKey indexer, not an int indexer. Furthermore, a KeyedCollection<int, TValue> may not cause errors, but will display unintended behaviour, because the indexer does not refer to an index but to a key.

Fortunately, the fix is simple: don't try to deserialize into the existing collection; just clear the list and add items anew. Here's my version of TryDeserialize with the offending bits commented out:

public override fsResult TryDeserialize(fsData data, ref object instance_, Type storageType) {
    var instance = (IEnumerable)instance_;
    var result = fsResult.Success;

    if ((result += CheckType(data, fsDataType.Array)).Failed) return result;

    // For general strategy, instance may already have items in it. We
    // will try to deserialize into the existing element.
    // Fix: No, don't. This strategy will fail for custom indexers that are 
    // not index-based and is overall a lot less predictable than just clearing
    // and adding new items.
    var elementStorageType = GetElementType(storageType);
    var addMethod = GetAddMethod(storageType);
    //var getMethod = storageType.GetFlattenedMethod("get_Item");
    //var setMethod = storageType.GetFlattenedMethod("set_Item");
    /*if (setMethod == null)*/ TryClear(storageType, instance);
    //var existingSize = TryGetExistingSize(storageType, instance);

    var serializedList = data.AsList;
    for (int i = 0; i < serializedList.Count; ++i) {
        var itemData = serializedList[i];
        object itemInstance = null;
        //if (getMethod != null && i < existingSize) {
        //    itemInstance = getMethod.Invoke(instance, new object[] { i });
        //}

        // note: We don't fail the entire deserialization even if the
        //       item failed
        var itemResult = Serializer.TryDeserialize(itemData, elementStorageType, ref itemInstance);
        result.AddMessages(itemResult);
        if (itemResult.Failed) continue;

        //if (setMethod != null && i < existingSize) {
        //    setMethod.Invoke(instance, new object[] { i, itemInstance });
        //}
        //else {
            addMethod.Invoke(instance, new object[] { itemInstance });
        //}
    }

    return result;
}

lazlo-bonin avatar Feb 22 '17 01:02 lazlo-bonin

I was having some exotic issues with corruptions and this fix seems to have hit the spot!

The crazy thing is that it would work, until it no longer did. I mean, I have certain objects for months, then one day they decided to no longer de-serialize.

Thanks!

SugoiDev avatar Jun 15 '17 14:06 SugoiDev