CODEWITHSUNDEEP
c#asp.net-core
Maxime Rossini
Maxime Rossini

Reputation: 3881

TypeFilter attribute throws NullReferenceException when instanciating inner filter with null argument

I created a simple ASP.NET MVC core filter following the documentation, but adding an argument on the filter:

public class MyFilterAttribute : TypeFilterAttribute
{
    public ApiAuthorizeAttribute(string arg1 = null) : base(typeof(MyFilterImpl))
    {
        this.Arguments = new object[] { arg1 };
    }

    private class MyFilterImpl : IAsyncActionFilter
    {
        private readonly string _arg1;

        public MyFilterImpl(string arg1 /*,  DI dependencies */)
        {
            this._arg1 = arg1;
        }

        async Task IAsyncActionFilter.OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next)
        {
            // Do stuff
            await next();
        }
    }
}

This works well when applying the attribute like this:

[MyFilter(arg1: "foo")]
public async Task<IActionResult> Get()
{
}

But it fails when passing null in the attribute declaration:

[MyFilter] // equivalent to [MyFilter(arg1: null)]
public async Task<IActionResult> Get()
{
}

The runtime throws an NullReferenceException on this line. I can work around this by passing "" instead of null in my case, but is this an expected -or unavoidable- behavior?

Upvotes: 1

Views: 800

Answers (2)

Simply Ged
Simply Ged

Reputation: 8642

You have instantiated this.Arguments in your constructor so I would say it is expected behaviour.

You have a couple of options to fix this.

You can either add an if clause to not initialize this.Arguments if the parameter is null

public MyFilterAttribute(string arg1 = null) : base(typeof(MyFilterImpl))
{
    if(!string.IsNullOrEmpty(arg1))
    {
        this.Arguments = new object[] { arg1 };
    }
}

Or add a second parameterless constructor which does not initialize this.Arguments

public MyFilterAttribute(string arg1) : base(typeof(MyFilterImpl))
{
    this.Arguments = new object[] { arg1 };
}

public MyFilterAttribute() : base(typeof(MyFilterImpl))
{
}

Both will result in the MyFilterImpl being instantiated with null or the passed in value.

Personally I would favour the second option as it keeps the code cleaner and upholds the Single Responsibility Principle

Upvotes: 4

Edward
Edward

Reputation: 29966

As you have found the root cause, 

if (_factory == null)
{
    var argumentTypes = Arguments?.Select(a => a.GetType())?.ToArray();
    _factory = ActivatorUtilities.CreateFactory(ImplementationType, argumentTypes ?? Type.EmptyTypes);
}

It will initialize argumentTypes with a.GetType(), if you pass null or with optionl parameter null, this line will throw error expected.

To avoid passing "" in every action, you could try set string arg1 with default value "". 

public MyFilterAttribute(string arg1 = "") : base(typeof(MyFilterImpl))
{
    this.Arguments = new object[] { arg1 };
}

Upvotes: 1

Related Questions