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

WebApi 5: DefaultBodyModelValidator throws StackOverflowException when validating model with LocalTime #249

Closed
GoogleCodeExporter opened this issue Mar 15, 2015 · 8 comments
Labels
Milestone

Comments

@GoogleCodeExporter
Copy link

Hi!

I recently upgraded a WebApi project to Visual Studio 2013.

My JavaScript client App is posting a model with a LocalTime to my WebApi 
controller:

public class MyModel {
   public LocalTime Hora { get; set; }
}

POST http://localhost:49165/api/prueba HTTP/1.1
Content-Type: application/json; charset=utf-8
{
  "Hora": "15:15:00"
}

I have configured JsonSerializerSettings with the JsonNet helpers for Noda Time

serializerSettings.ConfigureForNodaTime(NodaTime.DateTimeZoneProviders.Tzdb);


JsonNet is still deserializing correctly the object graph, but as soon as the 
model validation baked in WebApi kicks in 
(DefaultBodyModelValidator.ValidateProperties &  
DefaultBodyModelValidator.ValidateNodeAndChildren), it recursively begins to 
validate the model:

LocalTime => LocalTime.LocalDateTime => LocalDateTime.TimeOfDay => 
LocalTime.LocalDateTime => LocalDateTime.TimeOfDay => ...

The DefaultBodyModelValidator is recursively traversing these properties and 
does not detect infinite recursión although it seems that it has logic to 
handle that situation:

https://github.com/mono/aspnetwebstack/blob/master/src/System.Web.Http/Validatio
n/DefaultBodyModelValidator.cs#L91-94


This didn't happen before upgrading to latest WebApi version and I really don't 
know if the problem is from NodaTime or WebApi.

Any ideas or workaround? Thanks!!!

Germán Fernández






Original issue reported on code.google.com by germanft...@gmail.com on 29 Oct 2013 at 11:20

@GoogleCodeExporter
Copy link
Author

As a workaround, I have turned off default model validation following this 
stackoverflow question:

http://stackoverflow.com/questions/16698770/disable-default-validation-in-asp-ne
t-webapi

Germán


Original comment by germanft...@gmail.com on 29 Oct 2013 at 2:59

@GoogleCodeExporter
Copy link
Author

Hmm. Unfortunately I know nothing about Web API validation. It does sound like 
a bug in the validator, but it would be nice to prove that outside Noda Time.

Are you actually running on Mono? The code you've shown looks like it's the 
Mono implementation - unless I've missed something - so perhaps the MS 
implementation handles recursion differently?

Original comment by jonathan.skeet on 29 Oct 2013 at 3:46

@GoogleCodeExporter
Copy link
Author

Sorry for the confusion. I'm using standard MS implementation. I googled for 
DefaultBodyModelValidator and the mono source code just appeared on top. 
Anyway, it appears to be the same source code.

http://aspnetwebstack.codeplex.com/SourceControl/latest#src/System.Web.Http/Vali
dation/DefaultBodyModelValidator.cs

I will try to step into webapi source code.




Germán

Original comment by germanft...@gmail.com on 29 Oct 2013 at 4:13

@GoogleCodeExporter
Copy link
Author

Original comment by malcolm.rowe on 16 Nov 2013 at 2:39

  • Changed state: NeedMoreInfo
  • Added labels: Milestone-unscheduled

@GoogleCodeExporter
Copy link
Author

I recently ran into this issue using WebAPI 2.1.  To get around it, I created a 
class that derived from DefaultBodyModelValidator and specifically excluded 
LocalDate, LocalTime, LocalDateTime, ZonedDateTime, and OffsetDateTime in the 
ShouldValidateType method.  This is the first time I've tried to use Noda Time 
with WebAPI, so unfortunately I can't speak to whether or not the problem 
existed in previous versions of WebAPI.  I'd be happy to provide additional 
information if it may be of use.

Ian FitzGerald

Original comment by ifitzger...@gmail.com on 10 Mar 2014 at 3:22

@GoogleCodeExporter
Copy link
Author

I suspect this is fixed in 2.0 - certainly LocalTime.LocalDateTime has gone as 
a property.

I *could* try to write a test to ensure that every value type property graph 
forms a tree... but I suspect it's overkill.

If anyone is able to easily verify that this is okay in 2.0, that'd be really 
handy :)

Original comment by jonathan.skeet on 2 Aug 2014 at 9:51

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Looks like this popped up again here:
http://stackoverflow.com/q/25167708/634824

Original comment by mj1856 on 7 Aug 2014 at 6:05

@kentpickard
Copy link

I am running into this issue as well.

In the model class that I'm trying to deserialize I have a property of type OffsetDateTime.

OffsetDateTime has a property called LocalDateTime of type LocalDateTime.

The LocalDateTime class has a property called TimeOfDay of type LocalTime.

The LocalTime class has a property named TimeOfDay property of type LocalTime.

So it tries to validate OffsetDateTime -> LocalDateTime -> TimeOfDay -> LocalDateTime -> TimeOfDay -> LocalDateTime and so on. I think it is this design that's causing infinite recursion while trying to validate a model binding.

I would imagine the same exact problem would exist with the regular DateTime class (DateTime -> Date -> DateTime) but there is probably some custom code under the covers somewhere in Web API to handle that scenario.

I'm not sure the best way to handle this or work around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants