dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

CompareAttribute.IsValid results in null ref #42490

Open glararan opened 4 years ago

glararan commented 4 years ago
var compareAttribute = new CompareAttribute("Test");
compareAttribute.IsValid("foo");

Stack trace:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.ComponentModel.DataAnnotations.CompareAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value)

Describe the bug

I have multiple models however each model which contains Password/ConfirmPassword cause crash after setting ConfirmPassword. (EDIT: I found out its caused by CompareProperty attribute)

To Reproduce

I implemented my own ValidationContext class which validates on field changed (EditContext) all attributes per changing field.

Implementation of ValidationContext for OnFieldChanged looks like this:

void EditContext_OnFieldChanged(object sender, FieldChangedEventArgs e)
{
    PropertyInfo property = Model.GetType().GetProperty(e.FieldIdentifier.FieldName);

    if (property is null)
        return;

    IEnumerable<ValidationAttribute> attributes = property.GetCustomAttributes<ValidationAttribute>();

    List<string> errors = new List<string>();

    if(Errors.ContainsKey(e.FieldIdentifier.FieldName))
        Errors.Remove(e.FieldIdentifier.FieldName);

    foreach(ValidationAttribute attribute in attributes)
    {
        if (!attribute.IsValid(property.GetValue(Model)))
            errors.Add(attribute.ErrorMessage);
    }

    if (errors.Count > 0)
        Errors[e.FieldIdentifier.FieldName] = errors;
}

Crashing on this place. Only if I change field ConfirmPassword!

        if (!attribute.IsValid(property.GetValue(Model)))

Debugger says e.FieldIdentifier.FieldName is "ConfirmPassword"

Model definition

[Required, Display(Name = "Password", ResourceType = typeof(Translations.Accounts)), DataType(DataType.Password)]
public string Password { get; set; }
[Required, Display(Name = "ConfirmPassword", ResourceType = typeof(Translations.Accounts)), DataType(DataType.Password), CompareProperty("Password", ErrorMessageResourceName = "Compare", ErrorMessageResourceType = typeof(Translations.Accounts))]
public string ConfirmPassword { get; set; }

Razor

<div class="form-group">
    <label for="ConfirmPassword" class="control-label">@Translations.ConfirmPassword</label>
    <InputText type="password" name="ConfirmPassword" class="@($"form-control {ModelContext.DisplayError(x => x.ConfirmPassword)}")" placeholder="@Translations.ConfirmPassword" required="@(!Edit || !string.IsNullOrEmpty(Model.Password))" minlength="@Policy.MinLength" maxlength="@Policy.MaxLength" @bind-Value="Model.ConfirmPassword" />

    @if (ModelContext.HasError(x => x.ConfirmPassword))
    {
    <i class="fas fa-exclamation-triangle invalid-feedback"></i>
    <span class="invalid-feedback"><ValidationMessage For="@(() => Model.ConfirmPassword)" /></span>
    }
</div>

Exceptions (if any)

blazor.webassembly.js:1 crit: Microsoft.AspNetCore.Components.WebAssembly.Rendering.WebAssemblyRenderer[100]
      Unhandled exception rendering component: Object reference not set to an instance of an object.
System.NullReferenceException: Object reference not set to an instance of an object.
  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) <0x2877a78 + 0x000ce> in <filename unknown>:0 
--- End of stack trace from previous location where exception was thrown ---

  at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion (System.Threading.Tasks.Task task) <0x2ffc038 + 0x000da> in <filename unknown>:0 
  at Microsoft.AspNetCore.Components.RenderTree.Renderer.GetErrorHandledTask (System.Threading.Tasks.Task taskToHandle) <0x2ffffc0 + 0x000b6> in <filename unknown>:0 

Further technical details

Any idea how to get more debug info?

glararan commented 4 years ago

Well I find problem.. Its CompareProperty attribute.

glararan commented 4 years ago

If anyone else struggles with same issue. I did workaround.

However I dont know whats happening inside IsValid?

foreach(ValidationAttribute attribute in attributes)
{
    if (attribute is ComparePropertyAttribute comparePropertyAttribute)
    {
        PropertyInfo compareProperty = Model.GetType().GetProperty(comparePropertyAttribute.OtherProperty);

        if (compareProperty != null && (string)property.GetValue(Model) != (string)compareProperty.GetValue(Model))
            errors.Add(comparePropertyAttribute.ErrorMessage);
    }
    else if (!attribute.IsValid(property.GetValue(Model)))
        errors.Add(attribute.ErrorMessage);
}
mkArtakMSFT commented 4 years ago

Thanks for contacting us. It's not clear where in Blazor the error is here. The exception you're referring to indicates an error from your own code. If you believe there is an issue in the framework, please file a new issue with a minimalistic code to help repro the error.

glararan commented 4 years ago

@mkArtakMSFT well issue is in IsValid method of class ComparePropertyAttribute inside nuget package Microsoft.AspNetCore.Components.DataAnnotations.Validation which should be in this repo right?

mkArtakMSFT commented 4 years ago

@glararan can you please provide a repro project?

glararan commented 4 years ago

@mkArtakMSFT successfully reproduced at https://github.com/glararan/22709/tree/master/22709

steps:

launch project navigate into 22709 type something to "Password" type something to "Confirm Password" unfocus Confirm Password exception is thrown

ghost commented 4 years ago

Tagging subscribers to this area: @ajcvickers See info in area-owners.md if you want to be subscribed.

pranavkm commented 4 years ago

Moved to the runtime repo to provide a better error message for this case. That said, you are going to unable to call IsValid on individual attributes and get the right result. Have a look at how Blazor implements it's validation: https://github.com/dotnet/aspnetcore/blob/f52077f599f0d3b4545d8f87a0c3d15217b33069/src/Components/Forms/src/EditContextDataAnnotationsExtensions.cs#L26

kaide commented 1 year ago

I got the same null issue with CompareAttribute.IsValid(object value), so I use GetValidationResult(object value, ValidationContext validationContext) instead.