From 1e98fb607a274fc3cc49c24902d7c269f2946514 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 Spring web applications now needs to be 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 975f6feb1fe..8592987e388 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.protobuf.KotlinSerializationProtobufHt 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 8c3935f4107..a6ddae02509 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.smile.MappingJackson2SmileHttpMessageC 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 class RestTemplate extends InterceptingHttpAccessor implements RestOperat 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 57da8a22e1f..cc03f25e7aa 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.MediaType; 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 @@ import static org.springframework.http.MediaType.TEXT_XML; * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Sam Brannen + * @author Sebastien Deleuze */ public class FormHttpMessageConverterTests { @@ -155,6 +158,90 @@ public class FormHttpMessageConverterTests { @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 8ee86bae790..34aefb1d4d0 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.smile.MappingJackson2SmileHttpMessageC 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 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser { 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 9a3f5da09de..198f4492ed7 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.smile.MappingJackson2SmileHttpMessageC 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 @@ public class WebMvcConfigurationSupport implements ApplicationContextAware, Serv messageConverters.add(new StringHttpMessageConverter()); messageConverters.add(new ResourceHttpMessageConverter()); messageConverters.add(new ResourceRegionHttpMessageConverter()); - try { - messageConverters.add(new SourceHttpMessageConverter<>()); - } - 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 5f349f8b4a4..f67ea7e2e69 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.ByteArrayHttpMessageConverter; 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 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini 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 d7c2767fc25..0588f57fe5a 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.ByteArrayHttpMessageConverter; 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 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce } 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 74f02955c8e..b9429cc841d 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.ByteArrayHttpMessageConverter; 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 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter } 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 2d7996b5aed..964ec7b4a1d 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 class WebMvcConfigurationSupportTests { 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 -> {