0

I have observed a strange behavior in Mvc and not sure if it's a bug or a feature.

I have a customer validation attribute MyAttribute and use it e.g. like this:

public async Task<IActionResult> GetData([MyAttribute] string myparam)

Surprisingly enough the attribute (IsValid function) is never called if this parameter is not in url, but it gets called if the parameter is in Url, but empty (e.g. myParam=). It also gets called if I have a required attribute:

public async Task<IActionResult> GetData([MyAttribute, Required] string myparam)

Now if i have a request class like this:

public class MyRequest
{
    [MyAttribute]
    public string Test {get;set;}
}

public async Task<IActionResult> GetData([FromBody] MyRequest myparam)

Then the attribute gets called even without Required parameter.

Is there a bug somewhere here or is it intended to be like that?

Ilya Chernomordik
  • 27,817
  • 27
  • 121
  • 207
  • When it is called, what is the string parameter? – jdweng Nov 11 '19 at 14:32
  • Just null (e.g. with Requried case) – Ilya Chernomordik Nov 11 '19 at 14:40
  • Is it null or string.Empty? You can add a break point and then when the break point is hit use menu : Debug : Windows : Call Stack to find out which method is calling the GetData. – jdweng Nov 11 '19 at 14:58
  • It is null (not empty) when Required attribute is present – Ilya Chernomordik Nov 11 '19 at 15:19
  • null and nothing are not the same. If you have MyFunction() and MyFunction(object) they are not the same. You are getting the 2nd with object == null. – jdweng Nov 11 '19 at 15:37
  • I am sorry, I don't quite understand what you mean, I don't have any overloads that can cause this issue – Ilya Chernomordik Nov 11 '19 at 15:39
  • The calling method is passing one parameter which is equal to null. Since the class MyRequest has one property 'Test' you will always get one parameter. When Test is not set you are getting null as the value of Test. – jdweng Nov 11 '19 at 15:49

1 Answers1

1

First it will be very helpful if we debug with Asp.net core source code. See more here: debug-net-core-source-visual-studio-2019. The key point of this issue in source code is: EnforceBindRequiredAndValidate. Here is the source code:

private void EnforceBindRequiredAndValidate(
            ObjectModelValidator baseObjectValidator,
            ActionContext actionContext,
            ParameterDescriptor parameter,
            ModelMetadata metadata,
            ModelBindingContext modelBindingContext,
            ModelBindingResult modelBindingResult)
        {
            RecalculateModelMetadata(parameter, modelBindingResult, ref metadata);

            if (!modelBindingResult.IsModelSet && metadata.IsBindingRequired)
            {
                // Enforce BindingBehavior.Required (e.g., [BindRequired])
                var modelName = modelBindingContext.FieldName;
                var message = metadata.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(modelName);
                actionContext.ModelState.TryAddModelError(modelName, message);
            }
            else if (modelBindingResult.IsModelSet)
            {
               // Enforce any other validation rules
                baseObjectValidator.Validate(
                    actionContext,
                    modelBindingContext.ValidationState,
                    modelBindingContext.ModelName,
                    modelBindingResult.Model,
                    metadata);
            }
            else if (metadata.IsRequired)
            {
                // We need to special case the model name for cases where a 'fallback' to empty
                // prefix occurred but binding wasn't successful. For these cases there will be no
                // entry in validation state to match and determine the correct key.
                //
                // See https://github.com/aspnet/Mvc/issues/7503
                //
                // This is to avoid adding validation errors for an 'empty' prefix when a simple
                // type fails to bind. The fix for #7503 uncovered this issue, and was likely the
                // original problem being worked around that regressed #7503.
                var modelName = modelBindingContext.ModelName;

                if (string.IsNullOrEmpty(modelBindingContext.ModelName) &&
                    parameter.BindingInfo?.BinderModelName == null)
                {
                    // If we get here then this is a fallback case. The model name wasn't explicitly set
                    // and we ended up with an empty prefix.
                    modelName = modelBindingContext.FieldName;
                }

                // Run validation, we expect this to validate [Required].
                baseObjectValidator.Validate(
                    actionContext,
                    modelBindingContext.ValidationState,
                    modelName,
                    modelBindingResult.Model,
                    metadata);
            }
        }

If we add required attribute The condition 'else if(metadata.IsRequired)' will be hit, and then it will call Validate function.

Here is the differences between issue request and normal request: enter image description here enter image description here the value of modelBindingResult is very important. if the result contains success then it will call validate in function EnforceBindRequiredAndValidate enter image description here

From the source code we could find this is by-design behavior. 

  var modelBindingContext = DefaultModelBindingContext.CreateBindingContext(
                actionContext,
                valueProvider,
                metadata,
                parameter.BindingInfo,
                parameter.Name);

modelBindingResult is coming from above context. You will find that we need to provide related provider and parameter then modelBindingResult.IsModelSet will be set to true, at this time the function validate will be called.

DevPreSupport_MSFT
  • 274
  • 1
  • 5