Support multipart/related and multipart/mixed MediaTypes in RestTemplate

[sethcleveland]

Affects: <v5.2.0.M3>

At the moment, the RestTemplate posts multipart with Content-Type: multipart/form-data.

We have a requirement to post multipart data with a Content-Type of multipart/mixed or multipart/related.

C# provides flexibility for this with a MultipartContent with a subtype parameter.

https://docs.microsoft.com/en-us/dotnet/api/system.net.http.multipartcontent.-ctor?view=netframework-4.8#System_Net_Http_MultipartContent__ctor_System_String_

Any recommendations?

[sbrannen]

Any recommendations?

What happens if you configure explicit support for those content types as follows?

restTemplate.getMessageConverters().stream()
	.filter(FormHttpMessageConverter.class::isInstance)
	.map(FormHttpMessageConverter.class::cast)
	.findFirst()
	.ifPresent(converter -> {
		List<MediaType> supportedMediaTypes = new ArrayList<>(converter.getSupportedMediaTypes());
		supportedMediaTypes.add(new MediaType("multipart", "mixed"));
		supportedMediaTypes.add(new MediaType("multipart", "related"));
		converter.setSupportedMediaTypes(supportedMediaTypes);
	});

[sethcleveland]

That was our first inclination; however, it didn't work because of another check in the FormHttpMessageConverter, specifically this one:

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

Lines 279 to 291 in da582da

	private boolean isMultipart(MultiValueMap<String, ?> map, @Nullable MediaType contentType) {
	if (contentType != null) {
	return MediaType.MULTIPART_FORM_DATA.includes(contentType);
	}
	for (List<?> values : map.values()) {
	for (Object value : values) {
	if (value != null && !(value instanceof String)) {
	return true;
	}
	}
	}
	return false;
	}

The isMultipart method is coded to only check for multipart/form-data. For the proposed approach to work for us, it needs to be coded such that multipart/* is supported.

[sbrannen]

That was our first inclination; however, it didn't work because of another check in the FormHttpMessageConverter

Thanks for the feedback.

The isMultipart method is coded to only check for multipart/form-data. For the proposed approach to work for us, it needs to be coded such that multipart/* is supported.

@rstoyanchev, what do you think about making changes along those lines?

Also, what do you think about introducing MULTIPART_MIXED and MULTIPART_RELATED constants in MediaType?

[rstoyanchev]

As far as I can see, there are a couple of checks against MediaType.MULTIPART_FORM_DATA (one includes and one equals), and one case where MediaType.MULTIPART_FORM_DATA is set on the OutputMessage in the writeMultipart method.

The checks could probably be 1) relaxed to check only the type ("multipart"), 2) or changed to explicitly check all 3 (i.e. form-data, mixed, related), or 3) invert the check, i.e. that it's not APPLICATION_FORM_URLENCODED. I would think either 1) or 3) are sufficient in which case there would be no need to add mixed and related as constants.

Likewise for setting the multipart media type on the OutputMessage we would have to look at the supported media types and pick the first one that has multipart as the type. This could be done at initialization as well.

Taking both the checks and the setting of headers together, it's probably best to use the multipart type as the trigger for differentiating which supported media type is for multipart requests.

[sbrannen]

Thanks for the feedback, @rstoyanchev.

I'll look into implementing something along those lines.

[sbrannen]

Thanks to #23203, it's now a little easier to register custom supported MediaType objects with the FormHttpMessageConverter in the RestTemplate:

restTemplate.getMessageConverters().stream()
		.filter(FormHttpMessageConverter.class::isInstance)
		.map(FormHttpMessageConverter.class::cast)
		.findFirst()
		.orElseThrow(() -> new IllegalStateException("Failed to find FormHttpMessageConverter"))
		.addSupportedMediaTypes(new MediaType("multipart", "mixed"));

[sbrannen]

Current work on this issue can be viewed in my feature branch: https://github.com/sbrannen/spring-framework/commits/issues/gh-23159-RestTemplate-multipart

[sbrannen]

As far as I can see, there are a couple of checks against MediaType.MULTIPART_FORM_DATA (one includes and one equals), and one case where MediaType.MULTIPART_FORM_DATA is set on the OutputMessage in the writeMultipart method.

The checks could probably be 1) relaxed to check only the type ("multipart"), 2) or changed to explicitly check all 3 (i.e. form-data, mixed, related), or 3) invert the check, i.e. that it's not APPLICATION_FORM_URLENCODED. I would think either 1) or 3) are sufficient in which case there would be no need to add mixed and related as constants.

For the check in isMultipart(), I've gone with option # 1, with a MULTIPART_ALL = new MediaType("multipart", "*") constant that I used for the includes check.

Likewise for setting the multipart media type on the OutputMessage we would have to look at the supported media types and pick the first one that has multipart as the type. This could be done at initialization as well.

I've done this slightly differently in my feature branch. The code now honors the contentType supplied by the user (e.g., a custom Content-Type header supplied via a request HttpEntity which you can see in my tests).

If the user doesn't supply an explicit content type — for example, by invoking postForLocation(...) without an HttpEntity — we still fall back to multipart/form-data.

This works fine if the user has properly added the desired MediaType as I've shown in #23159 (comment).

@rstoyanchev, do you think that is OK like that? Or do feel strongly about transparently picking the current content type from the configured set of supported types?

I'm a little hesitant to do the latter, b/c that makes it a bit of a guessing game for the user. Until now, we have only supported multipart/form-data (with or without a user-supplied content-type for the request). So I'd like to keep that behavior while allowing the user to configure additional multipart media types for the same RestTemplate/FormHttpMessageConverter pair. Otherwise, the user would need to have a dedicated RestTemplate/FormHttpMessageConverter pair for each multipart type that needs to be supported.

Or am I missing something?

[rstoyanchev]

Looks good overall although there is one more explicit check against MULTIPART_FORM_DATA that should probably also be generalized.

[sbrannen]

Looks good overall

Thanks for the review.

although there is one more explicit check against MULTIPART_FORM_DATA that should probably also be generalized.

Indeed. Good catch!

I've addressed that locally as follows and added appropriate tests that I'll be pushing later.

for (MediaType supportedMediaType : getSupportedMediaTypes()) {
	if (MULTIPART_ALL.includes(supportedMediaType)) {
		// We can't read multipart, so skip this supported media type.
		continue;
	}
	if (supportedMediaType.includes(mediaType)) {
		return true;
	}
}

[sbrannen]

This support has been added in 5008423.

@sethcleveland, feel free to try it out in the next 5.2 snapshot and let us know if it works for you.

[sbrannen]

See also: #23209