From 0888a722e64d0baef4a8640f504486f4ae496a18 Mon Sep 17 00:00:00 2001 From: Sahana Bogar Date: Fri, 18 Sep 2026 11:54:40 +0530 Subject: [PATCH 1/6] copy introspector in defaultUseWrapper() instead of mutating shared one --- release-notes/CREDITS | 3 + release-notes/VERSION | 3 + .../xml/JacksonXmlAnnotationIntrospector.java | 27 ++++++- .../jackson/dataformat/xml/XmlMapper.java | 15 +++- .../dataformat/xml/MapperCopyTest.java | 79 +++++++++++++++++++ 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 733d22b37..dfbc98040 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -208,3 +208,6 @@ Christian Beikov (@beikov) * Fixed #911: Verify Stax factory type before instantiating in `XmlFactory.readResolve()` (3.1.7) + * Fixed #913: `XmlMapper.Builder.defaultUseWrapper()` changes mapper that + builder was created from (via `rebuild()`), or has already built + (3.3.0) diff --git a/release-notes/VERSION b/release-notes/VERSION index 80fd89892..c0fe4a107 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -44,6 +44,9 @@ Version: 3.x (for earlier see VERSION-2.x) #907: Recompute XML metadata for filtered properties in `XmlBeanSerializerBase` (wrong attribute/text/CDATA handling after `@JsonIgnoreProperties`) (fix by @Sahana2524) +#913: `XmlMapper.Builder.defaultUseWrapper()` changes mapper that builder was + created from (via `rebuild()`), or has already built + (fix by @Sahana2524) 3.2.3 (not yet released) diff --git a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java index 397c62179..3e2e430c0 100644 --- a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java @@ -19,7 +19,7 @@ */ public class JacksonXmlAnnotationIntrospector extends JacksonAnnotationIntrospector - implements XmlAnnotationIntrospector + implements XmlAnnotationIntrospector, Cloneable { private static final long serialVersionUID = 1L; @@ -65,6 +65,31 @@ public void setDefaultUseWrapper(boolean b) { _cfgDefaultUseWrapper = b; } + /** + * Mutant factory for getting an introspector that uses given default for + * List wrapping: returns this instance if it already does, a re-configured + * copy otherwise. Unlike {@link #setDefaultUseWrapper} never modifies this + * instance, which matters as an introspector may be shared by multiple + * (immutable) mappers; see {@code XmlMapper.rebuild()}. + *

