Yes, I think itβs good practice to limit your actions only to the appropriate HTTP method that it should handle, this will lead to failure of bad requests from your system, decrease the effectiveness of possible attacks, improve the documentation of your code, apply the RESTful project, etc.
Yes, using the attributes [HttpGet] , [HttpPost] .. can make it difficult to read your code, especially if you also use other attributes such as [OutputCache] , [Authorize] , etc.
I use a little trick with a custom IActionInvoker , instead of using attributes, I add an HTTP method to the name of the action method, for example:
public class AccountController : Controller { protected override IActionInvoker CreateActionInvoker() { return new HttpMethodPrefixedActionInvoker(); } public ActionResult GetLogOn() { ... } public ActionResult PostLogOn(LogOnModel model, string returnUrl) { ... } public ActionResult GetLogOff() { ... } public ActionResult GetRegister() { ... } public ActionResult PostRegister(RegisterModel model) { ... } [Authorize] public ActionResult GetChangePassword() { ... } [Authorize] public ActionResult PostChangePassword(ChangePasswordModel model) { ... } public ActionResult GetChangePasswordSuccess() { ... } }
Note that this does not change the names of actions that are still LogOn , LogOff , Register , etc.
Here is the code:
using System; using System.Collections.Generic; using System.Web.Mvc; public class HttpMethodPrefixedActionInvoker : ControllerActionInvoker { protected override ActionDescriptor FindAction(ControllerContext controllerContext, ControllerDescriptor controllerDescriptor, string actionName) { var request = controllerContext.HttpContext.Request; string httpMethod = request.GetHttpMethodOverride() ?? request.HttpMethod; // Implicit support for HEAD method. // Decorate action with [HttpGet] if HEAD support is not wanted (eg action has side effects) if (String.Equals(httpMethod, "HEAD", StringComparison.OrdinalIgnoreCase)) httpMethod = "GET"; string httpMethodAndActionName = httpMethod + actionName; ActionDescriptor adescr = base.FindAction(controllerContext, controllerDescriptor, httpMethodAndActionName); if (adescr != null) adescr = new ActionDescriptorWrapper(adescr, actionName); return adescr; } class ActionDescriptorWrapper : ActionDescriptor { readonly ActionDescriptor wrapped; readonly string realActionName; public override string ActionName { get { return realActionName; } } public override ControllerDescriptor ControllerDescriptor { get { return wrapped.ControllerDescriptor; } } public override string UniqueId { get { return wrapped.UniqueId; } } public ActionDescriptorWrapper(ActionDescriptor wrapped, string realActionName) { this.wrapped = wrapped; this.realActionName = realActionName; } public override object Execute(ControllerContext controllerContext, IDictionary<string, object> parameters) { return wrapped.Execute(controllerContext, parameters); } public override ParameterDescriptor[] GetParameters() { return wrapped.GetParameters(); } public override object[] GetCustomAttributes(bool inherit) { return wrapped.GetCustomAttributes(inherit); } public override object[] GetCustomAttributes(Type attributeType, bool inherit) { return wrapped.GetCustomAttributes(attributeType, inherit); } public override bool Equals(object obj) { return wrapped.Equals(obj); } public override int GetHashCode() { return wrapped.GetHashCode(); } public override ICollection<ActionSelector> GetSelectors() { return wrapped.GetSelectors(); } public override bool IsDefined(Type attributeType, bool inherit) { return wrapped.IsDefined(attributeType, inherit); } public override string ToString() { return wrapped.ToString(); } } }