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

Model binding using IParsable<T> doesn't allow handling of null values #59597

Open
1 task done
mladenb opened this issue Dec 22, 2024 · 0 comments
Open
1 task done

Model binding using IParsable<T> doesn't allow handling of null values #59597

mladenb opened this issue Dec 22, 2024 · 0 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Comments

@mladenb
Copy link

mladenb commented Dec 22, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Hi,

If we have an action method signature like this one: public IActionResult GetTasks(FromQuery] List<string> list)
and we call such a method, with an example url: /api/tasks?list= (or omit the list parameter)
ASP.NET will bind the GetTasks' list argument to an empty list, instead of a null value

If we provide a custom parsing for the lists, allowing, for example, csv values to be parsed as well, using a custom CsvList implementation:

	public class CsvList<T> : List<T>, IParsable<CsvList<T>>
	{
		public static CsvList<T> Parse(string value, IFormatProvider? provider)
		{
			...
		}

		public static bool TryParse(string? value, IFormatProvider? provider, out CsvList<T> result)
		{
			...
		}
	}

(which is a great improvement imho, comparing to custom model binders, value providers, etc. thanks for that!!)
and we also change the action method's signature to: public IActionResult GetTasks(FromQuery] CsvList<string> list)
but, it seems that CsvList.TryParse() (nor CsvList.Parse) gets called at all, if the query string doesn't contain the list parameter (or it is empty), even though the TryParse() method can accept nullable string values. The model binding process simply throws an error, saying the parameter is required, instead of calling the TryParse() with a null value.

It seems sad that we couldn't just handle the null values as well, and still return the empty list if we wanted to, instead of model binder deciding that for us.

Is this a bug or a feature? :)

Expected Behavior

IParsable<T> implementations should be called with null values as well, to allow us to handle all the cases/variations of input values.

Steps To Reproduce

To keep things simple, I haven't shared any additional code, hoping you'll manage to reproduce this issue easily.

Exceptions (if any)

No response

.NET Version

9.0.100 (but the project is configured for v8, if that's relevant)

Anything else?

Host:
Version: 9.0.0
Architecture: x64
Commit: 9d5a6a9aa4

.NET SDKs installed:
8.0.206 [C:\Program Files\dotnet\sdk]
9.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 9.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 9.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.11 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 9.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Dec 22, 2024
@martincostello martincostello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

2 participants