Aspnetcore: ASP.NET Core model binder deserilises UTC time string to local time rather than UTC time

[ZeanQin]

Describe the bug

When a user passes a UTC time string as query parameter of a GET request, the model binder converts it to a "Local" kind DateTime object rather than a UTC DateTime object. And the user has to call DateTime.ToUniversalTime() to convert it to utc time, or write custom model binder to change the behaviour globally.

To Reproduce

Steps to reproduce the behavior:

1. Send a GET request to an endpoint with a UTC time string such as https://<something>.com/{action}/{id}?time=2019-06-14T02:30:04.0576719Z
2. The time parameter in the corresponding controller is a local time

Expected behavior

The retrieved time should be a utc time.

[DamianEdwards]

Some repro code:

``` c#
[Route("[controller]")]
public class TimeController : Controller
{
public IActionResult Index(DateTime time)
{
var result = $"Current culture: {CultureInfo.CurrentCulture}";

    var rawTime = Request.Query["time"];
    result += $"\r\nRaw querystring value is: {rawTime}";

    // Format string "O" is ISO 8601 round-trippable https://docs.microsoft.com/en-us/dotnet/standard/base-types/standard-date-and-time-format-strings#Roundtrip
    result += $"\r\nModel-bound value is: {time.ToString("O")}, Kind is: {time.Kind}";

    var utcTime = time.ToUniversalTime();
    result += $"\r\nModel-bound converted to UTC time value is: {utcTime.ToString("O")}, Kind is: {utcTime.Kind}";

    var parsedTime = DateTime.Parse(rawTime);
    result += $"\r\nParsed time value is: {parsedTime.ToString("O")}, Kind is: {parsedTime.Kind}";

    var parsedTimeUtc = DateTime.Parse(rawTime, CultureInfo.CurrentCulture, DateTimeStyles.AdjustToUniversal);
    result += $"\r\nParsed UTC time value is: {parsedTimeUtc.ToString("O")}, Kind is: {parsedTimeUtc.Kind}";

    return Content(result);
}

}

Result from request to `https://localhost:44393/time?time=2019-06-14T02:30:04.0576719Z`:

Current culture: en-US
Raw querystring value is: 2019-06-14T02:30:04.0576719Z
Model-bound value is: 2019-06-13T19:30:04.0576719-07:00, Kind is: Local
Model-bound converted to UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc
Parsed time value is: 2019-06-13T19:30:04.0576719-07:00, Kind is: Local
Parsed UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc

Same result is returned from same request when `CultureInfo.CurrentCulture = CultureInfo.CurrentCulture = CultureInfo.InvariantCulture` is executed before model binding runs is the same:

Current culture:
Raw querystring value is: 2019-06-14T02:30:04.0576719Z
Model-bound value is: 2019-06-13T19:30:04.0576719-07:00, Kind is: Local
Model-bound converted to UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc
Parsed time value is: 2019-06-13T19:30:04.0576719-07:00, Kind is: Local
Parsed UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc

As can be seen, the model bound value is the same as the result of calling `DateTime.Parse` on the raw string. There doesn't appear to be a way to invoke `DateTime.Parse` such that it will detect the format of the original string and parse the result accordingly, rather one must call the overload that accepts a `DateTimeStyles` value and ensure the string passed matches. If we wanted to make this work in MVC we'd have to sniff the string value first to detect the trailing `Z` and in that case treat it specially.

Registering a custom model binder for `DateTime` with a different behavior will let you control this anyway you want however. For example, here's a custom `DateTime` model binder based on the in-box `DecimalModelBinder` I whipped up:

