Repository navigation
GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 - #2949
Open
codeconsole wants to merge 12 commits into
Open
codeconsole wants to merge 12 commits into
codeconsole wants to merge 12 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2949 +/- ##
==================================================
+ Coverage 72.2662% 72.4165% +0.1504%
- Complexity 40538 40606 +68
==================================================
Files 1643 1643
Lines 142620 142647 +27
Branches 26786 26789 +3
==================================================
+ Hits 103066 103300 +234
+ Misses 30093 29848 -245
- Partials 9461 9499 +38
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The advertised Jackson 2 XML fallback has no isolated test coverage after the test runtime moves exclusively to Jackson 3.
1 open finding
What changed in this PR
Migrates Groovy’s YAML, TOML, and CSV support to Jackson 3 while preserving Jackson 2 behavior where intended.
Changes:
- Updates dependencies and mapper implementations to Jackson 3.1.6.
- Preserves databinding and CSV ordering compatibility and adds parser-limit tests.
- Adds Jackson 3 support with Jackson 2 fallback for typed XML parsing and updates documentation.
| File | Description |
|---|---|
versions.properties |
Defines Jackson 3 and SnakeYAML versions. |
threat-model.yaml |
Documents revised parser limits. |
THREAT_MODEL.md |
Explains Jackson 3 constraint changes. |
subprojects/groovy-yaml/src/test/groovy/groovy/yaml/YamlRootSequenceTest.groovy |
Tests root-sequence handling. |
subprojects/groovy-yaml/src/test/groovy/groovy/yaml/YamlLimitsTest.groovy |
Tests YAML nesting limits. |
subprojects/groovy-yaml/src/spec/doc/yaml-userguide.adoc |
Updates SnakeYAML guidance. |
subprojects/groovy-yaml/src/main/java/org/apache/groovy/yaml/util/YamlConverter.java |
Migrates conversion to Jackson 3. |
subprojects/groovy-yaml/src/main/java/groovy/yaml/YamlSlurper.java |
Uses a compatible shared YAML mapper. |
subprojects/groovy-yaml/src/main/java/groovy/yaml/YamlBuilder.java |
Migrates YAML writing. |
subprojects/groovy-yaml/build.gradle |
Updates YAML dependencies. |
subprojects/groovy-xml/src/test/groovy/groovy/xml/XmlParserTypedTest.groovy |
Tests typed-conversion failures. |
subprojects/groovy-xml/src/spec/test/UserGuideXmlTypedTest.groovy |
Updates the Jackson XML example. |
subprojects/groovy-xml/src/spec/doc/xml-userguide.adoc |
Documents dual-version support. |
subprojects/groovy-xml/src/main/java/org/apache/groovy/xml/util/JacksonHelper.java |
Prefers Jackson 3 with Jackson 2 fallback. |
subprojects/groovy-xml/build.gradle |
Tests XML against Jackson 3. |
subprojects/groovy-toml/src/test/groovy/groovy/toml/TomlLimitsTest.groovy |
Tests TOML nesting limits. |
subprojects/groovy-toml/src/main/java/org/apache/groovy/toml/util/TomlConverter.java |
Migrates conversion to Jackson 3. |
subprojects/groovy-toml/src/main/java/groovy/toml/TomlSlurper.java |
Configures Jackson 3 TOML binding. |
subprojects/groovy-toml/src/main/java/groovy/toml/TomlBuilder.java |
Migrates TOML writing. |
subprojects/groovy-toml/build.gradle |
Updates TOML dependencies. |
subprojects/groovy-json/src/spec/test/json/JsonTest.groovy |
Updates the direct-Jackson example. |
subprojects/groovy-json/src/spec/doc/json-userguide.adoc |
Documents Jackson 3 coordinates. |
subprojects/groovy-json/src/main/java/org/apache/groovy/json/internal/BaseJsonParser.java |
Clarifies nesting-limit differences. |
subprojects/groovy-csv/src/test/groovy/groovy/csv/CsvLimitsTest.groovy |
Tests the larger field limit. |
subprojects/groovy-csv/src/main/java/groovy/csv/CsvSlurper.java |
Migrates CSV parsing and ordering. |
subprojects/groovy-csv/src/main/java/groovy/csv/CsvBuilder.java |
Migrates CSV writing. |
subprojects/groovy-csv/build.gradle |
Updates CSV dependencies. |
src/spec/test/ScriptsAndClassesSpecTest.groovy |
Runs the script example with Jackson 3. |
gradle/verification-metadata.xml |
Trusts new dependency signatures. |
build.gradle |
Updates root test dependencies. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+74
to
+81
| Class<?> jsonMapperClass = loadOptionalClass(JACKSON3_JSON_MAPPER_CLASS); | ||
| if (jsonMapperClass != null) { | ||
| // Jackson 2's defaults (for example failing on an unknown property), so a typed | ||
| // parse behaves the same whichever Jackson version the application carries | ||
| Object builder = jsonMapperClass.getMethod("builderWithJackson2Defaults").invoke(null); | ||
| mapper = builder.getClass().getMethod("build").invoke(builder); | ||
| } else { | ||
| Class<?> objectMapperClass = loadOptionalClass(JACKSON2_OBJECT_MAPPER_CLASS); |
Contributor
Author
There was a problem hiding this comment.
Added in a19601a: a jackson2Test task in groovy-xml, part of check, reruns XmlParserTypedTest on the test runtime classpath with Jackson 3 swapped for Jackson 2.22.3 (the two classpaths differ only in the Jackson artifacts). That covers successful conversion and unknown-element rejection through the reflective fallback. testRunsAgainstTheExpectedJackson asserts which Jackson each run sees, so the task fails instead of silently retesting Jackson 3 if the swap breaks; removing the exclusion makes it fail.
XmlParser.parseAs and parseTextAs convert through whichever jackson-databind the application carries. Jackson 3 (tools.jackson.core:jackson-databind) is now used when present, built with builderWithJackson2Defaults() so an unknown element still fails the conversion as it does with Jackson 2; Jackson 2 remains the fallback. A failed conversion now carries Jackson's exception instead of the reflective InvocationTargetException, whose message was empty. The mapper is also published after its convertValue method, so a concurrent first call can no longer see the mapper with a null method. groovy-xml's tests now run against Jackson 3.1.6, the version Spring Boot 4.2 manages.
groovy-yaml now depends on tools.jackson.core:jackson-databind and tools.jackson.dataformat:jackson-dataformat-yaml 3.1.6, the Jackson 3 version Spring Boot 4.2 manages. Its public API exposes no Jackson type, so callers are unaffected. java.time support is built into Jackson 3 databind, so jackson-datatype-jsr310 is no longer needed. The mappers are built with configureForJackson2() so parsing and serialization keep Jackson 2's defaults (declaration-ordered properties, failing on unknown properties, lenient primitives and trailing tokens), and are shared because Jackson 3 mappers are immutable once built. Jackson 3's YAML backend uses SnakeYAML Engine rather than SnakeYAML, so the user guide's SnakeYAML alias recipe now declares org.yaml:snakeyaml itself; both are signed with keys the dependency verification already trusts.
groovy-toml now depends on tools.jackson.dataformat:jackson-dataformat-toml 3.1.6. As with groovy-yaml, its public API exposes no Jackson type, java.time support comes from Jackson 3 databind itself, and the mappers are built with configureForJackson2() so parsing and serialization keep Jackson 2's defaults.
groovy-csv now depends on tools.jackson.dataformat:jackson-dataformat-csv and tools.jackson.core:jackson-databind 3.1.6. The mapper is built with configureForJackson2() like the YAML and TOML ones, plus SORT_PROPERTIES_ALPHABETICALLY: Jackson 2's CsvMapper turned that on itself, and the user guide documents alphabetical columns for plain classes. Record components still come out in declaration order, which Jackson 3 does without the SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER flag Jackson 2 needed.
The script-with-run example in the user guide now serializes through Jackson 3's ObjectMapper; its output is unchanged. The root build's jackson-dataformat-yaml test dependency had no users and is removed. Nothing in the build uses Jackson 2 any more, so the jackson and jacksonAnnotations versions go; Groovy10461 still grabs its own Jackson 2 release, as that regression test requires.
The recipes for using jackson-databind and jackson-dataformat-xml directly now grab the Jackson 3 artifacts, whose classes keep their names under the tools.jackson packages. The XML guide also says typed parsing uses Jackson 3 when present and Jackson 2 otherwise, rejecting an unknown element with either.
CodeNarc reports assert false as a ConstantAssertExpression. The call stays outside a closure because Closure.call, like a dynamic call site, unwraps the XmlRuntimeException to its cause.
Moving YamlConverter to Jackson 3 replaced ObjectMapper.readValues(JsonParser) with ObjectReader.readValues(Reader), and an ObjectReader treats a root-level array as a wrapped sequence of values: [[[]]] parsed as [[]], and a document holding the single item - x parsed as the string x. Reading through an explicit parser again gives the results groovy-yaml 6.0.0 does.
Jackson 3 lowered the default StreamReadConstraints and StreamWriteConstraints nesting depth from 1000 to 500 and raised the default string length from 20,000,000 to 100,000,000, and configureForJackson2() does not restore either: they belong to the stream factory, not the mapper. groovy-yaml, groovy-toml and groovy-csv now set Jackson 2's values on every factory, so a document nested 501 to 1000 levels deep still parses and builds, a longer string still fails, and their nesting stays bounded at the 1000 levels JsonSlurper and XmlParser allow, as the threat model states.
Replaces the previous commit's pinning of Jackson 2's limits. Jackson 3 lowered the default nesting depth to 500 because fuzzers reached a StackOverflowError before the old 1000-level limit (jackson-core apache#1233), and raised the default string length to 100,000,000 after users hit 20,000,000 with legitimate payloads (jackson-core apache#1538). groovy-yaml, groovy-toml and groovy-csv now take those defaults, which keeps the threat model's stance that their bounds are the dependency's, not a Groovy decision. The limit tests now document the Jackson 3 values, and the threat model and JsonSlurper's comment no longer say its 1000-level cap matches the Jackson the slurpers use.
This comment has been minimized.
This comment has been minimized.
| implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson}" | ||
| implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}" | ||
| implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson}" | ||
| implementation "tools.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson3}" |
Contributor
Author
There was a problem hiding this comment.
Keeping Jackson 3's resolution, since this targets 7.0, and documenting it as a breaking change. It's wider than booleans: Jackson 3.1.6 hard-codes snakeyaml-engine's JsonScalarResolver in YAMLParser, so True/FALSE, Null/~ and 0x1F/017 also become strings, and no feature or LoadSettings schema restores YAML 1.1 resolution. 3702e55 adds YamlScalarResolutionTest pinning the behaviour and a note in the YAML user guide; the PR description now lists it under breaking changes.
Jackson 3's YAMLParser resolves untagged plain scalars with snakeyaml-engine's JsonScalarResolver, fixed in the parser rather than taken from LoadSettings. Only lowercase true, false and null and plain decimal numbers take a type: yes/no/on/off, True/FALSE, Null/NULL, ~ and 0x1F/017/1_000 now arrive as strings, where Jackson 2 gave booleans, null or integers. A non-empty String is truthy, so `enabled: off` now reads as true without error; the typed API rejects such a value for a boolean property instead. The test pins the new resolution and the user guide calls out the change.
JacksonHelper falls back to Jackson 2 when Jackson 3 is absent, but the groovy-xml tests now carry only Jackson 3, so that path went untested. A jackson2Test task, part of check, reruns XmlParserTypedTest on the test runtime classpath with Jackson 3 swapped for Jackson 2.22.3, covering both successful conversion and the rejection of unknown elements. A new test checks which Jackson each run sees, so the task fails rather than quietly testing Jackson 3 again if the swap stops working.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/GROOVY-12433
Moves groovy-yaml, groovy-toml and groovy-csv from Jackson 2 to Jackson 3 (
tools.jackson3.1.6, the version Spring Boot 4.2 manages). None of them exposes a Jackson type, so caller code needs no changes, but unquoted YAML values now resolve differently (see the breaking change below).configureForJackson2()so databinding keeps Jackson 2's defaults (declaration-ordered properties, unknown properties rejected, lenient nulls for primitives and trailing tokens). The CSV mapper also enablesSORT_PROPERTIES_ALPHABETICALLY, which Jackson 2'sCsvMapperturned on itself: plain classes keep alphabetical columns and records keep declaration order, as the user guide documents.StackOverflowErrorbelow 1000 (Lower defaultmaxNestingDepthofStreamReadConstraints/StreamWriteConstraintsto 500 in 3.0 FasterXML/jackson-core#1233) and raised the string length for legitimate large payloads (Increase defaultStreamReadConstraints.maxStringLengthto 100M (from 20M) FasterXML/jackson-core#1538).JsonSlurperandXmlParserkeep their 1000-level caps; the threat model andJsonSlurper's comment no longer say that cap matches the Jackson the slurpers use.jackson-datatype-jsr310is dropped; java.time support is built into Jackson 3 databind.XmlParser.parseAs/parseTextAsuse Jackson 3 when present and fall back to Jackson 2. A failed conversion now carries Jackson's exception; from dynamic Groovy it used to surface as anInvocationTargetExceptionwith no message. The cached mapper is now published after its method, so a concurrent first call can't see a null method. Ajackson2Testtask, part ofcheck, runs the typed XML tests with only Jackson 2 present, so the fallback stays covered.org.yaml:snakeyaml, since Jackson 3's YAML backend uses SnakeYAML Engine.tools.jacksonandsnakeyaml-engineare signed with keys the dependency verification already trusts; two scoped trust entries are added.Breaking change: unquoted YAML values
Jackson 3's YAML parser resolves unquoted values by the YAML 1.2 JSON schema, and the resolver is fixed in the parser rather than configurable. Only lowercase
true,falseandnulland plain decimal numbers take a type:yes/no/on/off,True/FALSE,Null/NULL,~and0x1F/017/1_000now arrive as strings, where Groovy 6 read them as booleans,nullor integers. A non-empty String is truthy, soenabled: offread withYamlSlurper.parseTextnow tests as true, without any error.parseAs/parseTextAsinstead reject such a value for abooleanproperty.YamlScalarResolutionTestpins the new behaviour, and the YAML user guide calls it out.