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

Set Jackson FAIL_ON_UNKNOWN_PROPERTIES property to false by default [SPR-11891] #16510

Closed
spring-issuemaster opened this issue Jun 20, 2014 · 6 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jun 20, 2014

Oliver Drotbohm opened SPR-11891 and commented

Jackson's ObjectMapper has a weird default setting to completely fail the deserialization if a single unknown property is encountered. This violates Postel's law as the server could still successfully create an instance of the expected type. Actually this kind of resilient behavior is why people might choose JSON over the rather schema dominated XML world.

I've filed a ticket for that issue in Jackson itself but I thought it might be worth taking the lead here. I know changing defaults is kind of a risky thing to do but as Brian Clozel indicated in the comments of said ticket, it would actually increase resilience of already existing code.


Affects: 4.0.5

Issue Links:

  • #16857 Create builder for Jackson ObjectMapper ("depends on")
  • #19575 @JsonIgnoreProperties(ignoreUnknown=false) is not working in Spring 4.2.8 and upper version
  • #16793 Set Jackson DEFAULT_VIEW_INCLUSION property to false by default

Referenced from: commits 42aef5f

1 votes, 6 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 9, 2014

Rossen Stoyanchev commented

I agree that would be a better default. That said while it may not break existing code it could change expectations. In other words I do think there may be legitimate cases for the current default and applications relying on it.

A more subtle assumption is that currently MappingJackson2HttpMessageConverter uses a stock ObjectMapper or you can pass your own pre-configured instance. This would change that assumption since now we give an ObjectMapper configured the way we think it should be. In short I don't think the converter is a good place to make this change. If I was relying on the internal ObjectMapper and then decided to configure my own for unrelated reasons, suddenly I'd "lose" the good default(s) we provide unless I happened to notice them in the source and match them in my own ObjectMapper.

The Jackson2ObjectMapperFactoryBean where we help initialize the ObjectMapper seems a better fit for doing this since at least that default will "remain" with you for as long as you use the FactoryBean or until it's changed explicitly. I'd be okay with changing it there but otherwise propose leaving the converter as is.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 10, 2014

Brian Clozel commented

Even though I think this configuration should be the default one in Jackson, it's true spring developers rely on the fact that our ObjectMapper configuration is the "Jackson stock configuration" and we should not mess with it.

I've created a Spring Boot issue - let's see what they think about it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 10, 2014

Phil Webb commented

My vote would be to change the default in Boot but not in Spring Framework.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 11, 2014

Stéphane Nicoll commented

I second that.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 15, 2014

Rossen Stoyanchev commented

Closing in favor of the ticket opened in Boot.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 24, 2014

Sébastien Deleuze commented

Since #16793 will provide a way to modify default ObjectMapper properties thanks to ObjectMapper builder + Jackson2ObjectMapperFactoryBean, I reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.