Skip to content
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

ObjectMapper.valueToTree() Fails When DeserializationFeature.FAIL_ON_TRAILING_TOKENS Is Enabled #3308

Closed
raphaelNguyen opened this issue Oct 25, 2021 · 5 comments
Milestone

Comments

@raphaelNguyen
Copy link
Contributor

raphaelNguyen commented Oct 25, 2021

Describe the bug
ObjectMapper.valueToTree() throws the following error when being called on an ObjectMapper instance made with DeserializationFeature.FAIL_ON_TRAILING_TOKENS feature enabled.

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Trailing token (of type START_OBJECT) found after value (bound as com.fasterxml.jackson.databind.JsonNode): not allowed as per DeserializationFeature.FAIL_ON_TRAILING_TOKENS
at [Source: UNKNOWN; byte offset: #UNKNOWN]
at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
at com.fasterxml.jackson.databind.DeserializationContext.reportTrailingTokens(DeserializationContext.java:1814)
at com.fasterxml.jackson.databind.ObjectMapper._verifyNoTrailingTokens(ObjectMapper.java:4783)
at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4656)
at com.fasterxml.jackson.databind.ObjectMapper.readTree(ObjectMapper.java:2941)
at com.fasterxml.jackson.databind.ObjectMapper.valueToTree(ObjectMapper.java:3392)

Version information
2.13.0

To Reproduce
Create an ObjectMapper with the fail on trailing tokens option enabled and call valueToTree() using that mapper.
ObjectMapper mapper = JsonMapper.builder().enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).build;

Expected behavior
ObjectMapper should return a JsonNode representation of the object without any error.

Additional context
From what I can see debugging the method valueToTree(). It seems that the method works by serializing the java Object into a TokenBuffer before deserializing that buffer back into JsonNode.

However, as can be seen in the snippet referenced below, a recent change (2e986df) has added a line to serialize the java Object into the buffer (context.serializeValue(buf, fromValue)) without deleting the old code that does the same thing (writeValue(buf, fromValue)). This cause the buffer to contain 2 copies of the deserialized object instead of one.

The subsequent readTree() will serialize the 1st copy of the object out and judge the remaining 2nd copy of the serialized object to be trailing garbage token and failed according to the DeserializationFeature.FAIL_ON_TRAILING_TOKENS feature.

It seems to me that the fix is to simply remove the redundant call to writeValue in this method. Please advise if this is indeed a problem or I've made a mistake somewhere.

try {
context.serializeValue(buf, fromValue);
writeValue(buf, fromValue);
try (JsonParser p = buf.asParser()) {
return readTree(p);
}

@raphaelNguyen raphaelNguyen added the to-evaluate Issue that has been received but not yet evaluated label Oct 25, 2021
@cowtowncoder cowtowncoder added 2.13 and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 29, 2021
@cowtowncoder cowtowncoder added this to the 2.13.1 milestone Oct 29, 2021
cowtowncoder added a commit that referenced this issue Oct 29, 2021
@raphaelNguyen
Copy link
Contributor Author

@cowtowncoder Glad to be of help.

@cowtowncoder
Copy link
Member

Fix will be included in 2.13.1 -- was regression it seems, should not occurs with 2.12 or earlier.

@mosesn
Copy link

mosesn commented Nov 8, 2021

@cowtowncoder would you mind cutting a release of this? we just upgraded to 2.13.0 but might have to roll back because we can't take the performance hit of serializing twice.

@cowtowncoder
Copy link
Member

At some point, yes, it's always a balance between my spending 2-3 hours cutting the release -- no, it's not a trivial thing to do unfortunately, as every repo must be released for full patch set -- and timely delivery of fixes. At this point the job that pays for bills (no real revenue from my OSS work) is busy enough that I don't have much extra time so it'll have to be one of weekends.

I probably won't have time to do 2.13.1 release this week or next, but will try to get it out by November.

@mosesn
Copy link

mosesn commented Nov 9, 2021

That makes sense, thanks for being clear about it!

finaglehelper pushed a commit to twitter/util that referenced this issue Nov 9, 2021
Problem

2.13.0 introduced a regression, where they were double-serializing.  We're
worried that this will lead to a substantial performance regression. Github
ticket: FasterXML/jackson-databind#3308

Solution

This reverts commit e27353b784543196b9f5fd572110e4b4fe04f4c4.

JIRA Issues: CSL-11459

Differential Revision: https://phabricator.twitter.biz/D780287
finaglehelper pushed a commit to twitter/finagle that referenced this issue Nov 9, 2021
Problem

2.13.0 introduced a regression, where they were double-serializing.  We're
worried that this will lead to a substantial performance regression. Github
ticket: FasterXML/jackson-databind#3308

Solution

This reverts commit e27353b784543196b9f5fd572110e4b4fe04f4c4.

JIRA Issues: CSL-11459

Differential Revision: https://phabricator.twitter.biz/D780287
finaglehelper pushed a commit to twitter/twitter-server that referenced this issue Nov 9, 2021
Problem

2.13.0 introduced a regression, where they were double-serializing.  We're
worried that this will lead to a substantial performance regression. Github
ticket: FasterXML/jackson-databind#3308

Solution

This reverts commit e27353b784543196b9f5fd572110e4b4fe04f4c4.

JIRA Issues: CSL-11459

Differential Revision: https://phabricator.twitter.biz/D780287
finaglehelper pushed a commit to twitter/finatra that referenced this issue Nov 9, 2021
Problem

2.13.0 introduced a regression, where they were double-serializing.  We're
worried that this will lead to a substantial performance regression. Github
ticket: FasterXML/jackson-databind#3308

Solution

This reverts commit e27353b784543196b9f5fd572110e4b4fe04f4c4.

JIRA Issues: CSL-11459

Differential Revision: https://phabricator.twitter.biz/D780287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants