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).

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 a1cc074
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 a1cc074

Please sign in to comment.