``` c#
public class DateTimeModelBinderProvider : IModelBinderProvider
{
    // You could make this a property to allow customization
    internal static readonly DateTimeStyles SupportedStyles = DateTimeStyles.AdjustToUniversal | DateTimeStyles.AllowWhiteSpaces;

    /// <inheritdoc />
    public IModelBinder GetBinder(ModelBinderProviderContext context)
    {
        if (context == null)
        {
            throw new ArgumentNullException(nameof(context));
        }

        var modelType = context.Metadata.UnderlyingOrModelType;
        var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
        if (modelType == typeof(DateTime))
        {
            return new UtcAwareDateTimeModelBinder(SupportedStyles, loggerFactory);
        }

        return null;
    }
}

public class UtcAwareDateTimeModelBinder : IModelBinder
{
    private readonly DateTimeStyles _supportedStyles;
    private readonly ILogger _logger;

    public UtcAwareDateTimeModelBinder(DateTimeStyles supportedStyles, ILoggerFactory loggerFactory)
    {
        if (loggerFactory == null)
        {
            throw new ArgumentNullException(nameof(loggerFactory));
        }

        _supportedStyles = supportedStyles;
        _logger = loggerFactory.CreateLogger<UtcAwareDateTimeModelBinder>();
    }

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

        var modelName = bindingContext.ModelName;
        var valueProviderResult = bindingContext.ValueProvider.GetValue(modelName);
        if (valueProviderResult == ValueProviderResult.None)
        {
            // no entry
            return Task.CompletedTask;
        }

        var modelState = bindingContext.ModelState;
        modelState.SetModelValue(modelName, valueProviderResult);

        var metadata = bindingContext.ModelMetadata;
        var type = metadata.UnderlyingOrModelType;

        var value = valueProviderResult.FirstValue;
        var culture = valueProviderResult.Culture;

        object model;
        if (string.IsNullOrWhiteSpace(value))
        {
            model = null;
        }
        else if (type == typeof(DateTime))
        {
            // You could put custom logic here to sniff the raw value and call other DateTime.Parse overloads, e.g. forcing UTC
            model = DateTime.Parse(value, culture, _supportedStyles);
        }
        else
        {
            // unreachable
            throw new NotSupportedException();
        }

        // When converting value, a null model 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 && !metadata.IsReferenceOrNullableType)
        {
            modelState.TryAddModelError(
                modelName,
                metadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor(
                    valueProviderResult.ToString()));
        }
        else
        {
            bindingContext.Result = ModelBindingResult.Success(model);
        }

        return Task.CompletedTask;
    }
}

You'd wire it up in Startup.cs like this:

``` c#
public void ConfigureServices(IServiceCollection services)
{
services.AddControllers()
.AddMvcOptions(options =>
{
options.ModelBinderProviders.Insert(0, new DateTimeModelBinderProvider());
});
}

And here's the result of the action method above using this model binder:

