Skip to content

Different behaviour of string value deserialization in jackson 2.17 and 2.18 #5008

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

Closed
1 task done
askoog opened this issue Mar 6, 2025 · 20 comments
Closed
1 task done
Labels
to-evaluate Issue that has been received but not yet evaluated

Comments

@askoog
Copy link

askoog commented Mar 6, 2025

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

Jackson 2.17 and 2.18 behaves differently when deserializing simple objects that wraps a String, given that

  • The class wrapping a String has a single String field and one constructor taking a String value where the name of the parameter matches the field in the class.
  • The code is compiled with the javac flag -parameters to ensure parameter name retention in compiled code
  • The jackson module ParameterNamesModule is registered as a module in the object mapper

In 2.17 a json document with a raw string value ("abc123") is parsed correctly, but in 2.18 this is throws an exception and the json must be formatted as a json object ({ "value": "abc123" }) instead (where value is the name of the field in the class). In 2.17 the json object form cannot be parsed.

This makes the migration from 2.17 to 2.18 harder since it might cause existing json documents to became unparseable.

Version Information

2.17.x and 2.18.

Reproduction

Here is an example repository that reproduces the problem: https://github.com/askoog/jackson-deserialization.

Run

mvn test -Djackson.version=2.17.2

and

mvn test -Djackson.version=2.18.2

For different results, where one test case succeeds and one fails in each run.

Expected behavior

No regression should occur when upgrading to 2.18.x, the old json format should still be parseable.

Additional context

Value class used:

public class SimpleValue {

	private final String value;

	public SimpleValue(String value) {
		this.value = value;
	}

	public String getValue() {
		return value;
	}
@askoog askoog added the to-evaluate Issue that has been received but not yet evaluated label Mar 6, 2025
@pjfanning
Copy link
Member

Can you try 2.18.3?

@pjfanning
Copy link
Member

If this ever worked then I think that was an accident.
objectMapper.readValue("\"abc123\"", SimpleValue.class)

This is not JSON.

@askoog
Copy link
Author

askoog commented Mar 6, 2025

Same problem in 2.18.3 I'm afraid

@pjfanning
Copy link
Member

From reading the README in https://github.com/askoog/jackson-deserialization, the actual JSON case now passes - so that seems like an improvement.

@askoog
Copy link
Author

askoog commented Mar 6, 2025

The example is to make it easier to grasp. If you have a more complex json, it would look like this in 2.17

{ "aWrappedObject": { "mySimpleValue" : "abc123" } } 

And in jackson 2.18 it must now be strucutred as

{ "aWrappedObject": { "mySimpleValue" : { "value": "abc123" } } } 

Both examples are valid json as far as I can see

@pjfanning
Copy link
Member

The correct way to support flattening classes that have single string values to be that string value is to use Jackson annotations.

I don't personally like adding annotations and prefer to use customised DTO classes that match the JSON format that I want - so I am not an expert on the annotations that Jackson supports.

Good overview:
https://www.baeldung.com/jackson-annotations

If this was my problem, I would possibly start with adding @JsonValue to the getValue method.
https://github.com/askoog/jackson-deserialization/blob/master/src/test/java/acme/test/SimpleValue.java#L11

Since you do not have a setter for the value field, you might need a @JsonCreator on the constructor.

I suggest trying a few annotation set ups to find one that works.

@askoog
Copy link
Author

askoog commented Mar 6, 2025

Yes, I'm not asking for how to fix the code. You could also add a valueOf method to make the tests pass. What I'm saying is that there is a regression when upgrading from 2.17 to 2.18 and parsing a valid json document suddenly fails, which I would say is a bug

@pjfanning
Copy link
Member

pjfanning commented Mar 6, 2025

My view is that 2.18 fixed a bug in 2.17. If you don't like 2.18, you can

