Skip to content

Commit

Permalink
Make SourceHttpMessageConverter optional
Browse files Browse the repository at this point in the history 
As a follow-up to spring-projectsgh-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 spring-projectsgh-29535
  • Loading branch information
@sdeleuze
sdeleuze committed Nov 21, 2022
1 parent c1dfde5 commit d5657a5
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 48 deletions.
8 changes: 0 additions & 8 deletions ...a/org/springframework/http/converter/support/AllEncompassingFormHttpMessageConverter.java
View file
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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());
}
Expand Down
8 changes: 0 additions & 8 deletions spring-web/src/main/java/org/springframework/web/client/RestTemplate.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
87 changes: 87 additions & 0 deletions ...g-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java
View file
Expand Up @@ -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;
Expand All @@ -60,6 +62,7 @@
* @author Arjen Poutsma
* @author Rossen Stoyanchev
* @author Sam Brannen
* @author Sebastien Deleuze
*/
public class FormHttpMessageConverterTests {

Expand Down Expand Up @@ -155,6 +158,90 @@ public void writeForm() throws IOException {

@Test
public void writeMultipart() throws Exception {

MultiValueMap<String, Object> 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<MyBean> entity = new HttpEntity<>(myBean, entityHeaders);
parts.add("json", entity);

Map<String, String> 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<FileItem> 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<String, Object> parts = new LinkedMultiValueMap<>();
parts.add("name 1", "value 1");
parts.add("name 2", "value 2+1");
Expand Down
2 changes: 0 additions & 2 deletions ...ain/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 0 additions & 8 deletions ...in/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -880,13 +879,6 @@ protected final void addDefaultHttpMessageConverters(List<HttpMessageConverter<?
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) {
Expand Down
7 changes: 0 additions & 7 deletions ...src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -189,12 +188,6 @@ private void initMessageConverters() {
List<HttpMessageConverter<?>> 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;
Expand Down
7 changes: 0 additions & 7 deletions .../springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down
7 changes: 0 additions & 7 deletions ...a/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java
View file
Expand Up @@ -173,7 +173,7 @@ public void requestMappingHandlerAdapter() {
ApplicationContext context = initContext(WebConfig.class);
RequestMappingHandlerAdapter adapter = context.getBean(RequestMappingHandlerAdapter.class);
List<HttpMessageConverter<?>> converters = adapter.getMessageConverters();
assertThat(converters).hasSizeGreaterThanOrEqualTo(15);
assertThat(converters).hasSizeGreaterThanOrEqualTo(14);
converters.stream()
.filter(converter -> converter instanceof AbstractJackson2HttpMessageConverter)
.forEach(converter -> {
Expand Down

0 comments on commit d5657a5

Please sign in to comment.