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

`@JsonUnwrapped` prevents checks for `DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES`

Open kulabun opened this issue 11 years ago • 6 comments

Deserializer didn't handle unknown properties if they was not mapped to Unwrapped object fields. Here is unit test:

    private static class A {
        @JsonUnwrapped
        public B b;
    }

    private static class B {
        public String field;
    }

    private final ObjectMapper MAPPER = new ObjectMapper();

    public void testFailOnUnknownPropertyUnwrapped() throws IOException {
        assertTrue(MAPPER.isEnabled(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES));

        final String JSON = "{'field': 'value', 'bad':'bad value'}";
        try {
            MAPPER.readValue(aposToQuotes(JSON), A.class);
        } catch (JsonMappingException e) {
            // it's ok
            return;
        }
        fail("Exception was not thrown on unkown property");
    }

kulabun avatar Dec 12 '14 08:12 kulabun

This is sort of known, and due to implementation, where it is rather difficult to determine if property is or is not known. But obviously it'd be good to catch, so I can add the test under failing in case someone figures out a way to properly support strict checks.

Thanks!

cowtowncoder avatar Dec 12 '14 17:12 cowtowncoder

I have this ussue with com.fasterxml.jackson.core:jackson-databind:2.6.1

sgri avatar Dec 12 '17 13:12 sgri

@sgri this is an open issue (or, "feature"), exists in all versions. Will not be resolved before Jackson 3.0 since there isn't enough information available to do checks at level outside unwrapped properties.

cowtowncoder avatar Dec 12 '17 23:12 cowtowncoder

I wanted to comment on this issue since this has been bugging me for a while. I was able to make a hacky solution for this by creating a custom deserializer which would call Parser#readValueAsTree() and cast to an ObjectNode. It would then use an ObjectMapper with DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES disabled and read however many "unwrapped" objects from the parsed ObjectNode. For each of these objects it would serialize them, and check for any fields on the original ObjectNode and would throw an exception if there were extra fields. Now, this solution isn't perfect because it uses an outside ObjectMapper and because it requires that the objects can be serialized as well as deserialized the same way, but it does work.

Now I was digging through code and I'm in no position to try and go about figuring this out without investing a huge chunk of time, but I was wondering if when an unwrapped object was being deserialized, instead of deserializing it with "fail on unknown" off, could it be serialized with it on, then when an unknown property is come across, it would be handled by a DeserializerProblemHandler that's somehow injected into where unwrapped objects are deserialized. The handleUnknownProperty is defined on this DeserializerProblemHandler and will always return true. It will also pass back a list of unknown property names to where the object holding unwrapped objects is being deserialized. Once the deserializion process is almost over, it will compare all the unknown properties for each unwrapped object present. If a specific "unknown" property is included in all of the unwrapped objects, then that property was not used and the deserialization should fail.

@cowtowncoder does my above proposal seem possible, or is there something I'm missing that will make what I said above really difficult?

retrodaredevil avatar Mar 08 '21 01:03 retrodaredevil

The main problem right now is that there simply is no knowledge of what "unknown" property might be, given the way deserialization of Unwrapped values is handled -- contents of the "main" level are known and passed, but anything potentially unknown may as well be needed by one of Unwrapped values.

So part of the problem is to define a way for Unwrapped deserializers/handlers to be able to indicate whether they would be interested in specifically named key (of value). If so, buffering would be more efficient, and handling of unknown properties could be done the way you suggest -- yes, DeserializationProblemHandler should be invoked and so on.

As to why knowledge of known properties is challenging; beyond regular properties there are:

  1. "Any" properties (which are not enumerated)
  2. Possible "unwrapped in unwrapped" (so it needs to be recursive)
  3. Object and Type Ids

On existing solution I would just point out that in addition to adding overhead, serialization is not necessarily symmetric with deserialization, so detection that way might fail for some of more complicated cases.

I don't have a solution for this aspect, although the problem to solve is definitely on "to figure out for 3.0" list.

cowtowncoder avatar Mar 08 '21 19:03 cowtowncoder

NOTE: was not "figure out" for 3.0, still an architectural challenge.

cowtowncoder avatar Nov 17 '25 00:11 cowtowncoder

Phew! Finally fixed thanks to @JacksonJang's contribution (#5504)! Will be in 3.1.0.

cowtowncoder avatar Dec 20 '25 04:12 cowtowncoder