Update AutofacFilterProvider.GetFilters() to skip evaluation if the controller instance is null about autofac.mvc

[tillig]

Can you show us how to reproduce the issue?

It's not a big deal to guard against, but sometimes those checks are just there for testing, not a condition that would normally be hit in runtime. Hence the question - how would you get a null controller in a normal runtime environment?

[demonsweatlive]

Yes, well...I have no idea. That's the problem I was hoping you would have some insight into, such that I wouldn't have to detail our dependency structure and configuration. I was hoping that a null check would be the easy (and correct) solution, just based on what I saw in the base implementation. But here goes...

We use MVCSiteMapProvider for navigation on our site. We recently enabled "Security Trimming", which can show or hide nodes based on AuthorizeAttributes that decorate the routes that the menu nodes point to. This all worked fine when the site was using Windows Authentication.

We recently changed to Forms Auth (we use both in different production environments), and that's when it started breaking. The Stack trace ends with the null reference in AutofacFilterProvider. Now, there could just as likely be a problem in our site map configuration/usage (or even a bug in MVCSiteMapProvider code). I thought as much when I started debugging this, because this is happening upon first visiting the site. The user is redirected to the log-in page, as he is not yet authenticated.

I thought it would have something to do with the user being anonymous. However, when I started debugging the code, I hit a breakpoint in GetFilters() for 3 or 4 controller actions before we hit the action that contained the null Controller property.

Hopefully that's of some help. I'll keep digging in the mean time.

[NullReferenceException: Object reference not set to an instance of an object.]
System.Object.GetType() +0
Autofac.Integration.Mvc.AutofacFilterProvider.GetFilters(ControllerContext controllerContext, ActionDescriptor actionDescriptor) +336
MvcSiteMapProvider.AuthorizeAttributeAclModule.IsAccessibleToUser(IControllerTypeResolver controllerTypeResolver, DefaultSiteMapProvider provider, HttpContext context, SiteMapNode node) +1497
MvcSiteMapProvider.DefaultAclModule.IsAccessibleToUser(IControllerTypeResolver controllerTypeResolver, DefaultSiteMapProvider provider, HttpContext context, SiteMapNode node) +170
MvcSiteMapProvider.DefaultSiteMapProvider.IsAccessibleToUser(HttpContext context, SiteMapNode node) +448
System.Web.StaticSiteMapProvider.GetChildNodes(SiteMapNode node) +334
MvcSiteMapProvider.DefaultSiteMapProvider.FindControllerActionNode(SiteMapNode rootNode, IDictionary`2 values, RouteBase route) +65
MvcSiteMapProvider.DefaultSiteMapProvider.FindSiteMapNode(HttpContext context, RouteData routeData) +705
MvcSiteMapProvider.DefaultSiteMapProvider.FindSiteMapNode(HttpContext context) +122
System.Web.SiteMapProvider.get_CurrentNode() +70
MvcSiteMapProvider.DefaultSiteMapProvider.get_CurrentNode() +141
MvcSiteMapProvider.Web.Html.SiteMapPathHelper.SiteMapPath(MvcSiteMapHtmlHelper helper, String templateName) +48
WOTI.Xift.XiftWebApp.LayoutHelperExtensions.XiftSiteMapPath(HtmlHelper Html) +19

[demonsweatlive]

Okay, it turns out that it was the MvcSiteMapProvider.AuthorizeAttributeAclModule.IsAccessibleToUser() call that was setting the Controller to null. Ultimately, the problem was in our code, as the controller construction was throwing an exception with an anonymous user. The method above swallows the exception and proceeds as though nothing is amiss.

I don't know, frankly, if a null Controller property should be an exceptional condition or not. But you can make that determination on your own time, as this is no longer a pressing need for us.

[tillig]

I'll leave this open and we can add the check. It makes sense to behave like the base provider though I don't think it would have guaranteed correct behavior for you - if the controller is null it skips filter evaluation entirely, which means the authorization wouldn't necessarily have been calculated correctly.

As soon as you mentioned MvcSiteMapProvider I pretty much knew the issue was going to be there. I've spent a lot of time with MvcSiteMapProvider in the past. The idea is good, but given proper authorization filter eval requires a controller instance (as you're seeing in that default filter attribute provider) I found that we were eating up a lot of memory on every request when we had to actually instantiate every controller in the map just to evaluate whether or not it should show up in the map. I'm not sure if that issue's been resolved; for the projects I was on we ended up writing a different, custom version that didn't require that.

Anyway, glad you figured it out.