From 0be5a50319e7140be66c00eb98426c2b71917fe1 Mon Sep 17 00:00:00 2001 From: Tommy Karlsson Date: Fri, 21 Nov 2025 14:13:00 +0100 Subject: [PATCH] Fix multiple HTTP message converters of the same type being lost When multiple HttpMessageConverters of the same specialized type (e.g., two JSON converters) were configured, only the last one was registered correctly. The first converter would be discarded because the specialized registration method was called for all converters of the same type, causing each to overwrite the previous one. This fix ensures that only the first converter of each specialized type (String, Kotlin Serialization JSON, JSON, XML) is registered using the specialized method (e.g., withJsonConverter()). Subsequent converters of the same type are registered as custom converters via addCustomConverter(). Signed-off-by: Tommy Karlsson --- ...ClientHttpMessageConvertersCustomizer.java | 69 +++++++-- ...ServerHttpMessageConvertersCustomizer.java | 69 +++++++-- ...rHttpMessageConvertersCustomizerTests.java | 131 ++++++++++++++++++ 3 files changed, 243 insertions(+), 26 deletions(-) create mode 100644 module/spring-boot-http-converter/src/test/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizerTests.java diff --git a/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultClientHttpMessageConvertersCustomizer.java b/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultClientHttpMessageConvertersCustomizer.java index ed1185a184c6..5b5a140c9e25 100644 --- a/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultClientHttpMessageConvertersCustomizer.java +++ b/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultClientHttpMessageConvertersCustomizer.java @@ -17,6 +17,9 @@ package org.springframework.boot.http.converter.autoconfigure; import java.util.Collection; +import java.util.EnumSet; +import java.util.function.BiConsumer; +import java.util.function.Predicate; import org.jspecify.annotations.Nullable; @@ -47,24 +50,32 @@ public void customize(ClientBuilder builder) { } else { builder.registerDefaults(); - this.converters.forEach((converter) -> { - if (converter instanceof StringHttpMessageConverter) { - builder.withStringConverter(converter); - } - else if (converter instanceof KotlinSerializationJsonHttpMessageConverter) { - builder.withKotlinSerializationJsonConverter(converter); - } - else if (supportsMediaType(converter, MediaType.APPLICATION_JSON)) { - builder.withJsonConverter(converter); - } - else if (supportsMediaType(converter, MediaType.APPLICATION_XML)) { - builder.withXmlConverter(converter); + EnumSet registered = EnumSet.noneOf(ConverterType.class); + for (HttpMessageConverter converter : this.converters) { + ConverterType type = findConverterType(converter); + if (type != null) { + if (!registered.contains(type)) { + type.registerWith(builder, converter); + registered.add(type); + } + else { + builder.addCustomConverter(converter); + } } else { builder.addCustomConverter(converter); } - }); + } + } + } + + private static @Nullable ConverterType findConverterType(HttpMessageConverter converter) { + for (ConverterType type : ConverterType.values()) { + if (type.matches(converter)) { + return type; + } } + return null; } private static boolean supportsMediaType(HttpMessageConverter converter, MediaType mediaType) { @@ -76,4 +87,36 @@ private static boolean supportsMediaType(HttpMessageConverter converter, Medi return false; } + private enum ConverterType { + + STRING(StringHttpMessageConverter.class::isInstance, ClientBuilder::withStringConverter), + + KOTLIN_SERIALIZATION_JSON(KotlinSerializationJsonHttpMessageConverter.class::isInstance, + ClientBuilder::withKotlinSerializationJsonConverter), + + JSON(converter -> supportsMediaType(converter, MediaType.APPLICATION_JSON), + ClientBuilder::withJsonConverter), + + XML(converter -> supportsMediaType(converter, MediaType.APPLICATION_XML), ClientBuilder::withXmlConverter); + + private final Predicate> matcher; + + private final BiConsumer> registrar; + + ConverterType(Predicate> matcher, + BiConsumer> registrar) { + this.matcher = matcher; + this.registrar = registrar; + } + + boolean matches(HttpMessageConverter converter) { + return this.matcher.test(converter); + } + + void registerWith(ClientBuilder builder, HttpMessageConverter converter) { + this.registrar.accept(builder, converter); + } + + } + } diff --git a/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizer.java b/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizer.java index 2f4eed708cbd..90af514879e5 100644 --- a/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizer.java +++ b/module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizer.java @@ -17,6 +17,9 @@ package org.springframework.boot.http.converter.autoconfigure; import java.util.Collection; +import java.util.EnumSet; +import java.util.function.BiConsumer; +import java.util.function.Predicate; import org.jspecify.annotations.Nullable; @@ -47,24 +50,32 @@ public void customize(ServerBuilder builder) { } else { builder.registerDefaults(); - this.converters.forEach((converter) -> { - if (converter instanceof StringHttpMessageConverter) { - builder.withStringConverter(converter); - } - else if (converter instanceof KotlinSerializationJsonHttpMessageConverter) { - builder.withKotlinSerializationJsonConverter(converter); - } - else if (supportsMediaType(converter, MediaType.APPLICATION_JSON)) { - builder.withJsonConverter(converter); - } - else if (supportsMediaType(converter, MediaType.APPLICATION_XML)) { - builder.withXmlConverter(converter); + EnumSet registered = EnumSet.noneOf(ConverterType.class); + for (HttpMessageConverter converter : this.converters) { + ConverterType type = findConverterType(converter); + if (type != null) { + if (!registered.contains(type)) { + type.registerWith(builder, converter); + registered.add(type); + } + else { + builder.addCustomConverter(converter); + } } else { builder.addCustomConverter(converter); } - }); + } + } + } + + private static @Nullable ConverterType findConverterType(HttpMessageConverter converter) { + for (ConverterType type : ConverterType.values()) { + if (type.matches(converter)) { + return type; + } } + return null; } private static boolean supportsMediaType(HttpMessageConverter converter, MediaType mediaType) { @@ -76,4 +87,36 @@ private static boolean supportsMediaType(HttpMessageConverter converter, Medi return false; } + private enum ConverterType { + + STRING(StringHttpMessageConverter.class::isInstance, ServerBuilder::withStringConverter), + + KOTLIN_SERIALIZATION_JSON(KotlinSerializationJsonHttpMessageConverter.class::isInstance, + ServerBuilder::withKotlinSerializationJsonConverter), + + JSON(converter -> supportsMediaType(converter, MediaType.APPLICATION_JSON), + ServerBuilder::withJsonConverter), + + XML(converter -> supportsMediaType(converter, MediaType.APPLICATION_XML), ServerBuilder::withXmlConverter); + + private final Predicate> matcher; + + private final BiConsumer> registrar; + + ConverterType(Predicate> matcher, + BiConsumer> registrar) { + this.matcher = matcher; + this.registrar = registrar; + } + + boolean matches(HttpMessageConverter converter) { + return this.matcher.test(converter); + } + + void registerWith(ServerBuilder builder, HttpMessageConverter converter) { + this.registrar.accept(builder, converter); + } + + } + } diff --git a/module/spring-boot-http-converter/src/test/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizerTests.java b/module/spring-boot-http-converter/src/test/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizerTests.java new file mode 100644 index 000000000000..783e544ff2ef --- /dev/null +++ b/module/spring-boot-http-converter/src/test/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizerTests.java @@ -0,0 +1,131 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.http.converter.autoconfigure; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import org.springframework.http.MediaType; +import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.http.converter.HttpMessageConverters.ServerBuilder; +import org.springframework.http.converter.json.GsonHttpMessageConverter; +import org.springframework.http.converter.json.JacksonJsonHttpMessageConverter; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; + +/** + * Tests for {@link DefaultServerHttpMessageConvertersCustomizer}. + * + * @author Tommy Karlsson + */ +@SuppressWarnings("deprecation") +class DefaultServerHttpMessageConvertersCustomizerTests { + + @Test + void customizeWithTwoJsonConvertersConfiguresFirstAsJsonConverterAndSecondAsCustomConverter() { + // Create two JSON converters + JacksonJsonHttpMessageConverter jsonConverter1 = new JacksonJsonHttpMessageConverter(); + GsonHttpMessageConverter jsonConverter2 = new GsonHttpMessageConverter(); + + // Create the customizer with both converters + List> converters = List.of(jsonConverter1, jsonConverter2); + DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null, + converters); + + // Mock the ServerBuilder + ServerBuilder builder = mock(ServerBuilder.class); + + // Execute the customize method + customizer.customize(builder); + + // Verify that registerDefaults was called + then(builder).should().registerDefaults(); + + // Verify that the first JSON converter is configured as the JSON converter + then(builder).should(times(1)).withJsonConverter(jsonConverter1); + + // Verify that the second JSON converter is configured as a custom converter + // (since there should only be one JSON converter registered via withJsonConverter) + then(builder).should(times(1)).addCustomConverter(jsonConverter2); + + // Verify that withJsonConverter was only called once (for the first converter) + then(builder).should(times(1)).withJsonConverter(any()); + } + + @Test + void customizeWithJsonConverterVerifiesJsonConverterConfiguration() { + // Create a single JSON converter + JacksonJsonHttpMessageConverter jsonConverter = new JacksonJsonHttpMessageConverter(); + + // Verify it supports JSON + boolean supportsJson = jsonConverter.getSupportedMediaTypes() + .stream() + .anyMatch(mediaType -> mediaType.equalsTypeAndSubtype(MediaType.APPLICATION_JSON)); + assert supportsJson : "Converter should support APPLICATION_JSON"; + + // Create the customizer + List> converters = List.of(jsonConverter); + DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null, + converters); + + // Mock the ServerBuilder + ServerBuilder builder = mock(ServerBuilder.class); + + // Execute the customize method + customizer.customize(builder); + + // Verify that registerDefaults was called + then(builder).should().registerDefaults(); + + // Verify that withJsonConverter was called with the JSON converter + then(builder).should().withJsonConverter(jsonConverter); + + // Verify that addCustomConverter was not called for the JSON converter + then(builder).should(times(0)).addCustomConverter(any()); + } + + @Test + void customizeWithNonJsonConverterConfiguresAsCustomConverter() { + // Create a custom converter that doesn't support JSON + HttpMessageConverter customConverter = mock(HttpMessageConverter.class); + + // Create the customizer + List> converters = List.of(customConverter); + DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null, + converters); + + // Mock the ServerBuilder + ServerBuilder builder = mock(ServerBuilder.class); + + // Execute the customize method + customizer.customize(builder); + + // Verify that registerDefaults was called + then(builder).should().registerDefaults(); + + // Verify that addCustomConverter was called with the custom converter + then(builder).should().addCustomConverter(customConverter); + + // Verify that withJsonConverter was not called + then(builder).should(times(0)).withJsonConverter(any()); + } + +} \ No newline at end of file