[Bug] Jackson attempts to parse injected classes as if it were deserializing them.

[solonovamax]

Bug Report

This bug took me a while to track down, but after misunderstanding what the bug actually was like 6 different times, I eventually figured it out.

Description

When using Jackson to inject values with @JacksonInject, Jackson will attempt to parse the injected class as if it were a bean. It will search for setters and getters, even though it never actually uses the injected class as a bean.

I'm just going to explain that again, because it took be a while to grasp that:

• Let's say you are parsing a class, say class Parsed.
• And let's say you're injecting a class, say class Injected into class Parsed in the constructor.
  So Parsed's constructor would look like this:

  Parsed(@JacksonInject Injected injectMe) {
      this.injectMe = injectMe
  }

  Notes:
  • You can use getters and setters or public properties here. I chose a constructor parameter because it's easy. Though, when using Kotlin, I could only reproduce it when Injected was a constructor parameter but not with getters/setters or public properties. But that's probably Kotlin doing something fancy.
  • If you use getters/setters or public properties and annotate them with @JsonIgnore, then this exception also won't occur.
• Jackson will now attempt to recursively process the class Injected for any getters/setters.
• So, if Injected has two methods that Jackson would interpret as having "conflicting setter definitions", then it will throw an exception.
  Even though it never actually injects any json into the parsed Injected class.
  Here are two methods which would trigger an exception:

  private void setAbcd(Void a) {}
  
  private void setAbcd(Boolean a) {}

• Additionally, because it's parsing it recursively, those conflicting setters don't need to be in the class Injected.
  They can also be in an exposed property of Injected.

Version Information

This occurs on Jackson version 2.12.3, pulled from Maven Central.

To Reproduce

Steps to reproduce the behaviour:

Here is a test class for the bug: (Yeah, I know it's big. I wrote a bunch of comments because I found it kind of hard to initially understand what was going on)

public class CursedInjectionExceptionTest {
    @Test
    public void testInjectionBlackMagic() throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();
        
        Injected one = new Injected(); // instantiate new TestClassOne to be used for injection
        mapper.setInjectableValues(new InjectableValues.Std().addValue("TeStInG", one)); // setup injection
        
        Parsed two1 = new Parsed(one); // instantiate new TestClassTwo
        two1.someTestValue = "abcd"; // just setting it to some random value so it's not `null`
        
        String twoAsString = mapper.writeValueAsString(two1); // serialize two to a string.
        // twoAsString should just be '{"someTestValue":"abcd"}'
        
        Parsed two2 = mapper.readValue(twoAsString, Parsed.class); // test fails here
    }
    
    /**
     * For some reason Jackson interprets this class as a bean.
     * <p>
     * Normally, this wouldn't be much of an issue (other than perf sensitive applications with large injected classes)
     * But, if the injected class has any conflicting setters/getters,
     * or if any of its properties have conflicting getters/setters,
     * then it will throw an exception.
    */
    static class Injected {
        // some random property that fits *very* specific criteria.
        public ClassThatBreaks classThatBreaks = new ClassThatBreaks(); // only public to avoid setters & getters (because I'm lazy)
        
        @Override
        public String toString() {
            return "TestClassOne{}";
        }
    }
    
    
    static class Parsed {
        private final Injected injected;
        public        String   someTestValue; // only public to avoid setters & getters (because I'm lazy)
        
        Parsed(@JacksonInject("TeStInG") Injected injected) {
            this.injected = injected;
        }
        
        @Override
        public String toString() {
            return "TestClassTwo{" + "testClassOne=" + injected + ", someTestValue=" + someTestValue + '}';
        }
    }
    
    
    /**
     * The class used to break Jackson must fit a specific criteria:
     * It must have two overloaded methods beginning with "{@code set}", followed by whatever you want.
     * Both of those methods must take <i>exactly</i> one parameter.
     * If you use 2+ parameters or no parameters, then it will not break.
     * Basically, Jackson needs to interpret those as two "setters" and must determine that they are overlapping.
     * <p>
     * Also, I'm not sure why, but only <i>some</i> parameter types break with this.
     * <p>
     * {@link String} is the only class I've found so far that <i>doesn't</i> break with this.
     * (Meaning: if you use "String" as one of the {@link #setAbcd} types, then it will not work.
     * Honestly, I think {@code String} just has some weird special treatment in Jackson.)
     * <p>
     * Here's a list of other classes that I've tried and <i>do</i> break with it.
     * <ul>
     *     <li>{@link Injected}</li>
     *     <li>{@link Parsed}</li>
     *     <li>{@link Integer}</li>
     *     <li>{@link Boolean}</li>
     *     <li>{@link List}</li>
     *     <li>{@link Void}</li>
     * </ul>
     * <p>
     * One last thing, I promise.
     * As I said earlier, these methods can also be moved into {@link Injected} and the error will still occur.
     */
    static class ClassThatBreaks {
        private void setAbcd(Void a) {}
        
        private void setAbcd(Boolean a) {}
    }
}

Expected behavior

When injecting Injected into Parsed, Jackson should not attempt to process ClassThatBreaks for setters/getters.

The way I originally found this issue was by injecting a class which contained a reference to an ObjectMapper,
and it was failing because of two clashing definitions of ObjectMapper#setDefaultPropertyInclusion.

Additional Context

This may be similar to #2465, but I'm not entirely sure.
I do believe that this is something different, though.

[solonovamax]

Actually, when attempting to use the solution presented in this comment here on 2465, it succeeded in fixing the issue.

I'm going to close this issue now because I'm like 99% sure that it's the same issue.

[cowtowncoder]

@solonovamax yes, was about to suggests that: (a) yes, you are correct in that in some cases introspection may fail for no need and (b) there is an existing report.

[solonovamax]

Yeah, I found the report but the setup differed enough that I thought this might be different.
So monkey brain go ooga booga and I spent 3 hours bashing my head against a wall trying to figure out what the issue was, when I could have just used the solution all along.

[cowtowncoder]

Yes, the tricky thing is that since @JacksonInject valued properties generally may also come from input (unless specifically configured NOT to do so), many code paths may trigger the issue. So in a way multiple test cases may show different paths. Solution, likewise, is not simple since sometimes triggering of lookup is a side-effect.

One possible workaround really is to register bogus deserializer, either by direct registration or mix-in annotations.

[solonovamax]

Perhaps instead of having it automatically attempt to parse a deserializer, you need to explicitly tell jackson that the injected value comes from input. (So basically the inverse of the current state. Currently useInput defaults to OptBoolean.TRUE. I think useInput should instead default to OptBoolean.FALSE.)

Also, the documentation on @JacksonInject should def be improved. Because looking back on it now, I can kind of see what it means, but at the time, that didn't pop out to me at all.

[cowtowncoder]

Yes that is an option, but given that it directly changes existing semantics that would need to go in the next big major version, Jackson 3.0.

As to Javadoc improvements, those are always welcome!