  • Stick with 2.17
  • Use annotations to customise your classes
  • Fork Jackson

If enough people complain, we could add an optional feature to default the behaviour to 2.17 style. This is a lot of work so will probably only be done if there is a lot of complaints. The priority work in Jackson now is on 3.0.0.

@askoog
Copy link
Author

askoog commented Mar 6, 2025

Ok, thanks for your suggestions. So the support for stringCreator will be removed in 3.0.0 then?

See https://github.com/FasterXML/jackson-databind/blob/2.19/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java#L131

In 2.17 the CreatorCollector gets the stringCreator set, in 2.18 instead the propertyCreator is set:

'https://github.com/FasterXML/jackson-databind/blob/2.19/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java#L174

I don't agree that the way 2.17 works is a bug, instead it has default support for having a constructor taking a single String value without any need for extra annotations. This support has been removed in 2.18

@pjfanning
Copy link
Member

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.18 are the release notes for the record.

@pjfanning
Copy link
Member

There have been a lot of code changes in the last 6 months or more that may have affected this. One that relates to property creators is #4689.

Based on the release notes, it seems reasonably likely that this change was not deliberate because none of the issues listed and their descriptions seems to spell out a case like this. I may be overlooking something.

For me, I actually like the new behaviour so I would like a solution that supports makes both of the test cases in https://github.com/askoog/jackson-deserialization pass.

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 6, 2025

Could you please include POJO in question in-line, instead of (or in addition to) describing @askoog ? This is about binding JSON String into POJO, via -- I assume -- constructor of factory method, so the most important part is class definition.

EDIT: I added class def.

pjfanning added a commit to pjfanning/jackson-databind that referenced this issue Mar 6, 2025
pjfanning added a commit to pjfanning/jackson-databind that referenced this issue Mar 6, 2025
@pjfanning
Copy link
Member

I created #5010 - I can reproduce results in https://github.com/askoog/jackson-deserialization but my PR doesn't seem to reproduce it (yet).

@cowtowncoder
Copy link
Member

@askoog This is unfortunately very much ambiguous case and unfortunately 2.17 and previous selected "Delegating" case for Constructor. Auto-detection in 2.18 selects "Properties" approach because of existing of logical value property (from getter and cosntructor), hence heuristics selecting that.

There are multiple ways to annotate things differently, or possibly rename (f.ex, changing constructor parameter name).
But behavior of 2.18 itself won't be changed (it is regrettable 2.17 behavior was different).

Annotation to use would be

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

@pjfanning
Copy link
Member

By any chance, is it possible to register the ParameterNamesModule with delegating mode?

objectMapper.registerModule(new ParameterNamesModule(JsonCreator.Mode.DELEGATING));

@cowtowncoder
Copy link
Member

By any chance, is it possible to register the ParameterNamesModule with delegating mode?

No, ParameterNamesModule does not directly decide mode -- all it does is provide Implicit Name for Constructor/Factory method parameter names. It does not control choice.

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 6, 2025

There is configuration setting ConstructorDetector, which does allow specifying preference for this ambiguous case, fwtw -- there are tests (like src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetectorTest.java) that show how to use it.

There's also:

https://cowtowncoder.medium.com/jackson-2-12-most-wanted-3-5-246624e2d3d0

which gives better example.

It might solve @askoog 's problem if he always wants case like this to resolve to Delegating.

@askoog
Copy link
Author

askoog commented Mar 7, 2025

To give some context, we have roughly 500-ish systems with different kinds of json parsing (mostly rest-api:s) and we're in the process of upgrading these to a newer spring boot version (3.3 -> 3.4) which also updates jackson from 2.17 to 2.18. Any solutions that retain the old functionality would be greatly appreciated. As of now, we might end up with really detailed acceptance testing of each system to assure this doesn't break any existing functionality ...

I'll check if ConstructorDetector helps solving the problem

@cowtowncoder
Copy link
Member

@askoog Yes, this is unfortunate. The big challenge with auto-detection has been difference between intended/document and actual implementation, for the specific case of 1-argument Creator (constructor / static factory methods) which is fundamentally ambiguous. 2.18 attempted to resolve all known issues with property introspection. This lead to improved handling/combining of information from Constructors and other accessors (Fields, getter/setter methods), which in turn subtly changed actual heuristics (for this particular case, detection of implied "value" property -- and allowing Constructor to be auto-detected if (but only if!) there is just 1 declared visible (public) constructor).

So change b/w 2.17 and 2.18 was due to major rewrite of property introspection: and this was done without breaking any of existing unit tests. But the test suite did not cover all cases users had, obviously -- and in some cases behavior change, while fix, lead to incompatibilities.

Intent now, however, is to stabilize things to 2.18 behavior.

I hope this helps give some more context on why this unfortunate incompatibility occurred, and why it will not be reverted: this particular case should have worked the way it now does in 2.18, even in 2.17.

Having said that, wherever we can help with some options wrt backwards-compatibility, we should try to do so.
I hope ConstructorDetector can help here.

pjfanning added a commit to pjfanning/jackson-databind that referenced this issue Mar 7, 2025
@cowtowncoder
Copy link
Member

I will close this as "wont-fix" since, for better or worse, 2.18.x behavior is the way things are expected to work now. Use of @JsonCreator with mode or configuring ConstructorDetector is the way to force delegating handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

3 participants