Slapper.AutoMapper vulnerable to collisions

[kevingosse]

Hello,

I met an annoying bug with Slapper.AutoMapper. Here is a small repro:

class Program
{

    static void Main(string[] args)
    {

        var id2 = typeof(Employee).GetHashCode() - typeof(Contract).GetHashCode();

        var source = new List<object>();

        dynamic obj1 = new ExpandoObject();

        obj1.Id = 1;
        obj1.Contracts_Id = id2;

        source.Add(obj1);

        dynamic obj2 = new ExpandoObject();

        obj2.Id = 1;
        obj2.Contracts_Id = id2 + 1;

        source.Add(obj2);

        var result = Slapper.AutoMapper.MapDynamic<Employee>(source).First();
    }
}

public class Employee
{
    public int Id { get; set; }

    public List<Contract> Contracts { get; set; }

    public override int GetHashCode()
    {
        return this.Id;
    }
}

public class Contract
{
    public int Id { get; set; }

    public override int GetHashCode()
    {
        return this.Id;
    }
}

The problem here is that both the parent object (Employee) and the child object (Contract) will have the same computed key in the instance cache. Therefore, the method Slapper.AutoMapper.InternalHelpers.GetInstance will retrieve the previous Employee instance when checking if there's a previous Contract in the cache, and it'll result in a cast error.

The repro project may look unlikely (with all the GetHashCode overriding) but the collision issue actually happened to me with real code (without any overriding).

Digging into this code actually got me worried. This time the collision resulted in a cast error so I could notice it easily. But what if computing the key resulted in a collision for two objects of the same type? It's very unlikely for an object with a single identifier (still possible though, GetHashCode provides no guarantee to be absolutely unique: http://stackoverflow.com/a/11607221/869621), but the probability increases for objects that use multiple identifiers, since you just sum the hashcodes of each of the identifier (thus increasing the probability of collision).

I'm sorry to say, but the code of Slapper.AutoMapper.InternalHelpers.GetInstance is fundamentally broken because it's based on false assumptions about the behavior of the GetHashCode method. You can use the hashcode to speed up the lookup in the cache, but you still have to check the equality of each of the identifiers.

[odelvalle]

Fixed in pull request #24