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

Change of behavior (2.8 -> 2.9) with ObjectMapper.readTree(input) with no content #2211

Closed
cowtowncoder opened this issue Dec 18, 2018 · 1 comment
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 18, 2018

So, it looks like readTree() methods in ObjectMapper, ObjectReader that take input OTHER than JsonParser, and are given "empty input" (only white-space available before end), will

  • Return NullNode (Jackson 2.x up to and including 2.8)
  • Return null (Jackson 2.9)

Latter behavior is what readTree(JsonParser) has and will do; but this accidentally changed other methods due to refactoring that unified underlying call handling (and add checking for new DeserializationFeature.FAIL_ON_TRAILING_TOKENS).
Behavior for this edge case was not being tested, apparently.

Now: since behavior has been changed for all 2.9.x patch versions, I am not sure it should be changed for 2.9 branch. But it seems sub-optimal as behavior, and something to definitely change for 3.0... but probably also for 2.10.

There are multiple things we could do.

  1. Change it back to 2.8, to return NullNode
  2. Change to throw exception, as "not valid" use case
  3. Change it to return MissingNode
  4. Leave as-is, for rest of 2.x.

Although it might seem best to revert it to (1), that seems somewhat wrong, problematic, as it would now not be possible to distinguish between JSON null value and missing content.
And although (2) would probably make sense, if designing API from scratch, it is probably too intrusive.

So I think (3) is the best way: it avoids returning null or throwing Exception (both being likely to break 2.9 code), but still allows distinguishing between all possible input cases.

@cowtowncoder cowtowncoder changed the title Change of behavior (2.8 -> 2.9) with ObjectMappe.readTree(input) with no content Change of behavior (2.8 -> 2.9) with ObjectMapper.readTree(input) with no content Dec 20, 2018
cowtowncoder added a commit that referenced this issue Jan 20, 2019
@cowtowncoder
Copy link
Member Author

cowtowncoder commented Jan 23, 2019

Note: once upon a time there was #1406, which actually makes this intentional but misguided change.

cowtowncoder added a commit that referenced this issue Jan 23, 2019
cowtowncoder added a commit that referenced this issue Jan 23, 2019
@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Jan 24, 2019
mpfz0r added a commit to Graylog2/graylog2-server that referenced this issue Aug 19, 2019
…values

FasterXML/jackson-databind#2211
- In Jackson <2.9.0 an IOException was raised
- In Jackson >=2.9.0, null is returned
bernd pushed a commit to Graylog2/graylog2-server that referenced this issue Aug 26, 2019
* Allow trailing comma in Gelf JSON parser

* Update jackson to 2.9.9

* Keep exsiting behavior when using objectMapper.readTree() with empty values

FasterXML/jackson-databind#2211
- In Jackson <2.9.0 an IOException was raised
- In Jackson >=2.9.0, null is returned
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Sep 20, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Sep 24, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Sep 26, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Sep 27, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Sep 27, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Oct 3, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Oct 17, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Nov 11, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Nov 11, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Nov 11, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Nov 11, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
PascalSchumacher added a commit to PascalSchumacher/flowable-engine that referenced this issue Nov 12, 2019
…response with no content.

Jackson 2.10 changed the behavior for ObjectMapper.readTree(input) for no content (see: FasterXML/jackson-databind#2211).
@cowtowncoder cowtowncoder removed the 2.10 label Apr 12, 2020
shaikzakiriitm added a commit to shaikzakiriitm/kafka that referenced this issue Sep 18, 2020
…NG JsonNodeType in JsonConverter.

From 2.10.0 onwards, in jackson lib, ObjectMapper.readTree(input) started to return JsonNode of type MISSING for empty input, as mentioned in the issue: FasterXML/jackson-databind#2211.
Made changes in JsonParser to incorporate this, and treat MISSING JsonNodeType in a similar fashion as that of NULL JsonNodeType.
Added a unit test for this.
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit to apache/kafka that referenced this issue Oct 2, 2020
…ULL nodes (#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
cleiner added a commit to cleiner/dropwizard that referenced this issue Nov 1, 2020
Two of the tests were waiting for log output using a one-off call to
Thread.sleep(). In addition to being brittle, the tests would fail with
an NPE (while trying to assert on the "timestamp" field) that doesn't
point to the cause of the failure directly.

Also removed the redundant assertThat(jsonNode).isNotNull() as
objectMapper.readTree() will never return null in Jackson 2.10.x, see
FasterXML/jackson-databind#2211
joschi pushed a commit to dropwizard/dropwizard that referenced this issue Nov 2, 2020
Two of the tests were waiting for log output using a one-off call to `Thread.sleep()`.

In addition to being brittle, the tests would fail with an NPE (while trying to assert on the "timestamp" field) that doesn't point to the cause of the failure directly.

Also removed the redundant null-check as `objectMapper.readTree()` will never return `null` in Jackson 2.10.x, see FasterXML/jackson-databind#2211
joschi pushed a commit to dropwizard/dropwizard that referenced this issue Nov 3, 2020
Two of the tests were waiting for log output using a one-off call to `Thread.sleep()`.

In addition to being brittle, the tests would fail with an NPE (while trying to assert on the "timestamp" field) that doesn't point to the cause of the failure directly.

Also removed the redundant null-check as `objectMapper.readTree()` will never return `null` in Jackson 2.10.x, see FasterXML/jackson-databind#2211

(cherry picked from commit bdb829c)
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

1 participant