Current culture: en-US
Raw querystring value is: 2019-06-14T02:30:04.0576719Z
Model-bound value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc
Model-bound converted to UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc
Parsed time value is: 2019-06-13T19:30:04.0576719-07:00, Kind is: Local
Parsed UTC time value is: 2019-06-14T02:30:04.0576719Z, Kind is: Utc
```

[alanextar]

Could please someone tells me where exactly datetime binding is processing?

[JordanMunroe]

@DamianEdwards Thanks for this workaround! Upgrading my dotnet core from 2.x to 3.x broke all the dates and this fixed everything.

[deanmccrae]

@mkArtakMSFT How is a bug this fundamental taking 10 months to be scheduled for a fix (and as of now, not yet fixed)?

[alsami]

@mkArtakMSFT How is a bug this fundamental taking 10 months to be scheduled for a fix (and as of now, not yet fixed)?

You know you can use that custom model-binder provided by @DamianEdwards above and register it globally, right? Pretty easy work around and has been working solid for me for the past ~8 months.

[deanmccrae]

@alsami Yes, the work around is fine, thank you. But my questions still stands.

[chybisov]

Any updates on this? Will it be fixed in ASP.NET Core 3?

[Kikimora]

Workaround provided by @DamianEdwards works fine but throws unhandled exception if incoming date has wrong format. BindModelAsync can be augmented with try/catch as follows (code taken verbatim from DecmalModelBinder)

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

            var modelName = bindingContext.ModelName;
            var valueProviderResult = bindingContext.ValueProvider.GetValue(modelName);
            if (valueProviderResult == ValueProviderResult.None)
            {
                // no entry
                return Task.CompletedTask;
            }

            var modelState = bindingContext.ModelState;
            modelState.SetModelValue(modelName, valueProviderResult);

            var metadata = bindingContext.ModelMetadata;
            var type = metadata.UnderlyingOrModelType;

            try
            {
                var value = valueProviderResult.FirstValue;
                var culture = valueProviderResult.Culture;

                object model;
                if (string.IsNullOrWhiteSpace(value))
                {
                    model = null;
                }
                else if (type == typeof(DateTime))
                {
                    // You could put custom logic here to sniff the raw value and call other DateTime.Parse overloads, e.g. forcing UTC
                    model = DateTime.Parse(value, culture, _supportedStyles);
                }
                else
                {
                    // unreachable
                    throw new NotSupportedException();
                }

                // When converting value, a null model 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 && !metadata.IsReferenceOrNullableType)
                {
                    var message = metadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor(valueProviderResult.ToString());
                    modelState.TryAddModelError(modelName, message);
                }
                else
                {
                    bindingContext.Result = ModelBindingResult.Success(model);
                }
            }
            catch (Exception exception)
            {
                var isFormatException = exception is FormatException;
                if (!isFormatException && exception.InnerException != null)
                {
                    // Unlike TypeConverters, floating point types do not seem to wrap FormatExceptions. Preserve
                    // this code in case a cursory review of the CoreFx code missed something.
                    exception = ExceptionDispatchInfo.Capture(exception.InnerException).SourceException;
                }

                modelState.TryAddModelError(modelName, exception, metadata);

                // Conversion failed.
            }

            return Task.CompletedTask;
        }

[razvangoga]

Hi,

ran into the same issue, but with a twist (after i upgraded an MVC app from 4.7.2 to core 3.1) : i have multiple POST endpoints that serve as the back-end for a SPA front-end where that receive large / deep objects (the DateTime and DateTime? properties are generally 2, 3 levels down the object graph. During the migration i configured the endpoints with the [FromBody] attribute.

[HttpPost]
 public IActionResult SaveGrantInfo([FromBody] GrantEditViewModel viewModel)
{
   ...
}

I've tried the workaround from @DamianEdwards, but it's not working in my scenario due to the [FromBody] attribute that looks only for a model binder for the viewModel's type.

What other options do I have?

Problem is i have multiple such endpoints, each with each own reasonably big viewModel so I would like to avoid major rewrites if possible (if a real fix for this comes in 5.0).

[alsami]

I've tried the workaround from @DamianEdwards, but it's not working in my scenario due to the [FromBody] attribute that looks only for a model binder for the viewModel's type.

This workaround is for query-parameters (probably also path variables). When you POST it's using JSON deserialization. How is it configured for you?

[razvangoga]

@alsami : i haven't configured anything special - i'm using the out-of-the-box Json serialization that asp.net core 3.1 offers (System.text.json based).
My models are pretty big (in some cases i post also the contents of RTE style editors that may include base64 encoded images inside) - i think i would hit the query string limit with them.

Only json related configuration i do is (but it's only because the FE was originally written with the models not camel-cased):

            IMvcBuilder mvcBuilder = services
                .AddControllersWithViews()
                // .AddMvcOptions(op =>
                // {
                //     op.ModelBinderProviders.Insert(0, new DateTimeModelBinderProvider());
                // })
                .AddJsonOptions(opts => opts.JsonSerializerOptions.PropertyNamingPolicy = null);

[alsami]

@razvangoga not 100% sure why it's not working for you but what I'd try is the following

Install package Microsoft.AspNetCore.Mvc.NewtonsoftJson

and do the following instead of AddJsonOptions

.AddNewtonsoftJson(options => {
   options.SerializerSettings.ContractResolver =  new DefaultContractResolver(); // might be not required
   options.SerializerSettings.DateTimeZoneHandling = DateTimeZoneHandling.Utc; // this should be set if you always expect UTC dates in method bodies, if not, you can use RoundTrip instead.
});

It might be some other circumstances causing your problem here but just to make sure that it's not related to some missing setting of System.Text.Json.

[razvangoga]

@alsami so basically your advice would be to switch the app to use Json.net insted of System.Text.Json?

ok i'll give it a try and see what happens

[razvangoga]

@alsami made the switch to NewtonsoftJson and it seems to work as expected. i'll know more on Monday after testing. Thanks for your help!

@DamianEdwards : what would be the recommendation from the Asp.net team on this? Should I just stay on the NewtonsoftJson or is there any other workaround i can try with System.Text.Json?

[chybisov]

@razvangoga I hope this will be fixed in .NET 5. 😃

[alsami]

@alsami made the switch to NewtonsoftJson and it seems to work as expected. i'll know more on Monday after testing. Thanks for your help!

Great that it worked. I am sure there is some way to make it work with System.Text.Json. I thought it's some missing configuration probably, so now you know what to look for.

[CicheR]

This problem did take me by surprise (I lost a couple of hours for it 😭).
Luckily Google got me here.

This is a somewhat simplified version:

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

    string modelName = bindingContext.ModelName;
    ValueProviderResult valueProviderResult = bindingContext.ValueProvider.GetValue(modelName);
    if (valueProviderResult == ValueProviderResult.None)
    {
        // no entry
        return Task.CompletedTask;
    }

    ModelStateDictionary modelState = bindingContext.ModelState;
    modelState.SetModelValue(modelName, valueProviderResult);

    ModelMetadata metadata = bindingContext.ModelMetadata;
    Type type = metadata.UnderlyingOrModelType;

    string value = valueProviderResult.FirstValue;
    CultureInfo culture = valueProviderResult.Culture;

    if (string.IsNullOrWhiteSpace(value))
    {
        // When converting value, a null model 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 (!metadata.IsReferenceOrNullableType)
        {
            string msj = metadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor(valueProviderResult.ToString());
            modelState.TryAddModelError(modelName, msj);
        }
    }
    else if (type == typeof(DateTime))
    {
        // You could put custom logic here to sniff the raw value and call other DateTime.Parse overloads, e.g. forcing UTC
        if (DateTime.TryParse(value, culture, _supportedStyles, out DateTime dateTimeVale))
        {
            bindingContext.Result = ModelBindingResult.Success(dateTimeVale);
        }
        else
        {
            string msj = metadata.ModelBindingMessageProvider.ValueIsInvalidAccessor(valueProviderResult.ToString());
            modelState.TryAddModelError(modelName, msj);
        }
    }
    else
    {
        // unreachable
        throw new NotSupportedException();
    }

    return Task.CompletedTask;
}

It uses DateTime.TryParse to avoid the exception and try / catch when there is a format error. 🤓🤙
Another useful flag for SupportedStyles is DateTimeStyles.AssumeUniversal.