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
ASP.NET Core model binder deserilises UTC time string to local time rather than UTC time #11584
Comments
Some repro code: [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
Same result is returned from same request when
As can be seen, the model bound value is the same as the result of calling Registering a custom model binder for 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 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:
|
Could please someone tells me where exactly datetime binding is processing? |
@DamianEdwards Thanks for this workaround! Upgrading my dotnet core from 2.x to 3.x broke all the dates and this fixed everything. |
@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. |
@alsami Yes, the work around is fine, thank you. But my questions still stands. |
Any updates on this? Will it be fixed in ASP.NET Core 3? |
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;
} |
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). |
This workaround is for query-parameters (probably also path variables). When you |
@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). 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); |
@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 .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 |
@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 |
@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? |
@razvangoga I hope this will be fixed in .NET 5. 😃 |
Great that it worked. I am sure there is some way to make it work with |
This problem did take me by surprise (I lost a couple of hours for it 😭). 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 |
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:
https://<something>.com/{action}/{id}?time=2019-06-14T02:30:04.0576719Z
time
parameter in the corresponding controller is a local timeExpected behavior
The retrieved
time
should be a utc time.The text was updated successfully, but these errors were encountered: