From 008656202e0451ef5322e0f5e2d355217578ff43 Mon Sep 17 00:00:00 2001 From: seonwooj0810 Date: Sun, 14 Jun 2026 14:17:41 +0900 Subject: [PATCH 1/2] Restore property name from map key in handleUnwrapped to avoid null keys `ModelResolver.handleUnwrapped` forwards an inner model's property schemas into the outer model's `props` list. Because `Schema.getName()` is `@JsonIgnore`, an inner property schema can arrive with `name == null` (e.g. after a JSON-based clone such as `AnnotationsUtils.clone`) while the inner properties-map key still carries the correct name. The subsequent `modelProps.put(prop.getName(), prop)` then inserts a `null` key, which makes Jackson fail to serialize the document with `JsonMappingException: Null key for a Map not allowed in JSON`. Make `handleUnwrapped` defensive: iterate the inner properties by entry and restore the property name from the authoritative map key when the schema's transient name has been lost. The prefix/suffix branch falls back to the entry key the same way instead of producing a literal `prefix + "null" + suffix`. Relates to #5126 (reported with Spring HATEOAS `EntityModel`). The exact end-to-end trigger could not be reproduced from a plain annotated model, so this is intentionally a defensive guard on `handleUnwrapped`'s output rather than a fix targeting one specific resolver path. Covered by a unit test that drives `handleUnwrapped` with inner property schemas in the post-clone (null-name) state, asserting non-null keys with and without a prefix/suffix. --- .../v3/core/jackson/ModelResolver.java | 22 ++++- .../v3/core/resolving/Ticket5126Test.java | 95 +++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java diff --git a/modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java b/modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java index 675ea0a668..d545ebbb12 100644 --- a/modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java +++ b/modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java @@ -95,7 +95,6 @@ import java.math.BigDecimal; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -1461,7 +1460,18 @@ protected boolean ignore(final Annotated member, final XmlAccessorType xmlAccess private void handleUnwrapped(List props, Schema innerModel, String prefix, String suffix, List requiredProps) { if (StringUtils.isBlank(suffix) && StringUtils.isBlank(prefix)) { if (innerModel.getProperties() != null) { - props.addAll(innerModel.getProperties().values()); + // Schema.getName() is @JsonIgnore, so any prior JSON-based clone of innerModel + // (e.g. AnnotationsUtils.clone) leaves nested property schemas with a null name + // while the properties-map key still carries the correct name. Restore from the + // map key so the eventual `modelProps.put(prop.getName(), prop)` does not insert + // a null key (see swagger-api/swagger-core#5126). + for (Map.Entry entry : ((Map) innerModel.getProperties()).entrySet()) { + Schema prop = entry.getValue(); + if (prop.getName() == null) { + prop.setName(entry.getKey()); + } + props.add(prop); + } if (innerModel.getRequired() != null) { requiredProps.addAll(innerModel.getRequired()); } @@ -1475,10 +1485,14 @@ private void handleUnwrapped(List props, Schema innerModel, String prefi suffix = ""; } if (innerModel.getProperties() != null) { - for (Schema prop : (Collection) innerModel.getProperties().values()) { + for (Map.Entry entry : ((Map) innerModel.getProperties()).entrySet()) { + Schema prop = entry.getValue(); try { Schema clonedProp = Json.mapper().readValue(Json.pretty(prop), Schema.class); - clonedProp.setName(prefix + prop.getName() + suffix); + // Fall back to the map key when the prop's transient name has been lost + // by a prior clone (Schema.getName() is @JsonIgnore). + String baseName = prop.getName() != null ? prop.getName() : entry.getKey(); + clonedProp.setName(prefix + baseName + suffix); props.add(clonedProp); } catch (IOException e) { LOGGER.error("Exception cloning property", e); diff --git a/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java b/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java new file mode 100644 index 0000000000..f9eca6c955 --- /dev/null +++ b/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java @@ -0,0 +1,95 @@ +package io.swagger.v3.core.resolving; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.swagger.v3.core.jackson.ModelResolver; +import io.swagger.v3.oas.models.media.Schema; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; + +/** + * Regression test for #5126. + * + *

When an inner model's nested property schemas have a {@code null} name — which happens + * naturally after {@code AnnotationsUtils.clone} round-trips the schema through JSON, because + * {@code Schema.getName()} is {@code @JsonIgnore} — {@link ModelResolver#handleUnwrapped} would + * forward those schemas into the outer model's properties list as-is. The subsequent + * {@code modelProps.put(prop.getName(), prop)} then inserts a {@code null} key into the outer's + * properties map, which later causes Jackson to fail serializing the OpenAPI document with + * {@code JsonMappingException: Null key for a Map not allowed in JSON}. + * + *

This test invokes {@code handleUnwrapped} directly with a hand-crafted inner model whose + * property schemas have a {@code null} name (simulating the post-clone state) and verifies that + * the resulting property names come from the inner properties-map keys, not from the transient + * {@code Schema.name} field. + */ +public class Ticket5126Test extends SwaggerTestBase { + + private ModelResolver modelResolver; + private Method handleUnwrapped; + + @BeforeMethod + public void setup() throws Exception { + modelResolver = new ModelResolver(new ObjectMapper()); + handleUnwrapped = ModelResolver.class.getDeclaredMethod( + "handleUnwrapped", List.class, Schema.class, String.class, String.class, List.class); + handleUnwrapped.setAccessible(true); + } + + @Test + public void noPrefixOrSuffix_restoresNameFromInnerPropertiesMapKey() throws Exception { + Schema nameSchema = new Schema<>().type("string"); // name field unset + Schema countSchema = new Schema<>().type("integer"); // name field unset + Schema inner = new Schema<>(); + Map innerProps = new LinkedHashMap<>(); + innerProps.put("name", nameSchema); + innerProps.put("count", countSchema); + inner.setProperties(innerProps); + + List props = new ArrayList<>(); + List requiredProps = new ArrayList<>(); + handleUnwrapped.invoke(modelResolver, props, inner, "", "", requiredProps); + + assertEquals(props.size(), 2); + LinkedHashSet names = new LinkedHashSet<>(); + for (Schema p : props) { + assertNotNull(p.getName(), + "Each unwrapped property must carry a non-null name to avoid null keys in the outer's properties map"); + names.add(p.getName()); + } + assertEquals(names, new LinkedHashSet<>(java.util.Arrays.asList("name", "count"))); + } + + @Test + public void withPrefixAndSuffix_combinesMapKeyWhenInnerNameIsNull() throws Exception { + Schema nameSchema = new Schema<>().type("string"); + Schema countSchema = new Schema<>().type("integer"); + Schema inner = new Schema<>(); + Map innerProps = new LinkedHashMap<>(); + innerProps.put("name", nameSchema); + innerProps.put("count", countSchema); + inner.setProperties(innerProps); + + List props = new ArrayList<>(); + List requiredProps = new ArrayList<>(); + handleUnwrapped.invoke(modelResolver, props, inner, "p_", "_s", requiredProps); + + assertEquals(props.size(), 2); + LinkedHashSet names = new LinkedHashSet<>(); + for (Schema p : props) { + assertNotNull(p.getName(), + "Prefixed/suffixed names must be derived from the inner properties-map key when the original Schema.name has been lost"); + names.add(p.getName()); + } + assertEquals(names, new LinkedHashSet<>(java.util.Arrays.asList("p_name_s", "p_count_s"))); + } +} From c4cc70c7bde29afa3adb72514b27fe87805e1507 Mon Sep 17 00:00:00 2001 From: Ewa Ostrowska Date: Tue, 21 Jul 2026 15:49:47 +0200 Subject: [PATCH 2/2] fix: add tests to cover the clone case and non-null schema names --- .../v3/core/resolving/Ticket5126Test.java | 137 ++++++++++++++---- 1 file changed, 105 insertions(+), 32 deletions(-) diff --git a/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java b/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java index f9eca6c955..c629a96500 100644 --- a/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java +++ b/modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket5126Test.java @@ -1,6 +1,8 @@ package io.swagger.v3.core.resolving; import com.fasterxml.jackson.databind.ObjectMapper; +import io.swagger.v3.core.util.AnnotationsUtils; +import io.swagger.v3.core.util.Json; import io.swagger.v3.core.jackson.ModelResolver; import io.swagger.v3.oas.models.media.Schema; import org.testng.annotations.BeforeMethod; @@ -8,29 +10,30 @@ import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; /** - * Regression test for #5126. + * Defensive invariant tests related to + * #5126. * - *

When an inner model's nested property schemas have a {@code null} name — which happens - * naturally after {@code AnnotationsUtils.clone} round-trips the schema through JSON, because - * {@code Schema.getName()} is {@code @JsonIgnore} — {@link ModelResolver#handleUnwrapped} would - * forward those schemas into the outer model's properties list as-is. The subsequent - * {@code modelProps.put(prop.getName(), prop)} then inserts a {@code null} key into the outer's - * properties map, which later causes Jackson to fail serializing the OpenAPI document with - * {@code JsonMappingException: Null key for a Map not allowed in JSON}. + *

{@link ModelResolver#handleUnwrapped} must not forward null-named property schemas into the + * outer model's properties list. The later {@code modelProps.put(prop.getName(), prop)} step uses + * those names as map keys, and a {@code null} key makes Jackson fail when serializing the schema. * - *

This test invokes {@code handleUnwrapped} directly with a hand-crafted inner model whose - * property schemas have a {@code null} name (simulating the post-clone state) and verifies that - * the resulting property names come from the inner properties-map keys, not from the transient - * {@code Schema.name} field. + *

These tests exercise the private method directly because the reported Spring HATEOAS path has + * not been reproduced as a swagger-core-only resolver test. The clone-based case demonstrates the + * concrete intermediate state this guard is intended to tolerate: {@code AnnotationsUtils.clone} + * keeps the properties-map keys while losing nested {@code Schema.name} values because + * {@code Schema.getName()} is {@code @JsonIgnore}. */ public class Ticket5126Test extends SwaggerTestBase { @@ -46,31 +49,81 @@ public void setup() throws Exception { } @Test - public void noPrefixOrSuffix_restoresNameFromInnerPropertiesMapKey() throws Exception { - Schema nameSchema = new Schema<>().type("string"); // name field unset - Schema countSchema = new Schema<>().type("integer"); // name field unset + public void noPrefixOrSuffix_restoresNameFromClonedInnerPropertiesMapKey() throws Exception { + Schema inner = cloneLosingNestedPropertyNames(namedInnerSchema()); + + assertEquals(inner.getProperties().keySet(), new LinkedHashSet<>(Arrays.asList("name", "count"))); + assertNull(((Schema) inner.getProperties().get("name")).getName()); + assertNull(((Schema) inner.getProperties().get("count")).getName()); + + List props = invokeHandleUnwrapped(inner, "", ""); + + assertPropertyNames(props, "name", "count"); + Schema outer = schemaWithModelProps(props); + assertFalse(outer.getProperties().containsKey(null)); + Json.mapper().writeValueAsString(outer); + } + + @Test + public void noPrefixOrSuffix_restoresNameFromInnerPropertiesMapKeyForNullNameState() throws Exception { + Schema inner = innerSchemaWithNullPropertyNames(); + + List props = invokeHandleUnwrapped(inner, "", ""); + + assertPropertyNames(props, "name", "count"); + } + + @Test + public void noPrefixOrSuffix_preservesExistingPropertyName() throws Exception { + Schema nameSchema = new Schema<>().type("string"); + nameSchema.setName("schemaName"); Schema inner = new Schema<>(); Map innerProps = new LinkedHashMap<>(); - innerProps.put("name", nameSchema); - innerProps.put("count", countSchema); + innerProps.put("mapName", nameSchema); inner.setProperties(innerProps); - List props = new ArrayList<>(); - List requiredProps = new ArrayList<>(); - handleUnwrapped.invoke(modelResolver, props, inner, "", "", requiredProps); + List props = invokeHandleUnwrapped(inner, "", ""); - assertEquals(props.size(), 2); - LinkedHashSet names = new LinkedHashSet<>(); - for (Schema p : props) { - assertNotNull(p.getName(), - "Each unwrapped property must carry a non-null name to avoid null keys in the outer's properties map"); - names.add(p.getName()); - } - assertEquals(names, new LinkedHashSet<>(java.util.Arrays.asList("name", "count"))); + assertPropertyNames(props, "schemaName"); + } + + @Test + public void withPrefixAndSuffix_combinesMapKeyWhenClonedInnerNameIsNull() throws Exception { + Schema inner = cloneLosingNestedPropertyNames(namedInnerSchema()); + + List props = invokeHandleUnwrapped(inner, "p_", "_s"); + + assertPropertyNames(props, "p_name_s", "p_count_s"); + Schema outer = schemaWithModelProps(props); + assertFalse(outer.getProperties().containsKey(null)); + Json.mapper().writeValueAsString(outer); } @Test public void withPrefixAndSuffix_combinesMapKeyWhenInnerNameIsNull() throws Exception { + Schema inner = innerSchemaWithNullPropertyNames(); + + List props = invokeHandleUnwrapped(inner, "p_", "_s"); + + assertPropertyNames(props, "p_name_s", "p_count_s"); + } + + private Schema namedInnerSchema() { + Schema nameSchema = new Schema<>().type("string"); + nameSchema.setName("name"); + Schema countSchema = new Schema<>().type("integer"); + countSchema.setName("count"); + + Schema inner = new Schema<>(); + inner.setName("Inner"); + Map innerProps = new LinkedHashMap<>(); + innerProps.put("name", nameSchema); + innerProps.put("count", countSchema); + inner.setProperties(innerProps); + return inner; + } + + private Schema innerSchemaWithNullPropertyNames() { Schema nameSchema = new Schema<>().type("string"); Schema countSchema = new Schema<>().type("integer"); Schema inner = new Schema<>(); @@ -78,18 +131,38 @@ public void withPrefixAndSuffix_combinesMapKeyWhenInnerNameIsNull() throws Excep innerProps.put("name", nameSchema); innerProps.put("count", countSchema); inner.setProperties(innerProps); + return inner; + } + + private Schema cloneLosingNestedPropertyNames(Schema inner) { + return AnnotationsUtils.clone(inner, false); + } + private List invokeHandleUnwrapped(Schema inner, String prefix, String suffix) throws Exception { List props = new ArrayList<>(); List requiredProps = new ArrayList<>(); - handleUnwrapped.invoke(modelResolver, props, inner, "p_", "_s", requiredProps); + handleUnwrapped.invoke(modelResolver, props, inner, prefix, suffix, requiredProps); + return props; + } + + private Schema schemaWithModelProps(List props) { + Schema outer = new Schema<>(); + Map modelProps = new LinkedHashMap<>(); + for (Schema prop : props) { + modelProps.put(prop.getName(), prop); + } + outer.setProperties(modelProps); + return outer; + } - assertEquals(props.size(), 2); + private void assertPropertyNames(List props, String... expectedNames) { + assertEquals(props.size(), expectedNames.length); LinkedHashSet names = new LinkedHashSet<>(); for (Schema p : props) { assertNotNull(p.getName(), - "Prefixed/suffixed names must be derived from the inner properties-map key when the original Schema.name has been lost"); + "Each unwrapped property must carry a non-null name to avoid null keys in the outer's properties map"); names.add(p.getName()); } - assertEquals(names, new LinkedHashSet<>(java.util.Arrays.asList("p_name_s", "p_count_s"))); + assertEquals(names, new LinkedHashSet<>(Arrays.asList(expectedNames))); } }