Repository navigation
CAMEL-24426: camel-gson, camel-jsonb - use the unmarshal type header only with allowUnmarshallType - #28156
Conversation
…only with allowUnmarshallType The Gson and JSON-B data formats resolved the class named by their CamelGsonUnmarshalType and CamelJsonbUnmarshallType headers on every unmarshal. As the Jackson data formats do with CamelJacksonUnmarshalType, the header is now only used when the new allowUnmarshallType option is enabled (default false). The json data format passes the option to Gson and JSON-B too. The JAXB data format is left as it is: it only unmarshals into a class known to its JAXBContext, so its CamelJaxbPartClass header cannot select any other class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
This PR correctly introduces the allowUnmarshallType flag to gate header-driven type coercion in Gson, JSON-B, and Jackson data formats. The security default change (opt-in instead of always-on) is well-motivated and consistently applied across all three libraries.
Key points verified:
- The
allowUnmarshallTypeflag is properly wired throughJsonDataFormatand propagated to each library-specific format - Existing tests are updated to explicitly opt in where needed; a new test asserts the default
Map.classbehavior when the flag is false - The upgrade guide accurately describes the breaking change
- Generated catalog descriptions (mentioning all three library-specific headers) derive correctly from the shared
@Metadataannotation onJsonDataFormat— this is by design, not an inconsistency
No issues found. Clean implementation.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
davsclaus
left a comment
There was a problem hiding this comment.
LGTM, thanks. Gson and JSON-B now behave like Jackson: the header that picks the unmarshal class is ignored unless the route opts in, and the upgrade guide covers the change. Nothing else in the repo sets CamelGsonUnmarshalType or CamelJsonbUnmarshallType. The JAXB reasoning holds too, since the JAXBContext limits the part class.
One suggestion, which can be a follow-up:
allowUnmarshallType=truenow gates the header-selected type for all three JSON libraries, but it is not in the Per-option opt-ins table ofdocs/user-manual/modules/ROOT/pages/security-model.adoc(the Jackson header is already cited there as CVE-2016-8749). A row for it would make triage of a report that needs the option straightforward (OUT-OF-MODEL). Whether it should also carrysecurity = "insecure:serialization"(so theprodprofile flags it) is a bigger call, because it would affect existing Jackson users, so I'd leave that out of this PR.
CI is still running on the current head.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
Thanks @davsclaus. #28088 had already landed, so I added the row in this PR instead of a follow-up: 2dcb3d1 adds Claude Code on behalf of Andrea Cosentino (@oscerd) |
Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
… the security model The option now gates the unmarshal type header of the Jackson, Gson and JSON-B data formats, so a report that needs it enabled is out of the model, as for the other opt-ins. Adds it to the table of security-model.adoc and to its security-model.yaml index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
davsclaus
left a comment
There was a problem hiding this comment.
LGTM, thanks for adding the security-model row. Since my approval, the changes are the merge of main, which leaves the PR diff as it was, and the new allowUnmarshallType=true row in the Per-option opt-ins table of security-model.adoc, with its entry in security-model.yaml.
One nit, optional:
camel-jackson3has the sameallowUnmarshallTypeoption andCamelJacksonUnmarshalTypeheader (its generatedjackson.jsonis changed in this PR too), so it could be listed next tocamel-jacksonin both the table row and the YAML comment.
CI is still queued on the current head.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 567 of 707 tested, 23 compile-only — current: 567 all testedMaveniverse Scalpel detected 567 affected modules (current approach: 567). Skip-tests mode would test 567 modules (10 direct + 561 downstream), skip tests for 23 (generated code, meta-modules) Modules Scalpel would test (567)
Modules with tests skipped (23)
Build reactor — dependencies compiled but only changed modules were tested (10 modules, 2m 17s total)Total reactor time: 2m 17s
Top 20 slowest modules:
|
The Jackson data formats only read their
CamelJacksonUnmarshalTypeheader whenallowUnmarshallTypeis enabled.The Gson and JSON-B data formats honoured their equivalent header unconditionally:
GsonDataFormat.unmarshal()andJsonbDataFormat.unmarshal()resolved the class named byCamelGsonUnmarshalType/CamelJsonbUnmarshallTypewiththe class resolver and unmarshalled into it.
This PR gives both data formats the
allowUnmarshallTypeoption, with the same name, semantics and default (false)as Jackson:
GsonDataFormat/JsonbDataFormat: newallowUnmarshallTypeproperty (inincludeProperties, so it is in thegson/jsonbmetadata); the header is read only when it is enabled.JsonbDataFormat.UNMARSHAL_TYPEnames theheader that was a string literal.
JsonDataFormatReifier: passesallowUnmarshallTypeforlibrary=Gsonandlibrary=Jsonbtoo, so<json library="Gson" allowUnmarshallType="true"/>andnew JsonDataFormat(JsonLibrary.Gson).allowUnmarshallType(true)work; the model description of the option mentions the three headers.
security-model.adocand itssecurity-model.yamlindex:allowUnmarshallType=true(Jackson, Gson, JSON-B) islisted among the per-option opt-ins.
camel-jaxb is not changed, unlike what the Jira suggests: the JAXB data format unmarshals with
Unmarshaller.unmarshal(reader, partClass), and JAXB only accepts a class known to theJAXBContextof the dataformat, so the
CamelJaxbPartClassheader cannot select another class (checked: a class outside the context is refusedwith
JAXBException: ... is not known to this context, and is never instantiated). Gating it would break the documentedpartial unmarshalling override for no gain. camel-fastjson has no such header.
Tests
GsonUnmarshalTypeFromHeaderTest: the header is used withallowUnmarshallType(true)and ignored by default (aMapis returned);
SpringGsonUnmarshalTypeFromHeaderTestnow setsallowUnmarshallType="true", which also covers thereifier and the XML DSL.
JsonbUnmarshalTypeFromHeaderTest: the same for JSON-B.GsonDataFormatTestno longer stubsexchange.getIn(): the data format does not read the exchange any more when theoption is off (strict Mockito flagged the unused stub).
camel-gson: 28 tests,camel-jsonb: 18 tests, 0 failures. With the previous data formats, the default case returnsthe class named by the header (
TestPojo) instead of aMap.mvn clean install -DskipTests -DskipITs: 706/706, only the expected regenerated files (the new optionin the
gson/jsonbmetadata and configurers; the option description in the model, the Jackson metadata, the XMLschemas and the YAML DSL, including the catalog copy of the YAML DSL model schema, which the second commit adds).
JIRA: https://issues.apache.org/jira/browse/CAMEL-24426
Claude Code on behalf of Andrea Cosentino (@oscerd)
🤖 Generated with Claude Code