2

I have a simple API controller that accepts string values to identify an item to be deleted. So a DELETE call to /name/foo will delete foo, as expected.

When testing, I've found that URL encoding for a space %20 passed from the client as the first and only character of the final segment in a route (like /name/%20) causes the server to respond with a 500. When other characters are included, something like /name/first%20last, those values are properly passed through to the DeleteByName() method as first last, without issue.

When debugging, I've found that a DELETE to /name/%20 passes the framework validations as a valid route to be acted upon by the DeleteByName() method, but the name variable is null and throws a null pointer. I've resolved the null pointer by including a null check, but it doesn't feel like a best practice, and doesn't explain why this is happening in the first place.

Here's some example code

    [Route("api/[controller]")]
    [ApiController]

        ...

        // DELETE: api/name
        [HttpDelete("{name}")]
        public async Task<IActionResult> DeleteByName(string name)
        {
            if (name == null) // this check handles the error, but doesn't explain why it happens
            {
                return NotFound();
            }

            await person.Delete(name); // without the null check, a null pointer is thrown on 'name'

            return Ok();
        }

Why is this null, as opposed to a string with one space ?

Is there a better way to handle this? (Like from the framework; without requiring a null check in the method body)

1
  • Any update? Does my reply help you? Commented Dec 4, 2020 at 1:24

2 Answers 2

2

As far as I know, this is asp.net core default model binding expected behavior. According to the source codes, you could find, it will check the string IsNullOrWhiteSpace, if the string is the space, it will also be null.

If you don't want this behavior, the only way is creating a custom string model binder.

More details about how to create the custom string model binder, you could refer to below codes:

CustomStringBinder :

public class CustomStringBinder : IModelBinder
{
    private readonly TypeConverter _typeConverter;
    private readonly ILogger _logger;

    public CustomStringBinder(Type type, ILoggerFactory loggerFactory)
    {
        if (type == null)
        {
            throw new ArgumentNullException(nameof(type));
        }

        if (loggerFactory == null)
        {
            throw new ArgumentNullException(nameof(loggerFactory));
        }

        _typeConverter = TypeDescriptor.GetConverter(type);
        _logger = loggerFactory.CreateLogger<SimpleTypeModelBinder>();
    }

    /// <inheritdoc />
    public Task BindModelAsync(ModelBindingContext bindingContext)
    {
        if (bindingContext == null)
        {
            throw new ArgumentNullException(nameof(bindingContext));
        }


        var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName);
        if (valueProviderResult == ValueProviderResult.None)
        {

             return Task.CompletedTask;
        }

        bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);

        try
        {
            var value = valueProviderResult.FirstValue;

            object model;
            if (bindingContext.ModelType == typeof(string))
            {
                // Already have a string. No further conversion required but handle ConvertEmptyStringToNull.
                if (bindingContext.ModelMetadata.ConvertEmptyStringToNull && string.IsNullOrEmpty(value))
                {
                    model = null;
                }
                else
                {
                    model = value;
                }
            }
            else if (string.IsNullOrEmpty(value))
            {
                // Other than the StringConverter, converters Trim() the value then throw if the result is empty.
                model = null;
            }
            else
            {
                model = _typeConverter.ConvertFrom(
                    context: null,
                    culture: valueProviderResult.Culture,
                    value: value);
            }

            CheckModel(bindingContext, valueProviderResult, model);

           
            return Task.CompletedTask;
        }
        catch (Exception exception)
        {
            var isFormatException = exception is FormatException;
            if (!isFormatException && exception.InnerException != null)
            {
                // TypeConverter throws System.Exception wrapping the FormatException,
                // so we capture the inner exception.
                exception = ExceptionDispatchInfo.Capture(exception.InnerException).SourceException;
            }

            bindingContext.ModelState.TryAddModelError(
                bindingContext.ModelName,
                exception,
                bindingContext.ModelMetadata);

            // Were able to find a converter for the type but conversion failed.
            return Task.CompletedTask;
        }
    }

    protected virtual void CheckModel(
ModelBindingContext bindingContext,
ValueProviderResult valueProviderResult,
object model)
    {
        // When converting newModel a null value may indicate a failed conversion for an otherwise required
        // model (can't set a ValueType to null). This detects if a null model value is acceptable given the
        // current bindingContext. If not, an error is logged.
        if (model == null && !bindingContext.ModelMetadata.IsReferenceOrNullableType)
        {
            bindingContext.ModelState.TryAddModelError(
                bindingContext.ModelName,
                bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor(
                    valueProviderResult.ToString()));
        }
        else
        {
            bindingContext.Result = ModelBindingResult.Success(model);
        }
    }



}

StringBinderProvider

public class StringBinderProvider : IModelBinderProvider
{
    public IModelBinder GetBinder(ModelBinderProviderContext context)
    {
        if (context == null)
        {
            throw new ArgumentNullException(nameof(context));
        }

        if (context.Metadata.ModelType == typeof(string))
        {
            var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();

            return new CustomStringBinder(context.Metadata.ModelType, loggerFactory);
        }

        return null;
    }
}

Register the binder in startup.cs ConfigureServices method:

        services.AddControllers(options=> {
            options.ModelBinderProviders.Insert(0, new StringBinderProvider());
        });

Result:

enter image description here

Sign up to request clarification or add additional context in comments.

Comments

0

Because you are confusing FromQuery and FromRoute. The default is [FromQuery]. You need to specify it as [FromRuote]. If you want it to be from anywhere except quer yparamaters you need to specify it explicitly.

[HttpDelete("{name}")]
public async Task<IActionResult> DeleteByName([FromRoute]string name)

[HttpDelete]
public async Task<IActionResult> DeleteByName([FromQuery]string name)

So when referencing it in URL it will look as follow

From Route -> api/Controller/1

From Query -> api/Controller?name=1

Lets take another example:

[HttpGet("User/{username}/Account")]
public async Task<IActionResult> GetByUsername([FromRoute]string username)

[HttpGet("User/Account")]
public async Task<IActionResult> GetByUsername([FromQuery]string username)

Again lets look at how it will look from URL:

From Route -> api/Controller/Tom/Account

From Query -> api/Controller/Account?username=Tom

There are three main ones:

[FromQuery] // Get info from Query Parameters
[FromRoute] // Get Info From Route
[FromBody] // Get Information from Body

You can user these tags in the same endpoint. Example:

[HttpGet("User/{username}/Account")]
public async Task<IActionResult> CreateNewAccount(
    [FromRoute]string username,
    [FromQuery]string accountId,
    [FromBody]UserInfoClass userInfo
    ){...}

URL -> api/Controller/Tom/Account?accountId=123456

Ok now the %20 is just the encoding for a space in url. It's not the cause of the null reference. You are trying to find a query parameter, but you are specifying it as route parameter.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.