Nullable [FromQuery] primitive says "The value 'null' is not valid"

[snebjorn]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

null cannot be sent to a query parameter.

Given this ApiController

[ApiController]
[Route("/api/[controller]")]
public class FoosController : ControllerBase
{
    [HttpGet]
    // note the default value is just to help see if null was passed or the parameter was omitted
    public IActionResult Get([FromQuery] int? foo = 42) 
    {
        return Ok(foo);
    }
}

Note: the default value is just to help see if null was passed or the parameter was omitted.

This is the result:

• GET /api/foos?foo=null -> The value 'null' is not valid.
• GET /api/foos?foo= -> 42
• GET /api/foos?foo -> 42
• GET /api/foos? -> 42

This is unfortunate as there's a difference between omitting the foo param and setting it to null

Expected Behavior

• GET /api/foos?foo=null -> null
• GET /api/foos?foo= -> 42
• GET /api/foos?foo -> 42
• GET /api/foos? -> 42

Steps To Reproduce

1. Call GET /api/foos?foo=null
2. Observe this error

{
    "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
    "title": "One or more validation errors occurred.",
    "status": 400,
    "traceId": "00-52ecb8b97f812e46240c706a0a1b1d8a-1d9f488b55aaca74-00",
    "errors": {
        "foo": [
            "The value 'null' is not valid."
        ]
    }
}

Exceptions (if any)

No response

.NET Version

7.0.100

Anything else?

This is somewhat related #40415 though it's for POST requests.
There's an interesting discussion about how to set query params to null. Examples: ?key, ?key= or ?key=null.
My vote is on ?key=null.

dotnet --info

.NET SDK:
Version: 7.0.100
Commit: e12b7af219

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22000
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\7.0.100\

Host:
Version: 7.0.0
Architecture: x64
Commit: d099f075e4

.NET SDKs installed:
5.0.408 [C:\Program Files\dotnet\sdk]
6.0.403 [C:\Program Files\dotnet\sdk]
7.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.11 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
None

Environment variables:
Not set

global.json file:
Not found

Learn more:
https://aka.ms/dotnet/info

Download .NET:
https://aka.ms/dotnet/download

[captainsafia]

Triage: The reason this is happening is because the query parameter ?foo=null is being interpreted as a foo key with the value being the string "null". When the model binding layer of MVC attempts to bind this "null" (string) to an integer the binding fails because it is not parseable.

You can consider leveraging a resource filter to preprocess the parameter yourself before model-binding kicks in.

[ghost]

Hi @snebjorn. 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.

[snebjorn]

Hmm perhaps I can use the resource filter. But I cannot figure out how to do it :(
The parameters appear to be readonly.

public class TestFilter : IResourceFilter
{
    public void OnResourceExecuted(ResourceExecutedContext context) { }

    public void OnResourceExecuting(ResourceExecutingContext context)
    {
        context.HttpContext.Request.Query["foo"] = 123; // this is readonly
    }
}

Also how am I supposed to fix it :) Given that the foo is already parsed as null and the Query type is KeyValuePair<string, StringValues> where StringValues being only strings. So the string "null" is the only possibility.

[captainsafia]

The parameters appear to be readonly.

Yes, in this case you would have to override the Query collection in this case.

Also how am I supposed to fix it :) Given that the foo is already parsed as null and the Query type is KeyValuePair<string, StringValues> where StringValues being only strings. So the string "null" is the only possibility.

Indeed, you may to implement a custom value provider.

@snebjorn By the way, is there a particular reason that you want this behavior?

[ghost]

Hi @snebjorn. 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.

[snebjorn]

By the way, is there a particular reason that you want this behavior?

Besides sending null to a method like this public IActionResult Get([FromQuery] int? foo = 42) being intuitive and in line with how you'd normally call methods in C#

Then the specific issue was a take/limit parameter for a paged request. I wanted it to have a limit by default, should someone be playing around with the API. But have the ability to turn off the limit should they need to.
I "solved" this by treating -1 as null. It works but it feels very old and is less intuitive than sending null to turn off the limit.

[brunolins16]

@snebjorn as mentioned maybe a value provider could help you. Potentially you could create a custom QueryStringValueProvider overriding the GetValue method. Eg.:

using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using System.Globalization;

public class NullableQueryStringValueProvider : QueryStringValueProvider
{
    public NullableQueryStringValueProvider(
        BindingSource bindingSource, 
        IQueryCollection values, 
        CultureInfo? culture) 
        : base(bindingSource, values, culture)
    {
    }

    public override ValueProviderResult GetValue(string key)
    {
        var result = base.GetValue(key);

        if (result.Length > 0)
        {
            // Check for string `null` and recreate the ValueProviderResult with null
        }

        return result;
    }
}

public class NullableQueryStringValueProviderFactory : IValueProviderFactory
{
    /// <inheritdoc />
    public Task CreateValueProviderAsync(ValueProviderFactoryContext context)
    {
        if (context == null)
        {
            throw new ArgumentNullException(nameof(context));
        }
        var query = context.ActionContext.HttpContext.Request.Query;
        if (query != null && query.Count > 0)
        {
            var valueProvider = new NullableQueryStringValueProvider(
                BindingSource.Query,
                query,
                CultureInfo.InvariantCulture);
            context.ValueProviders.Add(valueProvider);
        }
        return Task.CompletedTask;
    }
}

You will also need to add it to the ValueProviders list."

builder.Services.AddControllers(options =>
{
    options.ValueProviderFactories.Insert(0, new NullableQueryStringValueProviderFactory());
});

More information Model Binding in ASP.NET Core

[halter73]

I "solved" this by treating -1 as null. It works but it feels very old and is less intuitive than sending null to turn off the limit.

Another option is to leave out the query string parameter when it's unspecified.

[ghost]

Hi @snebjorn. 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]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.