DataAnnotation validation ignored

[ducalai]

DataAnnotation validation is ignored if route parameter has the same name as object variable passed in the body of the HTTP request

Model:

public class TransferInfo
{
        [Range(1, int.MaxValue)] //or CustomValidationAttribute
        public int AccountId { get; set; }
        public double Amount { get; set; }
}

Controller:

[HttpPost("{accountId:int}")]
public IActionResult Transfer(int accountId, [FromBody] TransferInfo transferInfo)
{
    if (!ModelState.IsValid)
    {
        return HttpBadRequest();
    }
    ...
}

Posted data (JSON):

{
     "accountId": 0,
     "amount": 250.0
}

ModelState is valid even if I check that accountId must be greater than 0, using Range or Custom validation attribute.

If I update my TransferInfo class definition and rename "AccountId" by "ToAccountId" and try to send:

{
     "toAccountId: 0,
     "amount": 250.0
}

ModelState Validation will failed

[tuespetre]

IIRC from digging around in the validation class, this will be because a validation state entry with a key of "AccountId" (case-insensitive) has already been populated. If you place the TransferInfo argument first in the signature, does the validation take effect?

[ducalai]

I haven't think of that. You are right, validation works if TransferInfo is first in the signature.
Thanks!

[Eilon]

@rynowak thoughts on this? This seems orthogonally tangentially related to the issue with the cancellation token with regard to the truth of what's in the model state dictionary...

[Eilon]

@rynowak ping

[rynowak]

Yep. This is definitely related to #3743

[rynowak]

@Eilon - I don't think this is related to my other validation fix. Self-assigning and will fix.

[rynowak]

The issue here is that we don't try to validate something that already appears valid. The idea is to avoid duplicating work when we have a cycle. However, cycles are rare, and validation may depend on the container-object for correctness.

The criteria that we're using in this case is the model state dictionary care, which we've seen can definitely overlap between model objects. We should just change this check to proceed if the state is anything other than Invalid rather than requiring it to be Unvalidated.

[Eilon]

@kichalla donating this bug to you, it is a gift from @rynowak .