jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

`@JsonIgnoreProperties(ignoreUnknown=false)` can not override `DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES=false`

Open noschang opened this issue 3 years ago • 10 comments

Description

I'm not sure if this is a bug or a desired behavior, but in my opinion it's a bug. Here's what happens:

When you set DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES to false in an object mapper, the observed behavior is the expected, no exceptions are thrown, even if the JSON contains unknown properties.

However, if in combination with this config you annotate a bean with @JsonIgnoreProperties(ignoreUnknown = false), the behavior should change (in my opinion), since the annotation takes precedence over the configuration in the object mapper.

The annotation clearly states that the unknown properties should not be ignored, so, the expected behavior is that an excpetion is thrown.

I've searched the repository issues, but couldn't find this problem reported anywhere. Maybe, the only issue related to this is #1523, but it needs further investigation to confirm.

Version information

Tested with jackson-databind 2.12.1

To Reproduce

It's kinda hard to explain here how to reproduce. So, I created a project with some test that you can run to verify the behavior: JacksonBugTest.zip

Import the project on your IDE (Gradle support/plugins must be enabled) or run the tests with:

Linux: ./gradlew test -i Windows: gradlew.bat test -i

Alternatively, you can view the report generated by my test run: test-report.zip

Feel free to contact me if you have any doubts: [email protected] Thank you for your attention and congratulations for the excellent work you have been doing for the community!

noschang avatar Feb 23 '21 04:02 noschang

From the description it does sound like your assumption is correct: more specific annotation should override more general default setting. I hope to look into this in near future; thank you for providing a reproduction as well!

cowtowncoder avatar Feb 23 '21 20:02 cowtowncoder

Ohhhh. This might be much much more difficult to do anything about, and I think I remember now why.

The problem is due to use of simple boolean for JsonIgnoreProperties.ignoreUnknown, and enum requirement to always specify a default. In this case default would be false.

But one common usage is one where properties are explicitly named, something like

@JsonIgnoreProperties({ "password" })

which is effectively same as

@JsonIgnoreProperties(value = { "password" }, ignoreUnknown = false)

because there is no way to indicate value of "use default" or "not specified".

With current default settings (of global ignoreUnknown = false, effectively), many users will explicitly change this with FAIL_ON_UNKNOWN_PROPERTIES = false.

If we always used settings from @JsonIgnoreProperties, it is likely that usage by some users would break.

So the way annotation currently works is that you can only effectively override false with true; but not the reverse.

A solution to this would be use of OptBoolean as type for ignoreUnknown -- this Enum type was added in jackson-annotations 2.6 to cover such use cases. It can indicate "true/false/unspecified" trio. But I cannot think of a practical way to really change this for Jackson 2.x. And in fact even with 3.0, backwards-compatibility will come into play...

I would be open to solutions here but I suspect they would require addition of a tri-state setting; new property either for JsonIgnoreProperties or something else.

cowtowncoder avatar Feb 25 '21 00:02 cowtowncoder

@cowtowncoder, let me see if I understood correcty. You're saying that you check if ignoreUnknown is true or false.

If it's true, you assume that the user has intentionally set this flag with the @JsonIgnoreProperties annotation and thus override the default behavior of FAIL_ON_UNKNOWN_PROPERTIES

But if the value is false, you have three possible scenarios:

  1. The user didn't annotate the class with @JsonIgnoreProperties
  2. The user annotated the class with with @JsonIgnoreProperties, but didn't set the ignoreUnknown flag
  3. The user annotated the class with with @JsonIgnoreProperties and intentionally set the ignoreUnknown flag to false

In the first two scenarios the flag is falsebecause this is the default value for the booleanprimitive. On the third scenario, the value is falsebecause the user set it to falseintentionally

However, it's impossible to know witch of the 3 cases happened, so it's "dangerous" to infer one of the possibilities and change the default behavior

Did I got it right?

noschang avatar Feb 25 '21 00:02 noschang

Very close, yes, and explains the problem from my perspective. Minor pedantic differences would be

  • Whether user configured FAIL_ON_UNKNOWN_PROPERTIES is not tracked (but default is true). I have toyed with the idea of tracking of explicit change (to distinguish "default" from false/true), would be relatively easy to add but would not solve the whole issue here
  • ignoreUnknown defaults to false since it is indicated as the default value (annotation properties can only be optional with explicitly defined default value; otherwise they are mandatory).

Or another way to summarize this is that settings can only enable loose handling (currently), but not disable it.

cowtowncoder avatar Feb 25 '21 03:02 cowtowncoder

OK @cowtowncoder. How difficult would it be to change the annotation flag to an enum?

noschang avatar Feb 25 '21 18:02 noschang

@noschang for backwards compatibility reasons it cannot be changed for 2.x; the usual mechanism would be to add a replacement with higher precedence, deprecate old flag, remove old flag from 3.0. This might not be super difficult if we were to add OptBoolean valued alternative, with default of OptBoolean.DEFAULT (not set). Some of merging logic could be hidden in JsonIgnoreProperties.Value. With that, question would be that of name to use; or if it'd be better to use different annotation -- @JsonFormat.Feature could theoretically be used too (but may be less convenient).

Now. On name to use. I wonder if adding

@JsonIgnoreProperties(acceptUnknown = OptBoolean.TRUE)

would make sense? Or failOnUnknown or... ?

cowtowncoder avatar Feb 25 '21 18:02 cowtowncoder

Hi @cowtowncoder.

I understand that changing this on 2.x would break many things. So, for a temporary solution, what about a new annotation? I thought about this:

@JsonConfigOverride(feature = DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, active = true)
public class MyBean { 
... 
}

Using this annotation would have the same effect that calling objectMapper.config(...) prior to the object serialization/deserialization. Maybe it's not the best solution, but can help to solve the problem. Also it would allow to change other settings in a per object basis.

Also, I don't know if anyone else has this problem. I'm developing an API with Spring Boot and I configured the object mapper to ignore the unknown properties for all the endpoints setting DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES to false during the Spring startup. However, there is one specific endpoint of the API where I need strict JSON validation, thus the need of the ignoreUnkown flag.

If I had control over the object mapper in this specific endpoint, I could call the configure() method on it before serializing. But the object being serialized is a @RequestBody and Spring does this under the hood. Even If I could inject the object mapper and call the configure() method on it, I assume Spring uses the same instance for all the application. So, if I called the method in this endpoint I would be changing the object mapper behavior for all the application, which is not intended.

noschang avatar Mar 01 '21 12:03 noschang

upvote

sohrabsaran avatar Nov 16 '21 16:11 sohrabsaran

Anyone has a workaround to this problem? I am facing the same issue.

StefanBratanov avatar Aug 04 '22 19:08 StefanBratanov

@StefanBratanov You can write a custom AnnotationIntrospector to override behavior to return explicit false for @JsonIgnoreProperties, if you want. JacksonAnnotationIntrospector shows which method needs override (you can also sub-class that implementation).

cowtowncoder avatar Aug 05 '22 01:08 cowtowncoder