Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Receive "The returnUrl field is required" validation error when enabling Nullable reference types #22656

Closed
bzamora opened this issue Jun 8, 2020 · 11 comments
Assignees
Labels
area-identity Includes: Identity and providers

Comments

@bzamora
Copy link

bzamora commented Jun 8, 2020

Describe the bug

When enabling nullable reference types I receive a validation error "The returnUrl field is required" when navigating to:
~/Identity/Account/Register
or
~/Identity/Account/Login

To Reproduce

Start from a new aspnetcore identity template with individual account, all views scaffolded. Then enable Nullable reference types in the project,

<Nullable>enable</Nullable>

Navigate to ~/Identity/Account/Register. You should see in the validation summary

"The returnUrl field is required"

Further technical details

  • ASP.NET Core version Microsoft.AspNetCore.App 3.1.4

image

image

@bzamora
Copy link
Author

bzamora commented Jun 8, 2020

Changing the OnGetAsync parameter

from
public async Task OnGetAsync(string returnUrl = null)

to
public async Task OnGetAsync(string? returnUrl = null)

Solves the error. Maybe good to note in documentation somewhere as it took a bit to find the exact cause. Hopefully helps others...

@Pilchie Pilchie added the area-identity Includes: Identity and providers label Jun 8, 2020
@HaoK
Copy link
Member

HaoK commented Jun 9, 2020

What do we want to do here @blowdart generating different scaffolding code based on nullability options is not something we currently are planning are we?

@blowdart
Copy link
Contributor

blowdart commented Jun 9, 2020

Oh dear.

I honestly don't know, I'd assume that we want to support nullability, which in turn may need scaffolding changes. @Pilchie @DamianEdwards this is probably an opinion for you to weigh in on.

@Pilchie
Copy link
Member

Pilchie commented Jun 9, 2020

I'm confused as to how this causes a runtime error? The nullable annotations in the C# compiler shouldn't change the generated code afaik.

@HaoK
Copy link
Member

HaoK commented Jun 10, 2020

Yeah its weird that turning on the feature would cause a different runtime behavior, but wrt to the general question:

From https://devblogs.microsoft.com/dotnet/embracing-nullable-reference-types/

"We strongly encourage authors of libraries (and similar infrastructure, such as code generators) to adopt NRTs during the nullable rollout phase."

Should we be updating the identity ui code + scaffolded output to support nullable reference types in 5.0, basically to make this flow just work (scaffold all + enable nullable)

@Pilchie
Copy link
Member

Pilchie commented Jun 10, 2020

Ideally yes. @pranavkm has already been looking at adding nullable annotations to a bunch of the ASP.NET Core code in the last few weeks.

@pranavkm
Copy link
Contributor

MVC infers that all non-nullable types are required - https://docs.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-3.1#required-attribute. string returnUrl is non-nullable, which would explain the error message.

@blowdart
Copy link
Contributor

If we change the template would it break if you have have nullable types off?

@pranavkm
Copy link
Contributor

You can enable or disable nullables on individual C# files using pragmas.

#nullable enable

I don't know what the equivalent gesture is for .cshtml files (@NTaylorMullen).

@NTaylorMullen
Copy link
Contributor

I don't know what the equivalent gesture is for .cshtml files (@NTaylorMullen).

We don't have one to apply to the entire file sadly. Razor files inherit the projects nullability so if you turn it on the for the entire project then Razor files will have it turned on.

@blowdart
Copy link
Contributor

Closing in favor of aggregating issue, #25310

@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-identity Includes: Identity and providers
Projects
None yet
Development

No branches or pull requests

6 participants