carliwar / sharp-architecture

Automatically exported from code.google.com/p/sharp-architecture
Other
0 stars 0 forks source link

SharpModelBinder has a chance to swallow exceptions #128

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I'm not sure how to reproduce the problem, since it's just code that I
found out in SharpArch.Web\ModelBinder\EntityValueProviderResult.cs:

            try {
                object typedId =
                    (idType == typeof(Guid))
                        ? new Guid(rawId)
                        : Convert.ChangeType(rawId, idType);

                return GetEntityFor(typedId, idType);
            }
            // If the Id conversion failed for any reason, just return null
            catch (Exception) {
                return null;
            }

I believe that the "catch-all" block here should catch the Guid conversion
only. Currently it also swallows the GetEntityFor() call - that is,
repository.Get(). Something like:

object typedId = null;
try {
    typedId =
        (idType == typeof(Guid))
            ? new Guid(rawId)
            : Convert.ChangeType(rawId, idType);
}
// If the Id conversion failed for any reason, just return null
catch (Exception) {
    return null;
}
return GetEntityFor(typedId, idType);

Original issue reported on code.google.com by Sergey.P...@gmail.com on 16 Sep 2009 at 2:01

GoogleCodeExporter commented 9 years ago
With the changes to the model binding for MVC2, the likelihood that another 
exception
is swallowed is minimal. 

Looking at the code, I am not sure of your original reporting of this, we would 
want
to trap ANY conversion failure, not just the Guid. If the typedId is incorrect, 
then
trying to bind to the repo would fail.

Original comment by alec.whi...@gmail.com on 25 Mar 2010 at 4:54