From d5657a53bad7747a6febbcee121c9186beb9bd25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Mon, 21 Nov 2022 16:36:37 +0100 Subject: [PATCH] Make SourceHttpMessageConverter optional As a follow-up to gh-29277, and since the JAXB support is now triggered by the classpath presence of a JAXB implementation, it makes sense to make SourceHttpMessageConverter, previously configured unconditionally, optional. That makes a big difference on native (1M of RSS reduction with current typical Spring Boot 3 arrangement, 3.4M when other usages of XML are not reachable). It also brings more consistency between Spring MVC and Spring WebFlux, and means that XML support for web applications now needs to enabled explicitly. As a consequence, Spring web applications using javax.xml.transform.Source now needs to configure SourceHttpMessageConverter explicitly in RestTemplate or Spring MVC. Closes gh-29535 --- ...lEncompassingFormHttpMessageConverter.java | 8 -- .../web/client/RestTemplate.java | 8 -- .../FormHttpMessageConverterTests.java | 87 +++++++++++++++++++ .../AnnotationDrivenBeanDefinitionParser.java | 2 - .../WebMvcConfigurationSupport.java | 8 -- .../support/RouterFunctionMapping.java | 7 -- .../ExceptionHandlerExceptionResolver.java | 7 -- .../RequestMappingHandlerAdapter.java | 7 -- .../WebMvcConfigurationSupportTests.java | 2 +- 9 files changed, 88 insertions(+), 48 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/support/AllEncompassingFormHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/support/AllEncompassingFormHttpMessageConverter.java index 975f6feb1fe4..8592987e3887 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/support/AllEncompassingFormHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/support/AllEncompassingFormHttpMessageConverter.java @@ -26,7 +26,6 @@ import org.springframework.http.converter.smile.MappingJackson2SmileHttpMessageConverter; import org.springframework.http.converter.xml.Jaxb2RootElementHttpMessageConverter; import org.springframework.http.converter.xml.MappingJackson2XmlHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.util.ClassUtils; /** @@ -75,13 +74,6 @@ public class AllEncompassingFormHttpMessageConverter extends FormHttpMessageConv public AllEncompassingFormHttpMessageConverter() { - try { - addPartConverter(new SourceHttpMessageConverter<>()); - } - catch (Error err) { - // Ignore when no TransformerFactory implementation is available - } - if (jaxb2Present && !jackson2XmlPresent) { addPartConverter(new Jaxb2RootElementHttpMessageConverter()); } diff --git a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java index 8c3935f41076..a6ddae025093 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java @@ -66,7 +66,6 @@ import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; import org.springframework.http.converter.xml.Jaxb2RootElementHttpMessageConverter; import org.springframework.http.converter.xml.MappingJackson2XmlHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -173,13 +172,6 @@ public RestTemplate() { this.messageConverters.add(new StringHttpMessageConverter()); this.messageConverters.add(new ResourceHttpMessageConverter(false)); - try { - this.messageConverters.add(new SourceHttpMessageConverter<>()); - } - catch (Error err) { - // Ignore when no TransformerFactory implementation is available - } - this.messageConverters.add(new AllEncompassingFormHttpMessageConverter()); if (romePresent) { diff --git a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java index 57da8a22e1f1..cc03f25e7aae 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java @@ -44,11 +44,13 @@ import org.springframework.http.MockHttpInputMessage; import org.springframework.http.MockHttpOutputMessage; import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; +import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; +import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.http.MediaType.MULTIPART_FORM_DATA; import static org.springframework.http.MediaType.MULTIPART_MIXED; import static org.springframework.http.MediaType.TEXT_XML; @@ -60,6 +62,7 @@ * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Sam Brannen + * @author Sebastien Deleuze */ public class FormHttpMessageConverterTests { @@ -155,6 +158,90 @@ public void writeForm() throws IOException { @Test public void writeMultipart() throws Exception { + + MultiValueMap parts = new LinkedMultiValueMap<>(); + parts.add("name 1", "value 1"); + parts.add("name 2", "value 2+1"); + parts.add("name 2", "value 2+2"); + parts.add("name 3", null); + + Resource logo = new ClassPathResource("/org/springframework/http/converter/logo.jpg"); + parts.add("logo", logo); + + // SPR-12108 + Resource utf8 = new ClassPathResource("/org/springframework/http/converter/logo.jpg") { + @Override + public String getFilename() { + return "Hall\u00F6le.jpg"; + } + }; + parts.add("utf8", utf8); + + MyBean myBean = new MyBean(); + myBean.setString("foo"); + HttpHeaders entityHeaders = new HttpHeaders(); + entityHeaders.setContentType(APPLICATION_JSON); + HttpEntity entity = new HttpEntity<>(myBean, entityHeaders); + parts.add("json", entity); + + Map parameters = new LinkedHashMap<>(2); + parameters.put("charset", StandardCharsets.UTF_8.name()); + parameters.put("foo", "bar"); + + MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); + this.converter.write(parts, new MediaType("multipart", "form-data", parameters), outputMessage); + + final MediaType contentType = outputMessage.getHeaders().getContentType(); + assertThat(contentType.getParameters()).containsKeys("charset", "boundary", "foo"); // gh-21568, gh-25839 + + // see if Commons FileUpload can read what we wrote + FileUpload fileUpload = new FileUpload(); + fileUpload.setFileItemFactory(new DiskFileItemFactory()); + RequestContext requestContext = new MockHttpOutputMessageRequestContext(outputMessage); + List items = fileUpload.parseRequest(requestContext); + assertThat(items.size()).isEqualTo(6); + FileItem item = items.get(0); + assertThat(item.isFormField()).isTrue(); + assertThat(item.getFieldName()).isEqualTo("name 1"); + assertThat(item.getString()).isEqualTo("value 1"); + + item = items.get(1); + assertThat(item.isFormField()).isTrue(); + assertThat(item.getFieldName()).isEqualTo("name 2"); + assertThat(item.getString()).isEqualTo("value 2+1"); + + item = items.get(2); + assertThat(item.isFormField()).isTrue(); + assertThat(item.getFieldName()).isEqualTo("name 2"); + assertThat(item.getString()).isEqualTo("value 2+2"); + + item = items.get(3); + assertThat(item.isFormField()).isFalse(); + assertThat(item.getFieldName()).isEqualTo("logo"); + assertThat(item.getName()).isEqualTo("logo.jpg"); + assertThat(item.getContentType()).isEqualTo("image/jpeg"); + assertThat(item.getSize()).isEqualTo(logo.getFile().length()); + + item = items.get(4); + assertThat(item.isFormField()).isFalse(); + assertThat(item.getFieldName()).isEqualTo("utf8"); + assertThat(item.getName()).isEqualTo("Hall\u00F6le.jpg"); + assertThat(item.getContentType()).isEqualTo("image/jpeg"); + assertThat(item.getSize()).isEqualTo(logo.getFile().length()); + + item = items.get(5); + assertThat(item.getFieldName()).isEqualTo("json"); + assertThat(item.getContentType()).isEqualTo("application/json"); + } + + @Test + public void writeMultipartWithSourceHttpMessageConverter() throws Exception { + + converter.setPartConverters(List.of( + new StringHttpMessageConverter(), + new ResourceHttpMessageConverter(), + new SourceHttpMessageConverter<>())); + MultiValueMap parts = new LinkedMultiValueMap<>(); parts.add("name 1", "value 1"); parts.add("name 2", "value 2+1"); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java index 8ee86bae790a..34aefb1d4d03 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java @@ -55,7 +55,6 @@ import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; import org.springframework.http.converter.xml.Jaxb2RootElementHttpMessageConverter; import org.springframework.http.converter.xml.MappingJackson2XmlHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -569,7 +568,6 @@ private ManagedList getMessageConverters(Element element, @Nullable Object so messageConverters.add(createConverterDefinition(ResourceHttpMessageConverter.class, source)); messageConverters.add(createConverterDefinition(ResourceRegionHttpMessageConverter.class, source)); - messageConverters.add(createConverterDefinition(SourceHttpMessageConverter.class, source)); messageConverters.add(createConverterDefinition(AllEncompassingFormHttpMessageConverter.class, source)); if (romePresent) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java index 9a3f5da09dea..198f4492ed74 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java @@ -58,7 +58,6 @@ import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; import org.springframework.http.converter.xml.Jaxb2RootElementHttpMessageConverter; import org.springframework.http.converter.xml.MappingJackson2XmlHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; @@ -880,13 +879,6 @@ protected final void addDefaultHttpMessageConverters(List()); - } - catch (Throwable ex) { - // Ignore when no TransformerFactory implementation is available... - } - messageConverters.add(new AllEncompassingFormHttpMessageConverter()); if (romePresent) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java index 5f349f8b4a41..f67ea7e2e692 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java @@ -29,7 +29,6 @@ import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; import org.springframework.web.filter.ServerHttpObservationFilter; @@ -189,12 +188,6 @@ private void initMessageConverters() { List> messageConverters = new ArrayList<>(4); messageConverters.add(new ByteArrayHttpMessageConverter()); messageConverters.add(new StringHttpMessageConverter()); - try { - messageConverters.add(new SourceHttpMessageConverter<>()); - } - catch (Error err) { - // Ignore when no TransformerFactory implementation is available - } messageConverters.add(new AllEncompassingFormHttpMessageConverter()); this.messageConverters = messageConverters; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java index d7c2767fc255..0588f57fe5a8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java @@ -37,7 +37,6 @@ import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.ui.ModelMap; import org.springframework.web.accept.ContentNegotiationManager; @@ -260,12 +259,6 @@ private void initMessageConverters() { } this.messageConverters.add(new ByteArrayHttpMessageConverter()); this.messageConverters.add(new StringHttpMessageConverter()); - try { - this.messageConverters.add(new SourceHttpMessageConverter<>()); - } - catch (Error err) { - // Ignore when no TransformerFactory implementation is available - } this.messageConverters.add(new AllEncompassingFormHttpMessageConverter()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 74f02955c8ee..b9429cc841db 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -46,7 +46,6 @@ import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; import org.springframework.lang.Nullable; import org.springframework.ui.ModelMap; import org.springframework.util.CollectionUtils; @@ -568,12 +567,6 @@ private void initMessageConverters() { } this.messageConverters.add(new ByteArrayHttpMessageConverter()); this.messageConverters.add(new StringHttpMessageConverter()); - try { - this.messageConverters.add(new SourceHttpMessageConverter<>()); - } - catch (Error err) { - // Ignore when no TransformerFactory implementation is available - } this.messageConverters.add(new AllEncompassingFormHttpMessageConverter()); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java index 2d7996b5aeda..964ec7b4a1de 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java @@ -173,7 +173,7 @@ public void requestMappingHandlerAdapter() { ApplicationContext context = initContext(WebConfig.class); RequestMappingHandlerAdapter adapter = context.getBean(RequestMappingHandlerAdapter.class); List> converters = adapter.getMessageConverters(); - assertThat(converters).hasSizeGreaterThanOrEqualTo(15); + assertThat(converters).hasSizeGreaterThanOrEqualTo(14); converters.stream() .filter(converter -> converter instanceof AbstractJackson2HttpMessageConverter) .forEach(converter -> {