Search code examples
asp.net-web-apidependency-injectioncustom-attributesaction-filter

Why isn't setting the ActionContext.Response to BadRequest in the OnActionExecuting() method returning it straight back to the caller?


I've written an ActionFilter that checks the length of a specified string parameter passed to any given [Web API] action method and if the length isn't correct, is setting the ActionContext.Response to an HttpStatusCode.BadRequest (via a call to actionContext.Request.CreateErrorResponse()), but I'm still ending up in my action method code. This is basically meant to work like all those ActionFilterAttribute classes that people create to handle the validation of the ModelState outside the action method(s), but I needed Dependency Injection as well so that I could use a logger, and have my Attribute/ActionFilter be testable.

My searching turned up this blog post, within which the author describes a way to have a "passive Attribute" (where the Attrib is just basically a DTO) and a 'scanning' ActionFilter that implements the behavior of said Attribute. https://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=98

The problem I'm having is as follows;

(Sorry folks, please bear with me. While I have a lot of years of C# experience, this is my first real forray into Attribute(s) and/or ActionFilter(s))

I've written my Attribute as passive (where the Attribute is just a DTO), and an ActionFilter that inherits from an IActionFilter< CheckStringParamLengthAttribute > as the example(s) in the above blog post show.

Here is my Attribute code.

[AttributeUsage(AttributeTargets.Method, Inherited = true)]
public class CheckStringParamLengthAttribute : Attribute
{
    private int _minLength;
    private int _maxLength;
    private string _errorMessage;
    private string _paramName;

    public CheckStringParamLengthAttribute(
        int MinimumLength,
        string parameterName,
        string ErrorMessage = "",
        int MaximumLength = 0)
    {
        if (MinimumLength < 0 || MinimumLength > int.MaxValue)
        {
            throw new ArgumentException("MinimumLength parameter value out of range.");
        }
        _minLength = MinimumLength;

        if (string.IsNullOrEmpty(parameterName))
        {
            throw new ArgumentException("parameterName is null or empty.");
        }
        _paramName = parameterName;

        // these two have defaults, so no Guard check needed.
        _maxLength = MaximumLength;
        _errorMessage = ErrorMessage;
    }

    public int MinLength { get { return _minLength; } }
    public int MaxLength { get { return _maxLength; } }
    public string ErrorMessage { get { return _errorMessage; } }
    public string ParameterName { get { return _paramName; } }
}

..and the IActionFilter declaration.

public interface IActionFilter<TAttribute> where TAttribute : Attribute
{
    void OnActionExecuting(TAttribute attr, HttpActionContext ctx);
}

All seemed well until I realized that while my ActionFilter sets the ActionContext.Response to an 'error response' ...

actionContext.Response = actionContext.Request.CreateErrorResponse(
    HttpStatusCode.BadRequest, "my error msg");

It's not then returning said BadRequest back to the caller and instead I end up in my action method's code as if the filter wasn't even executed.

Here is the crux of my ActionFilter / 'behavior' code.

public class CheckStringParamLengthActionFilter : IActionFilter<CheckStringParamLengthAttribute>
{
    ...
    public void OnActionExecuting(
        CheckStringParamLengthAttribute attribute, 
        HttpActionContext actionContext)
    {
        Debug.WriteLine("OnActionExecuting (Parameter Name being checked: " + attribute.ParameterName + ")");

        // get the attribute from the method specified in the ActionContext, if there.
        var attr = this.GetStringLengthAttribute(
            actionContext.ActionDescriptor);

        if (actionContext.ActionArguments.Count < 1) {
            throw new Exception("Invalid number of ActionArguments detected.");
        }

        var kvp = actionContext.ActionArguments
            .Where(k => k.Key.Equals(attr.ParameterName, StringComparison.InvariantCulture))
            .First();
        var paramName = kvp.Key;
        var stringToCheck = kvp.Value as string;
        string errorMsg;

        if (stringToCheck.Length < attr.MinLength) {
            errorMsg = string.IsNullOrEmpty(attr.ErrorMessage)
                ? string.Format(
                    "The {0} parameter must be at least {1} characters in length.",
                    paramName, attr.MinLength)
                : attr.ErrorMessage;

            // SEE HERE
            actionContext.Response = actionContext.Request.CreateErrorResponse(
                HttpStatusCode.BadRequest, errorMsg);
            actionContext.Response.ReasonPhrase += " (" + errorMsg + ")";

            return;
        }
        ...
    }
    ...
}

Here's the Application_Start() method (from Global.asax.cs) showing the Simple Injector registration code, etc.

protected void Application_Start()
{
    // DI container spin up. (Simple Injector)
    var container = new Container();
    container.Options.DefaultScopedLifestyle = new WebApiRequestLifestyle();

    container.Register<ILogger, Logger>(Lifestyle.Scoped);

    container.RegisterWebApiControllers(GlobalConfiguration.Configuration);

    GlobalConfiguration.Configuration.Filters.Add(
        new ActionFilterDispatcher(container.GetAllInstances));

    container.RegisterCollection(typeof(IActionFilter<>), typeof(IActionFilter<>).Assembly);

    container.Verify();

    GlobalConfiguration.Configuration.DependencyResolver =
        new SimpleInjectorWebApiDependencyResolver(container);

    // the rest of this is 'normal' Web API registration stuff.
    AreaRegistration.RegisterAllAreas();
    GlobalConfiguration.Configure(WebApiConfig.Register);
    FilterConfig.RegisterGlobalFilters(GlobalFilters.Filters);
    RouteConfig.RegisterRoutes(RouteTable.Routes);
    BundleConfig.RegisterBundles(BundleTable.Bundles);
}

I thought that if I simply set the actionContext.Response to an 'ErrorResponse' that the 'Bad Request' would be sent back to the caller and the action method on which my attribute had been placed would not even get executed. Frustratingly, that is not the case.

So the question is, what am I missing in order to get that Bad Request sent straight back to the caller without going into the action method? Or for that matter, is this even possible?

Push comes to shove, I can always inject another 'service layer' class instance into the Controller(s) and have code in each action method that needs to call a/the string parameter length validator, but this seemed, at least when I started, to be a better, cleaner way.

UPDATE: Well dang me! I apparently forgot the most important part.

I know this because, well, see the answer below.

Meanwhile, here is the ActionFilterDispatcher which is registered in the Application_Start() method in Global.asax.cs. e.g.

protected void Application_Start()
{
    ...
    GlobalConfiguration.Configuration.Filters.Add(
        new ActionFilterDispatcher(container.GetAllInstances));
    ...
}

Registered ActionFilter(s) are called from the ExecuteActionFilterAsync() method of this class. This, as it happens, is key.

public sealed class ActionFilterDispatcher : IActionFilter
{
    private readonly Func<Type, IEnumerable> container;

    public ActionFilterDispatcher(Func<Type, IEnumerable> container)
    {
        this.container = container;
    }

    public Task<HttpResponseMessage> ExecuteActionFilterAsync(
        HttpActionContext context,
        CancellationToken cancellationToken, 
        Func<Task<HttpResponseMessage>> continuation)
    {
        var descriptor = context.ActionDescriptor;
        var attributes = descriptor.ControllerDescriptor.GetCustomAttributes<Attribute>(true)
            .Concat(descriptor.GetCustomAttributes<Attribute>(true));

        foreach (var attribute in attributes)
        {
            Type filterType = typeof(IActionFilter<>).MakeGenericType(attribute.GetType());
            IEnumerable filters = this.container.Invoke(filterType);

            foreach (dynamic actionFilter in filters)
            {
                actionFilter.OnActionExecuting((dynamic)attribute, context);
            }
        }

        return continuation();
    }

    public bool AllowMultiple { get { return true; } }
}

Solution

  • In order to give credit where credit is due, a great and very helpful developer [from the #asp.net channel on efnet] gave me the answer to this issue.

    Since the ActionFilter is called from the ExecuteActionFilterAsync() method of this class, I needed to add a very simple if statement to check and see if the HttpActionContext.Response object had been populated, and if so, exit straight away, which then sends the created response right back to the caller.

    Here's the fixed method.

    public sealed class ActionFilterDispatcher : IActionFilter
    {
        ...
    
        public Task<HttpResponseMessage> ExecuteActionFilterAsync(
            HttpActionContext context,
            CancellationToken cancellationToken, 
            Func<Task<HttpResponseMessage>> continuation)
        {
            var descriptor = context.ActionDescriptor;
            var attributes = descriptor.ControllerDescriptor.GetCustomAttributes<Attribute>(true)
                .Concat(descriptor.GetCustomAttributes<Attribute>(true));
    
            foreach (var attribute in attributes)
            {
                Type filterType = typeof(IActionFilter<>).MakeGenericType(attribute.GetType());
                IEnumerable filters = this.container.Invoke(filterType);
    
                foreach (dynamic actionFilter in filters)
                {
                    actionFilter.OnActionExecuting((dynamic)attribute, context);
    
                    // ADDED THIS in order to send my BadRequest response right 
                    // back to the caller [of the Web API endpoint]
                    if (context.Response != null)
                    {
                        return Task.FromResult(context.Response);
                    }
                }
            }
    
            return continuation();
        }
        ...
    }