Skip to content

Jackson update#2285

Open
Julius278 wants to merge 9 commits intoLFDT-web3j:mainfrom
Julius278:jackson-update
Open

Jackson update#2285
Julius278 wants to merge 9 commits intoLFDT-web3j:mainfrom
Julius278:jackson-update

Conversation

@Julius278
Copy link
Copy Markdown
Contributor

@Julius278 Julius278 commented Apr 28, 2026

What does this PR do?

update the jackson dependencies to new packages

Why is it needed?

vulnerabilities and new package structure

Checklist

  • I've read the contribution guidelines.
  • I've added tests (if applicable).
  • I've added a changelog entry if necessary.

Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
# Conflicts:
#	core/src/main/java/org/web3j/protocol/core/methods/response/EthLog.java
@Dev10-sys
Copy link
Copy Markdown
Contributor

hey @Julius278 ,thanks for putting in the effort for this Jackson upgrade. This is a big change
I just went through the PR in detail and overall the migration direction looks correct but there are a few important concerns we should address.

First, this upgrade moves to Jackson 3.x which requires Java 17 as a minimum. The current project still supports Java 11, and I don’t see any update to the build configuration or documentation reflecting this change. This would silently break users on older Java versions, so we should explicitly handle or at least discuss this before merging.

Second, Jackson 3 changes ObjectMapper behavior to be effectively immutable. If we are still using the old pattern with new ObjectMapper() followed by configure calls, those configurations may not actually take effect anymore. It would be good to double check ObjectMapper initialization (especially in WebSocketService and related areas) and confirm we are using the builder-based approach where required.

Third, I noticed convertValue() was replaced with treeToValue() in some places. These two are not equivalent. treeToValue() expects a JsonNode, so if the input is not guaranteed to be a JsonNode, this can lead to runtime issues. Can you confirm that the input type is always safe in those paths?

Also, this is a major dependency upgrade affecting core serialization so a changelog entry would be important for users.

Finally, this PR also includes some functional changes (like WebSocketService logic updates) along with the dependency migration. It might be safer to keep those separate to make review and rollback easier.
Let me know your thoughts.

Signed-off-by: Julius Lauterbach <git@juliuslauterbach.de>
@Julius278
Copy link
Copy Markdown
Contributor Author

Hey @Dev10-sys,

thank you very much for your detailed analysis.

The implied Java version upgrade is something I hadn't thought of. Thats something which should not come via a patch version.

I found one treeToValue which was not converted from convertValue and whould be a convertValue, changed that just now.

@Julius278
Copy link
Copy Markdown
Contributor Author

(force pushed cause I forgot the sign off)

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

Successfully merging this pull request may close these issues.

2 participants