+ * Copy retains the actual (sub-)type along with other settings. + * + * @since 3.3 + */ + public JacksonXmlAnnotationIntrospector withDefaultUseWrapper(boolean b) { + if (_cfgDefaultUseWrapper == b) { + return this; + } + final JacksonXmlAnnotationIntrospector copy; + try { + copy = (JacksonXmlAnnotationIntrospector) clone(); + } catch (CloneNotSupportedException e) { // should never occur, we are `Cloneable` + throw new IllegalStateException(e); + } + copy._cfgDefaultUseWrapper = b; + return copy; + } + /* /********************************************************************** /* Overrides of JacksonAnnotationIntrospector impls diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java index 25f6bccb7..15b2b16ee 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java @@ -279,11 +279,24 @@ public Builder defaultUseWrapper(boolean b) { if (_defaultUseWrapper != b) { _defaultUseWrapper = b; + // Introspector may be shared with the mapper this builder was created + // from (see `XmlMapper.rebuild()`), as well as with mappers it has + // already built: so must not modify it in place but swap in + // re-configured copy (same as `nameForTextElement()` does for factory) AnnotationIntrospector ai0 = annotationIntrospector(); + AnnotationIntrospector newAi = null; + boolean changed = false; for (AnnotationIntrospector ai : ai0.allIntrospectors()) { if (ai instanceof JacksonXmlAnnotationIntrospector xmlAi) { - xmlAi.setDefaultUseWrapper(b); + ai = xmlAi.withDefaultUseWrapper(b); + changed |= (ai != xmlAi); } + // pairs are flattened (in precedence order) by `allIntrospectors()` + newAi = (newAi == null) ? ai + : XmlAnnotationIntrospector.Pair.instance(newAi, ai); + } + if (changed) { + annotationIntrospector(newAi); } } return this; diff --git a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java index 86a0c507c..72ec09f3f 100644 --- a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java @@ -1,6 +1,7 @@ package tools.jackson.dataformat.xml; import java.io.*; +import java.util.*; import org.junit.jupiter.api.Test; @@ -18,6 +19,24 @@ static class Pojo282 public int a = 3; } + // [dataformat-xml#913]: separate types per test so that no cached + // (de)serializer can hide effects of introspector changes + static class ListBean913A { + public List values = new ArrayList<>(Arrays.asList("a", "b")); + } + + static class ListBean913B { + public List values = new ArrayList<>(Arrays.asList("a", "b")); + } + + static class ListBean913C { + public List values = new ArrayList<>(Arrays.asList("a", "b")); + } + + static class CustomIntrospector913 extends JacksonXmlAnnotationIntrospector { + private static final long serialVersionUID = 1L; + } + @Test public void testMapperCopy() { @@ -93,4 +112,64 @@ public void testCopyWith() throws Exception fail("Should NOT use name 'AnnotatedName' but 'Pojo282', xml = "+xml1); } } + + // [dataformat-xml#913]: `defaultUseWrapper()` of builder from `rebuild()` must + // not change the (immutable) mapper that builder was created from + @Test + public void testRebuildWithDefaultUseWrapper() throws Exception + { + final String WRAPPED = "ab"; + final String UNWRAPPED = "ab"; + + final XmlMapper mapper1 = newMapper(); + final XmlMapper mapper2 = mapper1.rebuild() + .defaultUseWrapper(false) + .build(); + + assertEquals(UNWRAPPED, mapper2.writeValueAsString(new ListBean913A())); + assertEquals(Arrays.asList("a", "b"), + mapper2.readValue(UNWRAPPED, ListBean913A.class).values); + + // original mapper must keep using wrapping, for writing and reading + assertEquals(WRAPPED, mapper1.writeValueAsString(new ListBean913A())); + assertEquals(Arrays.asList("a", "b"), + mapper1.readValue(WRAPPED, ListBean913A.class).values); + } + + // [dataformat-xml#913]: ... nor mapper that builder itself built earlier + @Test + public void testDefaultUseWrapperAfterBuild() throws Exception + { + final XmlMapper.Builder b = mapperBuilder(); + final XmlMapper mapper1 = b.build(); + final XmlMapper mapper2 = b.defaultUseWrapper(false).build(); + + assertEquals("ab", + mapper1.writeValueAsString(new ListBean913B())); + assertEquals("ab", + mapper2.writeValueAsString(new ListBean913B())); + } + + // [dataformat-xml#913]: copy used must retain introspector (sub-)type, and + // other introspectors it may be paired with + @Test + public void testDefaultUseWrapperWithCustomIntrospectors() throws Exception + { + final CustomIntrospector913 xmlIntr = new CustomIntrospector913(); + final AnnotationIntrospector jaxbIntr = jakartaXMLBindAnnotationIntrospector(); + final XmlMapper mapper = mapperBuilder() + .annotationIntrospector(XmlAnnotationIntrospector.Pair.instance(xmlIntr, jaxbIntr)) + .defaultUseWrapper(false) + .build(); + + List all = new ArrayList<>( + mapper.serializationConfig().getAnnotationIntrospector().allIntrospectors()); + assertEquals(2, all.size()); + assertEquals(CustomIntrospector913.class, all.get(0).getClass()); + assertNotSame(xmlIntr, all.get(0)); + assertSame(jaxbIntr, all.get(1)); + + assertEquals("ab", + mapper.writeValueAsString(new ListBean913C())); + } } From d36b52bd95397552cce8fa5facdf38dadfe593ec Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 22 Sep 2026 18:30:37 -0700 Subject: [PATCH 2/6] Deprecation of `JacksonXmlAnnotationIntrospector.setDefaultUseWrapper` --- README.md | 2 +- .../dataformat/xml/JacksonXmlAnnotationIntrospector.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3b85745cb..87187392f 100644 --- a/README.md +++ b/README.md @@ -283,7 +283,7 @@ Currently, following limitations exist beyond general Jackson (JSON) limitations * Note: over time some level of support has been added, and `Collection`s, for example, often work. * Lists and arrays are "wrapped" by default, when using Jackson annotations, but unwrapped when using JAXB annotations (if supported, see below) * `@JacksonXmlElementWrapper.useWrapping` can be set to 'false' to disable wrapping - * `JacksonXmlModule.setDefaultUseWrapper()` can be used to specify whether "wrapped" or "unwrapped" setting is the default + * `XmlMapper.builder().defaultUseWrapper()` can be used to specify whether "wrapped" or "unwrapped" setting is the default * Polymorphic Type Handling works, but only some inclusion mechanisms are supported (`WRAPPER_ARRAY`, for example is not supported due to problems with reference to mapping of XML, Arrays) * JAXB-style "compact" Type Id where property name is replaced with Type Id is not supported. * Mixed Content (elements and text in same element) is not supported in databinding: child content must be either text OR element(s) (attributes are fine) diff --git a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java index 3e2e430c0..07860ef2a 100644 --- a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java @@ -61,6 +61,12 @@ public JacksonXmlAnnotationIntrospector(boolean defaultUseWrapper) { /********************************************************************** */ + /** + * @deprecated Since 3.3 use {@link #withDefaultUseWrapper} instead: modifying + * an introspector in place also affects any mapper that is already using it + * (including ones created via {@code XmlMapper.rebuild()}) + */ + @Deprecated // since 3.3 public void setDefaultUseWrapper(boolean b) { _cfgDefaultUseWrapper = b; } From cf787f4babd30e4c383559d5357ae01067a746eb Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 22 Sep 2026 18:43:32 -0700 Subject: [PATCH 3/6] Rework the fix a bit --- .../xml/JacksonXmlAnnotationIntrospector.java | 40 ++++++++------ .../xml/XmlAnnotationIntrospector.java | 51 ++++++++++++++++++ .../jackson/dataformat/xml/XmlMapper.java | 34 ++++++------ .../dataformat/xml/MapperCopyTest.java | 52 +++++++++++++++++++ 4 files changed, 146 insertions(+), 31 deletions(-) diff --git a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java index 07860ef2a..ea76de229 100644 --- a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java @@ -7,6 +7,7 @@ import tools.jackson.databind.PropertyName; import tools.jackson.databind.cfg.MapperConfig; import tools.jackson.databind.introspect.*; +import tools.jackson.databind.util.ClassUtil; import tools.jackson.dataformat.xml.annotation.*; /** @@ -19,7 +20,7 @@ */ public class JacksonXmlAnnotationIntrospector extends JacksonAnnotationIntrospector - implements XmlAnnotationIntrospector, Cloneable + implements XmlAnnotationIntrospector { private static final long serialVersionUID = 1L; @@ -55,6 +56,20 @@ public JacksonXmlAnnotationIntrospector(boolean defaultUseWrapper) { _cfgDefaultUseWrapper = defaultUseWrapper; } + /** + * Copy constructor for sub-classes to use when overriding + * {@link #withDefaultUseWrapper}: copies settings of {@code src} other + * than default for List wrapping, which is set to given value. + * + * @since 3.3 + */ + protected JacksonXmlAnnotationIntrospector(JacksonXmlAnnotationIntrospector src, + boolean defaultUseWrapper) + { + _cfgConstructorPropertiesImpliesCreator = src._cfgConstructorPropertiesImpliesCreator; + _cfgDefaultUseWrapper = defaultUseWrapper; + } + /* /********************************************************************** /* Extended API XML format module requires @@ -72,28 +87,21 @@ public void setDefaultUseWrapper(boolean b) { } /** - * Mutant factory for getting an introspector that uses given default for - * List wrapping: returns this instance if it already does, a re-configured - * copy otherwise. Unlike {@link #setDefaultUseWrapper} never modifies this - * instance, which matters as an introspector may be shared by multiple - * (immutable) mappers; see {@code XmlMapper.rebuild()}. - *

- * Copy retains the actual (sub-)type along with other settings. + * Sub-classes MUST override this method (usually using copy constructor + * {@link #JacksonXmlAnnotationIntrospector(JacksonXmlAnnotationIntrospector, boolean)}) + * to retain their type and settings; otherwise an {@link IllegalStateException} + * is thrown when a re-configured copy would be needed. * * @since 3.3 */ + @Override public JacksonXmlAnnotationIntrospector withDefaultUseWrapper(boolean b) { if (_cfgDefaultUseWrapper == b) { return this; } - final JacksonXmlAnnotationIntrospector copy; - try { - copy = (JacksonXmlAnnotationIntrospector) clone(); - } catch (CloneNotSupportedException e) { // should never occur, we are `Cloneable` - throw new IllegalStateException(e); - } - copy._cfgDefaultUseWrapper = b; - return copy; + ClassUtil.verifyMustOverride(JacksonXmlAnnotationIntrospector.class, this, + "withDefaultUseWrapper"); + return new JacksonXmlAnnotationIntrospector(this, b); } /* diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java index 7c8f9d829..e68f623fa 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java @@ -5,6 +5,7 @@ import tools.jackson.databind.cfg.MapperConfig; import tools.jackson.databind.introspect.Annotated; import tools.jackson.databind.introspect.AnnotationIntrospectorPair; +import tools.jackson.databind.util.ClassUtil; /** * Additional extension interface used above and beyond @@ -13,6 +14,27 @@ public interface XmlAnnotationIntrospector extends AnnotationIntrospector.XmlExtensions { + /** + * Mutant factory for getting an introspector that uses given default for + * List wrapping (for Lists and arrays without explicit wrapper annotation): + * returns this instance if there is no change (or no such setting), a + * re-configured copy otherwise. Must never modify this instance, as an + * introspector may be shared by multiple (immutable) mappers; see + * {@code XmlMapper.rebuild()}. + *

+ * Default implementation returns {@code this}, for introspectors that have + * no such setting. + * + * @param defaultUseWrapper Whether to use wrapping by default or not + * + * @return Introspector that uses given default for wrapping + * + * @since 3.3 + */ + default XmlAnnotationIntrospector withDefaultUseWrapper(boolean defaultUseWrapper) { + return this; + } + /* /********************************************************************** /* Replacement of 'AnnotationIntrospector.Pair' to use when combining @@ -51,6 +73,35 @@ public Pair(AnnotationIntrospector p, AnnotationIntrospector s) public static XmlAnnotationIntrospector.Pair instance(AnnotationIntrospector a1, AnnotationIntrospector a2) { return new XmlAnnotationIntrospector.Pair(a1, a2); } + + /** + * Sub-classes MUST override this method to retain their type; otherwise + * an {@link IllegalStateException} is thrown when a re-configured copy + * would be needed. + * + * @since 3.3 + */ + @Override + public XmlAnnotationIntrospector withDefaultUseWrapper(boolean defaultUseWrapper) + { + AnnotationIntrospector p = _withDefaultUseWrapper(_primary, defaultUseWrapper); + AnnotationIntrospector s = _withDefaultUseWrapper(_secondary, defaultUseWrapper); + if ((p == _primary) && (s == _secondary)) { + return this; + } + ClassUtil.verifyMustOverride(XmlAnnotationIntrospector.Pair.class, this, + "withDefaultUseWrapper"); + return new XmlAnnotationIntrospector.Pair(p, s); + } + + protected static AnnotationIntrospector _withDefaultUseWrapper(AnnotationIntrospector ai, + boolean defaultUseWrapper) + { + if (ai instanceof XmlAnnotationIntrospector xmlAi) { + return (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(defaultUseWrapper); + } + return ai; + } @Override public String findNamespace(MapperConfig config, Annotated ann) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java index 15b2b16ee..d3c350cf3 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java @@ -277,27 +277,31 @@ public boolean defaultUseWrapper() { */ public Builder defaultUseWrapper(boolean b) { if (_defaultUseWrapper != b) { - _defaultUseWrapper = b; - // Introspector may be shared with the mapper this builder was created // from (see `XmlMapper.rebuild()`), as well as with mappers it has // already built: so must not modify it in place but swap in // re-configured copy (same as `nameForTextElement()` does for factory) - AnnotationIntrospector ai0 = annotationIntrospector(); - AnnotationIntrospector newAi = null; - boolean changed = false; - for (AnnotationIntrospector ai : ai0.allIntrospectors()) { - if (ai instanceof JacksonXmlAnnotationIntrospector xmlAi) { - ai = xmlAi.withDefaultUseWrapper(b); - changed |= (ai != xmlAi); + AnnotationIntrospector ai = annotationIntrospector(); + if (ai instanceof XmlAnnotationIntrospector xmlAi) { + AnnotationIntrospector newAi = (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(b); + if (newAi != ai) { + annotationIntrospector(newAi); + } + } else { + // Otherwise can not re-configure (without changing structure): must + // fail if contained introspector (like one within databind-provided + // `AnnotationIntrospectorPair`) would need change + for (AnnotationIntrospector curr : ai.allIntrospectors()) { + if ((curr instanceof XmlAnnotationIntrospector xmlAi) + && (xmlAi.withDefaultUseWrapper(b) != xmlAi)) { + throw new IllegalStateException(String.format( +"Cannot change `defaultUseWrapper` of `%s` contained in `%s`: combine introspectors using `%s` instead", + curr.getClass().getName(), ai.getClass().getName(), + XmlAnnotationIntrospector.Pair.class.getName())); + } } - // pairs are flattened (in precedence order) by `allIntrospectors()` - newAi = (newAi == null) ? ai - : XmlAnnotationIntrospector.Pair.instance(newAi, ai); - } - if (changed) { - annotationIntrospector(newAi); } + _defaultUseWrapper = b; } return this; } diff --git a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java index 72ec09f3f..a276c5a89 100644 --- a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java @@ -8,6 +8,7 @@ import com.fasterxml.jackson.annotation.JsonRootName; import tools.jackson.databind.*; +import tools.jackson.databind.introspect.AnnotationIntrospectorPair; import static org.junit.jupiter.api.Assertions.*; @@ -35,6 +36,22 @@ static class ListBean913C { static class CustomIntrospector913 extends JacksonXmlAnnotationIntrospector { private static final long serialVersionUID = 1L; + + public CustomIntrospector913() { } + + protected CustomIntrospector913(CustomIntrospector913 src, boolean defaultUseWrapper) { + super(src, defaultUseWrapper); + } + + @Override + public JacksonXmlAnnotationIntrospector withDefaultUseWrapper(boolean b) { + return (_cfgDefaultUseWrapper == b) ? this : new CustomIntrospector913(this, b); + } + } + + // sub-class that (incorrectly) does not override `withDefaultUseWrapper()` + static class NonOverridingIntrospector913 extends JacksonXmlAnnotationIntrospector { + private static final long serialVersionUID = 1L; } @Test @@ -172,4 +189,39 @@ public void testDefaultUseWrapperWithCustomIntrospectors() throws Exception assertEquals("ab", mapper.writeValueAsString(new ListBean913C())); } + + // [dataformat-xml#913]: sub-class not overriding `withDefaultUseWrapper()` must + // fail, instead of silently losing its type (or modifying shared instance) + @Test + public void testDefaultUseWrapperWithNonOverridingIntrospector() throws Exception + { + final NonOverridingIntrospector913 intr = new NonOverridingIntrospector913(); + final XmlMapper.Builder b = mapperBuilder().annotationIntrospector(intr); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> b.defaultUseWrapper(false)); + verifyException(e, "must override method 'withDefaultUseWrapper'"); + // and neither introspector nor builder must have been modified + assertTrue(intr._cfgDefaultUseWrapper); + assertTrue(b.defaultUseWrapper()); + } + + // [dataformat-xml#913]: databind `AnnotationIntrospectorPair` cannot be re-configured, + // so must fail if it contains introspector that would need change + @Test + public void testDefaultUseWrapperWithDatabindPair() throws Exception + { + final JacksonXmlAnnotationIntrospector xmlIntr = new JacksonXmlAnnotationIntrospector(); + final AnnotationIntrospector pair = AnnotationIntrospectorPair.create(xmlIntr, + jakartaXMLBindAnnotationIntrospector()); + final XmlMapper.Builder b = mapperBuilder().annotationIntrospector(pair); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> b.defaultUseWrapper(false)); + verifyException(e, "Cannot change `defaultUseWrapper`"); + verifyException(e, XmlAnnotationIntrospector.Pair.class.getName()); + assertTrue(xmlIntr._cfgDefaultUseWrapper); + assertTrue(b.defaultUseWrapper()); + + // but no-change is fine + assertSame(b, b.defaultUseWrapper(true)); + } } From 49f738ec208a4d9848f255967a00398def16ed8d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 22 Sep 2026 18:53:31 -0700 Subject: [PATCH 4/6] More fixes --- .../xml/XmlAnnotationIntrospector.java | 5 +++ .../jackson/dataformat/xml/XmlMapper.java | 31 +++++++++---------- .../dataformat/xml/MapperCopyTest.java | 20 ++++++++++++ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java index e68f623fa..1e469e7cc 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlAnnotationIntrospector.java @@ -24,6 +24,11 @@ public interface XmlAnnotationIntrospector *

* Default implementation returns {@code this}, for introspectors that have * no such setting. + *

+ * NOTE: implementations are expected to extend {@link AnnotationIntrospector}, + * and value returned MUST also be an {@link AnnotationIntrospector} (since it + * is used as the replacement introspector by + * {@code XmlMapper.Builder.defaultUseWrapper()}). * * @param defaultUseWrapper Whether to use wrapping by default or not * diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java index d3c350cf3..a5508e5bb 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java @@ -282,25 +282,24 @@ public Builder defaultUseWrapper(boolean b) { // already built: so must not modify it in place but swap in // re-configured copy (same as `nameForTextElement()` does for factory) AnnotationIntrospector ai = annotationIntrospector(); - if (ai instanceof XmlAnnotationIntrospector xmlAi) { - AnnotationIntrospector newAi = (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(b); - if (newAi != ai) { - annotationIntrospector(newAi); - } - } else { - // Otherwise can not re-configure (without changing structure): must - // fail if contained introspector (like one within databind-provided - // `AnnotationIntrospectorPair`) would need change - for (AnnotationIntrospector curr : ai.allIntrospectors()) { - if ((curr instanceof XmlAnnotationIntrospector xmlAi) - && (xmlAi.withDefaultUseWrapper(b) != xmlAi)) { - throw new IllegalStateException(String.format( + AnnotationIntrospector newAi = (ai instanceof XmlAnnotationIntrospector xmlAi) + ? (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(b) + : ai; + // But not all introspectors can be re-configured (without changing + // structure), like ones within databind-provided `AnnotationIntrospectorPair`: + // must fail if any of those would still need change + for (AnnotationIntrospector curr : newAi.allIntrospectors()) { + if ((curr instanceof XmlAnnotationIntrospector xmlAi) + && (xmlAi.withDefaultUseWrapper(b) != xmlAi)) { + throw new IllegalStateException(String.format( "Cannot change `defaultUseWrapper` of `%s` contained in `%s`: combine introspectors using `%s` instead", - curr.getClass().getName(), ai.getClass().getName(), - XmlAnnotationIntrospector.Pair.class.getName())); - } + curr.getClass().getName(), ai.getClass().getName(), + XmlAnnotationIntrospector.Pair.class.getName())); } } + if (newAi != ai) { + annotationIntrospector(newAi); + } _defaultUseWrapper = b; } return this; diff --git a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java index a276c5a89..245b92ae9 100644 --- a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java @@ -224,4 +224,24 @@ public void testDefaultUseWrapperWithDatabindPair() throws Exception // but no-change is fine assertSame(b, b.defaultUseWrapper(true)); } + + // [dataformat-xml#913]: ... including when nested within `XmlAnnotationIntrospector.Pair` + @Test + public void testDefaultUseWrapperWithNestedDatabindPair() throws Exception + { + final JacksonXmlAnnotationIntrospector nestedIntr = new JacksonXmlAnnotationIntrospector(); + final JacksonXmlAnnotationIntrospector otherIntr = new JacksonXmlAnnotationIntrospector(); + final AnnotationIntrospector pair = XmlAnnotationIntrospector.Pair.instance( + AnnotationIntrospectorPair.create(nestedIntr, jakartaXMLBindAnnotationIntrospector()), + otherIntr); + final XmlMapper.Builder b = mapperBuilder().annotationIntrospector(pair); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> b.defaultUseWrapper(false)); + verifyException(e, "Cannot change `defaultUseWrapper`"); + // neither introspectors nor builder modified + assertTrue(nestedIntr._cfgDefaultUseWrapper); + assertTrue(otherIntr._cfgDefaultUseWrapper); + assertSame(pair, b.annotationIntrospector()); + assertTrue(b.defaultUseWrapper()); + } } From 8094933fcff463ecbe46f61251ed80a23f0777eb Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 22 Sep 2026 19:04:16 -0700 Subject: [PATCH 5/6] ... --- .../dataformat/xml/JacksonXmlAnnotationIntrospector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java index ea76de229..196d56ae4 100644 --- a/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java +++ b/src/main/java/tools/jackson/dataformat/xml/JacksonXmlAnnotationIntrospector.java @@ -66,7 +66,7 @@ public JacksonXmlAnnotationIntrospector(boolean defaultUseWrapper) { protected JacksonXmlAnnotationIntrospector(JacksonXmlAnnotationIntrospector src, boolean defaultUseWrapper) { - _cfgConstructorPropertiesImpliesCreator = src._cfgConstructorPropertiesImpliesCreator; + super(src); _cfgDefaultUseWrapper = defaultUseWrapper; } From 3dd8b906c49e43f84cb17327703e03ea6024eb82 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 22 Sep 2026 19:15:19 -0700 Subject: [PATCH 6/6] Further fix --- .../jackson/dataformat/xml/XmlMapper.java | 49 ++++++++++--------- .../dataformat/xml/MapperCopyTest.java | 48 ++++++++++++++++++ 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java index a5508e5bb..dd55d91f3 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlMapper.java @@ -276,32 +276,33 @@ public boolean defaultUseWrapper() { * Jackson annotations have different default due to backwards compatibility. */ public Builder defaultUseWrapper(boolean b) { - if (_defaultUseWrapper != b) { - // Introspector may be shared with the mapper this builder was created - // from (see `XmlMapper.rebuild()`), as well as with mappers it has - // already built: so must not modify it in place but swap in - // re-configured copy (same as `nameForTextElement()` does for factory) - AnnotationIntrospector ai = annotationIntrospector(); - AnnotationIntrospector newAi = (ai instanceof XmlAnnotationIntrospector xmlAi) - ? (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(b) - : ai; - // But not all introspectors can be re-configured (without changing - // structure), like ones within databind-provided `AnnotationIntrospectorPair`: - // must fail if any of those would still need change - for (AnnotationIntrospector curr : newAi.allIntrospectors()) { - if ((curr instanceof XmlAnnotationIntrospector xmlAi) - && (xmlAi.withDefaultUseWrapper(b) != xmlAi)) { - throw new IllegalStateException(String.format( -"Cannot change `defaultUseWrapper` of `%s` contained in `%s`: combine introspectors using `%s` instead", - curr.getClass().getName(), ai.getClass().getName(), - XmlAnnotationIntrospector.Pair.class.getName())); - } + // Introspector may be shared with the mapper this builder was created + // from (see `XmlMapper.rebuild()`), as well as with mappers it has + // already built: so must not modify it in place but swap in + // re-configured copy (same as `nameForTextElement()` does for factory). + // NOTE: done even if builder setting is unchanged, since introspector + // may have been replaced with one that uses a different setting + AnnotationIntrospector ai = annotationIntrospector(); + AnnotationIntrospector newAi = (ai instanceof XmlAnnotationIntrospector xmlAi) + ? (AnnotationIntrospector) xmlAi.withDefaultUseWrapper(b) + : ai; + // But not all introspectors can be re-configured (without changing + // structure), like ones within databind-provided `AnnotationIntrospectorPair`: + // must fail if any of those would still need change + for (AnnotationIntrospector curr : newAi.allIntrospectors()) { + if ((curr instanceof XmlAnnotationIntrospector xmlAi) + && (xmlAi.withDefaultUseWrapper(b) != xmlAi)) { + throw new IllegalStateException(String.format( +"Cannot change `defaultUseWrapper` of `%s`: it is contained in a pair other than `%s` (like databind `AnnotationIntrospectorPair`); combine all introspectors using `%s` instead", + curr.getClass().getName(), + XmlAnnotationIntrospector.Pair.class.getName(), + XmlAnnotationIntrospector.Pair.class.getName())); } - if (newAi != ai) { - annotationIntrospector(newAi); - } - _defaultUseWrapper = b; } + if (newAi != ai) { + annotationIntrospector(newAi); + } + _defaultUseWrapper = b; return this; } diff --git a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java index 245b92ae9..e88a610fe 100644 --- a/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/MapperCopyTest.java @@ -34,6 +34,10 @@ static class ListBean913C { public List values = new ArrayList<>(Arrays.asList("a", "b")); } + static class ListBean913D { + public List values = new ArrayList<>(Arrays.asList("a", "b")); + } + static class CustomIntrospector913 extends JacksonXmlAnnotationIntrospector { private static final long serialVersionUID = 1L; @@ -54,6 +58,15 @@ static class NonOverridingIntrospector913 extends JacksonXmlAnnotationIntrospect private static final long serialVersionUID = 1L; } + // `Pair` sub-class that (incorrectly) does not override `withDefaultUseWrapper()` + static class NonOverridingPair913 extends XmlAnnotationIntrospector.Pair { + private static final long serialVersionUID = 1L; + + public NonOverridingPair913(AnnotationIntrospector p, AnnotationIntrospector s) { + super(p, s); + } + } + @Test public void testMapperCopy() { @@ -244,4 +257,39 @@ public void testDefaultUseWrapperWithNestedDatabindPair() throws Exception assertSame(pair, b.annotationIntrospector()); assertTrue(b.defaultUseWrapper()); } + + // [dataformat-xml#913]: `Pair` sub-class not overriding `withDefaultUseWrapper()` + // must fail, instead of silently losing its type (or modifying shared instances) + @Test + public void testDefaultUseWrapperWithNonOverridingPair() throws Exception + { + final JacksonXmlAnnotationIntrospector xmlIntr = new JacksonXmlAnnotationIntrospector(); + final AnnotationIntrospector pair = new NonOverridingPair913(xmlIntr, + jakartaXMLBindAnnotationIntrospector()); + final XmlMapper.Builder b = mapperBuilder().annotationIntrospector(pair); + IllegalStateException e = assertThrows(IllegalStateException.class, + () -> b.defaultUseWrapper(false)); + verifyException(e, "must override method 'withDefaultUseWrapper'"); + // neither introspectors nor builder modified + assertTrue(xmlIntr._cfgDefaultUseWrapper); + assertSame(pair, b.annotationIntrospector()); + assertTrue(b.defaultUseWrapper()); + + // but no-change is fine + assertSame(b, b.defaultUseWrapper(true)); + } + + // [dataformat-xml#913]: setting must be applied to introspector even if builder + // setting is unchanged (introspector replaced with one using different setting) + @Test + public void testDefaultUseWrapperAfterIntrospectorReplaced() throws Exception + { + final XmlMapper mapper = mapperBuilder() + .defaultUseWrapper(false) + .annotationIntrospector(new JacksonXmlAnnotationIntrospector(true)) + .defaultUseWrapper(false) + .build(); + assertEquals("ab", + mapper.writeValueAsString(new ListBean913D())); + } }