dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.37k stars 9.99k forks source link

Blazor-Server bypassing form (model) validations. #49319

Closed VitalJeevanjot closed 1 year ago

VitalJeevanjot commented 1 year ago

Is there an existing issue for this?

Describe the bug

As I observed, I created 3 buttons in EditForm component, One (named= back) at top with no type mentioned (I presume something like this can be edited in front-end), 2 buttons at bottom one with type=submit and one with type=button. Now OnValidSubmit and OnInvalidSubmit are configured where one referring to store operations in database and another a console log respectively.

There first button (back) have @onclick event to do some bool assignment on different function. OnValidSubmit function works fine on submit button click but when i click back button it fires the onclick as well as the submit operation from function without validation.

Although my route was to first check for validation with submit clicked and then click back button and while submit do not update database, back button was updating it.

Expected Behavior

The back button should not call the OnValidSubmit function.

Steps To Reproduce

I cannot share repo it is private. But please use above description from where you can re create this.

Exceptions (if any)

No response

.NET Version

7

Anything else?

No response

Flachdachs commented 1 year ago

When a button is associated with a form the default value for the type attribute is submit (i.e. if not specified or invalid). That's a standard browser behavior. You have to make your intentions clear and give the button a type="button" to not activate the submit action.

VitalJeevanjot commented 1 year ago

Understood, Now it also makes sense that a form can have 2 submit buttons. But why explicitly defined one validates model (data annotations) and the other one (with @onclick attrib) do not (that was the reason why i opened it)

javiercn commented 1 year ago

@VitalJeevanjot thanks for contacting us.

Can you provide a minimal repro project as a public github repository with instructions on how to reproduce the issue?

ghost commented 1 year ago

Hi @VitalJeevanjot. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost commented 1 year ago

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project that illustrates the problem.

VitalJeevanjot commented 1 year ago

Sure, I will prepare one tomorrow.

ghost commented 1 year ago

Hi @VitalJeevanjot. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

VitalJeevanjot commented 1 year ago

Hi @javiercn @mkArtakMSFT and Everyone! ,

So I have uploaded 2 repos. A: One is very trimmed down version of the project i am working: https://github.com/VitalJeevanjot/form-testor-blazor-server-bug/tree/main B: Another is I tried to replicate from scratch the possibilities that can reproduce bug: https://github.com/VitalJeevanjot/BlazorApp-server-form-submit-testing

To Reproduce:

A at this line https://github.com/VitalJeevanjot/form-testor-blazor-server-bug/blob/main/tutorial-maker-blazor-server/Shared/SidebarLeft.razor#L39C29-L39C29 will log Valid OK ........... after its own click function fired (Which as of explained makes sense that it fired as submit) but what doesn't is that when I click Save (intentional submit) it will fire NotValid printing not valid log and showing the first one as differentiated itself from Model validations (approving of errors).


Note: I tried to re-create this in project B by trying to copy possible conflicting zones but it not producing the required result to show.

Please try out and let me know if A produces bug and if not I will create video of it. Or possibly I am just missing a crucial piece here.

Thanks.

VitalJeevanjot commented 1 year ago

I have re-created this in Project B as well (Shows in last commit message).

The thing is that hiding form (possibly InputText component or related ones or validating messages or the whole EditForm) will produce this bug when click button which is either not type=button or type=submit with hiding form through its @onclick event.

So In-Short, form existence can be removed with a custom button element causing it to validate on blazor-server.

VitalJeevanjot commented 1 year ago

As suspected:

I removed, The type=button from the button (Rendered In browser) image

and it immediately pushed Invalid Values to the DB

Screenshot 2023-07-12 051841

Causing the name field to be empty

I think this is because of the way OnValidSubmit works which could be the cause of this bug.

Flachdachs commented 1 year ago

Using your project B - the code in question can be found in Shared/SurveyPrompt.razor - I figured something out. At first, as I said, your Back button needs a type="button" otherwise it is a submit button, which is not intended in your case. Doing this you should be fine.

