Search before asking

  • [x] I searched in the issues and found nothing similar.

Describe the bug

Our unit tests caught regression in Jackson behaviour when upgrading to 2.18.3. We used ConstructorDetector.USE_PROPERTIES_BASED to prevent cases where json string can be used instead of proper json object when class contains only one string field. After migration to 2.18.3 our tests started to fail as ConstructorDetector.USE_PROPERTIES_BASED stopped protecting from this behaviour

Version Information

2.18.3

Reproduction

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.cfg.ConstructorDetector;
import com.fasterxml.jackson.databind.json.JsonMapper;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertThrows;

public class JacksonSerializationTest {

    // succeeds with 2.12.7
    // also succeeds with 2.17.0, 2.17.2, 2.18.0, 2.18.1, 2.18.2
    // fails with 2.18.3+
    @Test
    public void testSerialization() {
        String json = "{\"someFiled\":\"imSomeStringVal\"}";

        ObjectMapper objectMapper = JsonMapper.builder()
                .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
                .build();

        // no exception starting from 2.18.3
        assertThrows(JsonMappingException.class, () -> {
            objectMapper.readValue(json, WrapperClass.class);
        });
    }

    public static class WrapperClass {
        @JsonProperty("someFiled")
        private final SingleStringClass someFiled;

        private WrapperClass() {
            someFiled = null;
        }

        public WrapperClass(SingleStringClass someFiled) {
            this.someFiled = someFiled;
        }
    }

    public static class SingleStringClass {
        @JsonProperty("someStringVal")
        private final String someStringVal;

        private SingleStringClass() {
            someStringVal = null;
        }

        public SingleStringClass(String someStringVal) {
            this.someStringVal = someStringVal;
        }
    }
}

Expected behavior

No response

Additional context

No response

Comment From: pjfanning

Maybe related to https://github.com/FasterXML/jackson-databind/issues/4860 - a fix made for 2.18.3

Comment From: Saljack

I can also confirm it. It works in 2.17.2. 2.18.0 - 2.18.2 have the issue described in #4860 with multiple constructors. The multiple constructor issue was fixed in 2.18.3, but the fix enabled one string construction for ConstructorDetector.USE_PROPERTIES_BASED.

Comment From: JooHyukKim

Filed failing test for https://github.com/FasterXML/jackson-databind/pull/5213. Feels like almost as if this issue is conflicting with #4860.

Comment From: Saljack

Maybe I did some confusion in the #4860 where I mentioned that a string "something" is not also deserialized but it was just an example that nothing worked and I tried to explain it here https://github.com/FasterXML/jackson-databind/issues/4860#issuecomment-2557626211

Here is a simpler example without any wrapper object with just one parameter string constructor.

@Test
public void testDeserialization()  throws Exception{
  String json = "\"invalid value\"";

  JsonMapper objectMapper = JsonMapper.builder()
          .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
          .build();

    // no exception starting from 2.18.3
    assertThrows(Exception.class, () -> {
        objectMapper.readValue(json, StringConstructorClass.class);
    });
}

public record StringConstructorRecord(String x) {
  public StringConstructorRecord() {
    this(null);
  }
}

public static class StringConstructorClass {
  private String x;

  public StringConstructorClass() {
  }

  public StringConstructorClass(String x) {
    this.x = x;
  }

  public String getX() {
    return x;
  }

  public void setX(String x) {
    this.x = x;
  }

}

I would expect that if ConstructorDetector.USE_PROPERTIES_BASED is set, then deserializeFromString never would be used for objects. The funny thing is that it works with records:

@Test
public void testDesserializationRecord()  throws Exception{
  String json = "\"invalid value\"";

  JsonMapper objectMapper = JsonMapper.builder()
          .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
          .build();

    // Works as expected with record
    assertThrows(Exception.class, () -> {
        objectMapper.readValue(json, StringConstructorRecord.class);
    });
}

public record StringConstructorRecord(String x) {
  public StringConstructorRecord() {
    this(null);
  }
}

But if a record has a custom single string constructor then it does not work

@Test
public void testDeserializationCustomStringConstructorRecord()  throws Exception{
  String json = "\"invalid value\"";

  JsonMapper objectMapper = JsonMapper.builder()
          .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
          .build();

    // no exception starting from 2.18.3
    assertThrows(Exception.class, () -> {
        objectMapper.readValue(json, CustomStringConstructorRecord.class);
    });
}

public record CustomStringConstructorRecord(String x, String y) {

  public CustomStringConstructorRecord() {
    this(null, null);
  }

  public CustomStringConstructorRecord(String x) {
    this(x, null);
  }

}

Comment From: JooHyukKim

@Saljack I ran tests locally and the two test cases behaves the same both with/without ConstructorDetector.USE_PROPERTIES_BASED?

Comment From: cowtowncoder

The problem here is, I think, that Constructor detection is using legacy single-arg constructor discovery as no @JsonCreator annotation is used. This only works for a small set of types, but that includes String.

I would suggest trying:

  @JsonCreator
  public StringConstructorClass(String x) {

which I think would then use explicit Creator path. Although TBH, if you add that, might as well use

@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)

to make ConstructorDetector unnecessary.

Comment From: cowtowncoder

Oh. Also: auto-detection of that 1-arg Constructor does not (and is not supposed to) work because:

  1. Constructor parameter names only available if ParameterNamesModule registered. Test does not show it (but maybe is supposed to)
  2. There is another constructor: default (no args one). Auto-detection works with one-and-only-one-constructor case

so I don't see how this -- as presented -- would have worked on 2.18.2 or previous.. :-/