Skip to content

Commit ad007ca

Browse files
committed
Refine multipart converter registration in HttpMessageConverters
As of #33894, `HttpMessageConverters` auto-detects converters and use custom-provided ones to configure a collection of converters for the client or the server. Right now the multipart converter is only configured if core converters (JSON, XML...) are configured/detected. We do not reuse the base converters (resource, string, byte array) for the multipart converter as it applies different encoding defaults (ISO for the main ones, UTF-8 for multipart). This commit refines the configuration to not only include the multipart converter when core converters are present, but also if any other converter was configured. Closes gh-35203
1 parent 2e0cc63 commit ad007ca

File tree

3 files changed

+34
-11
lines changed

3 files changed

+34
-11
lines changed

spring-web/src/main/java/org/springframework/http/converter/DefaultHttpMessageConverters.java

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
package org.springframework.http.converter;
1818

1919
import java.util.ArrayList;
20-
import java.util.Arrays;
21-
import java.util.Collections;
2220
import java.util.Iterator;
2321
import java.util.List;
2422
import java.util.function.Consumer;
@@ -110,7 +108,9 @@ abstract static class DefaultBuilder {
110108

111109
@Nullable HttpMessageConverter<?> stringMessageConverter;
112110

113-
List<HttpMessageConverter<?>> resourceMessageConverters = Collections.emptyList();
111+
@Nullable HttpMessageConverter<?> resourceMessageConverter;
112+
113+
@Nullable HttpMessageConverter<?> resourceRegionMessageConverter;
114114

115115
@Nullable Consumer<HttpMessageConverter<?>> configurer;
116116

@@ -386,7 +386,7 @@ public ClientBuilder configureMessageConverters(Consumer<HttpMessageConverter<?>
386386
@Override
387387
public HttpMessageConverters build() {
388388
if (this.registerDefaults) {
389-
this.resourceMessageConverters = Collections.singletonList(new ResourceHttpMessageConverter(false));
389+
this.resourceMessageConverter = new ResourceHttpMessageConverter(false);
390390
detectMessageConverters();
391391
}
392392
List<HttpMessageConverter<?>> allConverters = new ArrayList<>();
@@ -396,8 +396,10 @@ public HttpMessageConverters build() {
396396

397397
allConverters.addAll(this.getCustomConverters());
398398
allConverters.addAll(this.getBaseConverters());
399-
allConverters.addAll(this.resourceMessageConverters);
400-
if (!partConverters.isEmpty()) {
399+
if (this.resourceMessageConverter != null) {
400+
allConverters.add(this.resourceMessageConverter);
401+
}
402+
if (!partConverters.isEmpty() || !allConverters.isEmpty()) {
401403
allConverters.add(new AllEncompassingFormHttpMessageConverter(partConverters));
402404
}
403405
allConverters.addAll(this.getCoreConverters());
@@ -468,7 +470,8 @@ public ServerBuilder configureMessageConverters(Consumer<HttpMessageConverter<?>
468470
@Override
469471
public HttpMessageConverters build() {
470472
if (this.registerDefaults) {
471-
this.resourceMessageConverters = Arrays.asList(new ResourceHttpMessageConverter(), new ResourceRegionHttpMessageConverter());
473+
this.resourceMessageConverter = new ResourceHttpMessageConverter();
474+
this.resourceRegionMessageConverter = new ResourceRegionHttpMessageConverter();
472475
detectMessageConverters();
473476
}
474477
List<HttpMessageConverter<?>> allConverters = new ArrayList<>();
@@ -479,8 +482,13 @@ public HttpMessageConverters build() {
479482

480483
allConverters.addAll(this.getCustomConverters());
481484
allConverters.addAll(this.getBaseConverters());
482-
allConverters.addAll(this.resourceMessageConverters);
483-
if (!partConverters.isEmpty()) {
485+
if (this.resourceMessageConverter != null) {
486+
allConverters.add(this.resourceMessageConverter);
487+
}
488+
if (this.resourceRegionMessageConverter != null) {
489+
allConverters.add(this.resourceRegionMessageConverter);
490+
}
491+
if (!partConverters.isEmpty() || !allConverters.isEmpty()) {
484492
allConverters.add(new AllEncompassingFormHttpMessageConverter(partConverters));
485493
}
486494
allConverters.addAll(this.getCoreConverters());

spring-web/src/test/java/org/springframework/http/converter/DefaultHttpMessageConvertersTests.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,13 @@ void registerCustomConverterInMultipartConverter() {
160160
assertThat(multipartConverter.getPartConverters()).hasAtLeastOneElementOfType(CustomHttpMessageConverter.class);
161161
}
162162

163+
@Test
164+
void registerMultipartConverterWhenOtherConvertersPresent() {
165+
var converters = HttpMessageConverters.forClient()
166+
.stringMessageConverter(new StringHttpMessageConverter()).build();
167+
assertThat(converters).hasExactlyElementsOfTypes(StringHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
168+
}
169+
163170
@Test
164171
void shouldUseSpecificConverter() {
165172
var jacksonConverter = new JacksonJsonHttpMessageConverter();
@@ -248,7 +255,14 @@ void registerCustomConverterInMultipartConverter() {
248255
}
249256

250257
@Test
251-
void shouldUseServerSpecificConverter() {
258+
void registerMultipartConverterWhenOtherConvertersPresent() {
259+
var converters = HttpMessageConverters.forServer()
260+
.stringMessageConverter(new StringHttpMessageConverter()).build();
261+
assertThat(converters).hasExactlyElementsOfTypes(StringHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
262+
}
263+
264+
@Test
265+
void shouldUseSpecificConverter() {
252266
var jacksonConverter = new JacksonJsonHttpMessageConverter();
253267
var converters = HttpMessageConverters.forServer().registerDefaults()
254268
.jsonMessageConverter(jacksonConverter).build();

spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.http.client.support.BasicAuthenticationInterceptor;
3434
import org.springframework.http.converter.HttpMessageConverter;
3535
import org.springframework.http.converter.StringHttpMessageConverter;
36+
import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter;
3637
import org.springframework.web.util.DefaultUriBuilderFactory;
3738

3839
import static org.assertj.core.api.Assertions.assertThat;
@@ -150,7 +151,7 @@ void configureMessageConverters() {
150151

151152
assertThat(fieldValue("messageConverters", defaultBuilder))
152153
.asInstanceOf(InstanceOfAssertFactories.LIST)
153-
.containsExactly(stringConverter);
154+
.hasExactlyElementsOfTypes(StringHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
154155
}
155156

156157
@Test

0 commit comments

Comments
 (0)