search

FasterXML / jackson-dataformat-xml

Extension for Jackson JSON processor that adds support for serializing POJOs as XML (and deserializing from XML) as an alternative to JSON
Apache License 2.0
561 stars 221 forks source link

Fix mismatch in `setNextIsUnwrapped(boolean)` in `XmlBeanSerializerBase#serializeFieldsFiltered()` #616

Closed vmi closed 7 months ago

vmi commented 7 months ago

In serializeFields() of XmlBeanSerializerBase, setNextIsUnwrapped(true) is executed in L.200-203, and setNextIsUnwrapped(false) is executed in L.215-219. However, in serializeFieldsFiltered(), setNextIsUnwrapped(true) is executed in L.282-285, but there is no corresponding setNextIsUnwrapped(false).

pjfanning commented 7 months ago

Could you provide a test case?

vmi commented 7 months ago

I have no test cases. I will make it if necessary, but please give me some time.

cowtowncoder commented 7 months ago

This looks correct, but I agree with @pjfanning that it'd be really useful to have a test case to show the fix.

We can also backport the fix into 2.16 for 2.16.1, I think. I can cherry-pick merge unless you want to re-base (not sure if Github UI has a good way to do that).

cowtowncoder commented 7 months ago

@vmi One other process thing: since this is your first (but hopefully not last! :) ) contribution, I think we will need CLA from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and it's usually easiest to fill, sign, print & scan/photo, email to cla at fasterxml dot com. This only needs to be done once, before the first contribution and is good for all other contributions. Apologies for this extra step but it is something we need as OSS project used by corporations.

Thank you again for contributing this -- looking forward to merging it!

vmi commented 7 months ago

I added unit test, and sent signed CLA.

cowtowncoder commented 7 months ago

Excellent! CLA successfully received, test looks good; will merge after CI passes.

cowtowncoder commented 7 months ago

Since this seems like a safe fix, I backported it in 2.16.1 as well so will get released sooner (we only just started 2.17 branch).