However, there is another issue. The GoBack method sets example2=true and in this case the whole EditForm is not rendered, but this should happen after the validation, I think. I dug a bit in the framework code (7.0.7) and figured out so far that the EditContext of the EditForm has an empty _fieldStates collection in this case, and this means: no validation messages =>form is valid. If you don't change example2 or in other words: if you don't suppress the rendering of the EditForm everything is as expected. - I still can't explain why this has already an influence in the validation phase. My guess is that the @onclick handler has an implicit StateHasChanged() call, which leads to an immediately re-rendering of the EditForm, which changes things in the EditContext, and then the validation can't happen as planned.

VitalJeevanjot commented 1 year ago

However, there is another issue.

That's the current issue actually . I am not suppressing it. It is accidental insertion of un-rendering edit form button over not thinking for possible issues with it.... Explained below 👇

Using your project B - the code in question can be found in Shared/SurveyPrompt.razor - I figured something out. At first, as I said, your Back button needs a type="button" otherwise it is a submit button, which is not intended in your case. Doing this you should be fine.

Please checkout last post that through devtools it's easier for anybody to remove type=button on some behaviour where EditForm will all un-render itself at front which causes this issue. The only solution so far to me remain that to use backbutton outside the EditForm.

I still can't explain why this has already an influence in the validation phase. My guess is that the @onclick handler has an implicit StateHasChanged()

StatehasChange() explicit call have no effect on these, I just tried every single possibility I could think of to re create this bug in fresh project.

VitalJeevanjot commented 1 year ago

Also, Please Note: However i do tried to deepcopy elements to hijack the behaviour through elements by putting (copying) buttons in editform through different libraries (Jquery, Moo, and also tried with vanilla) on different browsers (Chrome/Firefox for now) and so far so good. But don't know which browser will allow (bypass existing securities) and copy all the events / handlers properties objects of blazor etc. allowing unexpected validation passes.

And even in current state a button that un-render Edit Form inside edit form (for some reason) is added it can bypass validations.

mkArtakMSFT commented 1 year ago

@SteveSandersonMS can you please investigate this? The challenge here is that if this is an issue, it's in 7, which means changing it will be a behavior breaking change (unless I'm missing something) so we may be in a tough spot here.

SteveSandersonMS commented 1 year ago

This happens because of a combination of things:

  1. You have a button (the one labelled 'back') that is inside a form and does not have type="button", and so it submits the form when clicked. This is a standard HTML behavior, not specific to Blazor.
  2. That button also has an @onclick event.
  3. By standard HTML behaviors (again, not specific to Blazor), this means that when you click that button, it fires first an click event and then later fires a submit event on the enclosing form.
  4. Your onclick handler removes all validation rules from the form (because it removes the DataAnnotationsValidator).
  5. The later submit event then runs and has no validation rules to enforce, so regards it as a valid submit.

I know the behavior is surprising and not what you want, but everything here is by design and follows from the standard HTML behaviors for form elements.

To avoid this problem, you have various options:

<EditForm EditContext="@editContext" ... > ... </EditForm>

@code {
    private EditContext editContext;
    public DataModelExample dme = new DataModelExample();

    protected override void OnInitialized()
    {
        // By adding Data Annotations rules here, that becomes independent of rendering
        editContext = new EditContext(dme).AddDataAnnotationsValidation();
    }
}

At the very least I think you should add type="button" to avoid unintentional submit events.

VitalJeevanjot commented 1 year ago

@SteveSandersonMS Hi,

Thanks for adding it that it is By Design which makes a solution to not add any button with @onclick event that make DB changes and hides form further inside the form.

Because as i mentioned earlier from devtools of any browser you can easily (very easily) remove type="button" attribute which creates same issue as tested again and again

But what i was asking for is that (Because I consider you a pro developer) is your conclusion that an element (button that can make changes to DB from form) serving from same server cannot be cloned with all its blazor events and properties and then cannot be appended form devtools inside form where it can remove the required form or hide it with its validations or hide validations alone as this seems to be counting validations messages before saying invalid(just a presumption) with some appended js to @onclick event (I tested it many times that day as mentioned above and after that but was not succeeded so far but more curious on your sayings, Hence finished my project ) in some insecure browser (because server changes will be universal).