resttemplate multipart post with InputStreamResource not working [SPR-13571]

[spring-projects-issues]

Greg Adams opened SPR-13571 and commented

I've been trying to send a multipart post via restTemplate and have been unable to get it to work with anything but FileSystemResource. In my use case (a weird file-forwarding use case) this forces me to copy a MultiPartFile InputStream into a temp file in order be able to create a FileSystemResource, which seems undesirable.

Here's a testing version of the file-receiving controller (from another project, running in another servlet container):

@RestController
public class FileReceiveController {
	
	private Log log = LogFactory.getLog(FileReceiveController.class);

	@RequestMapping(method = RequestMethod.POST)
	public void uploadFile(@RequestParam("customerId") int customerId, @RequestPart("file") MultipartFile multipartFile) {
		log.info("customerId: " + customerId);
		log.info("Received multipart file - original filename: " + multipartFile.getOriginalFilename());
		log.info("content-type: " + multipartFile.getContentType());
		log.info("size: " + multipartFile.getSize());
	}
}

Here's the file-forwarding controller:

@RestController
public class FileForwardController {
	
	private RestTemplate restTemplate;
	
	public FileForwardController() {
		SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
		requestFactory.setBufferRequestBody(false);
		this.restTemplate = new RestTemplate(requestFactory);
	}

	@RequestMapping(method = RequestMethod.POST)
	public void uploadFile(@RequestParam("customerId") int customerId, @RequestPart("file") MultipartFile multipartFile) {
		MultiValueMap<String,Object> parts = new LinkedMultiValueMap<>();
		parts.add("customerId", customerId);
		try {
			// copy to temp file and use FileSystemResource
//			File tempFile = File.createTempFile("xyz", "");
//			FileCopyUtils.copy(multipartFile.getInputStream(), new FileOutputStream(tempFile));
//			parts.add("file", new FileSystemResource(tempFile));
			// OR use InputStreamResource (broken)
			parts.add("file", new InputStreamResource(multipartFile.getInputStream()));
			// OR use ByteArrayResource (broken)
//			parts.add("file", new ByteArrayResource(multipartFile.getBytes()));
		} catch (IOException e) {
			throw new RuntimeException(e);
		}
		HttpHeaders headers = new HttpHeaders();
		headers.setContentType(MediaType.MULTIPART_FORM_DATA);
		HttpEntity<MultiValueMap<String,Object>> request = new HttpEntity<>(parts, headers);
		restTemplate.exchange("http://localhost:8080", HttpMethod.POST, request, Void.class);
	}
}

In this form, the restTemplate.exchange call throws

org.springframework.web.client.HttpClientErrorException: 400 Bad Request
	at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:91)
	at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:614)
	at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:570)

The ByteArrayResource form does the same thing. Only the FileSystemResource form works.

---

Affects: 4.1.7

Issue Links:

• Read large data using InputStreamResource at ResourceHttpMessageConverter [SPR-13443] #18023 Read large data using InputStreamResource at ResourceHttpMessageConverter
• RestTemplate with InputStreamResource does not work if Content-Length is not set [SPR-12017] #16633 RestTemplate with InputStreamResource does not work if Content-Length is not set
• HTTP Response should not contain both Transfer-Encoding and Content-Length headers [SPR-15212] #19776 HTTP Response should not contain both Transfer-Encoding and Content-Length headers
• Update advice on contentLength() in subclasses of InputStreamResource  #20990 Consistent treatment of InputStreamResource subclasses
• Support use of MultipartFile as input to RestTemplate or WebClient [SPR-16808] #21348 Support use of MultipartFile as input to RestTemplate or WebClient

Referenced from: commits 27c1280

0 votes, 6 watchers

[spring-projects-issues]

Brian Clozel commented

In order to properly write the multipart request, the FormHttpMessageConverter configured automatically with the RestTemplate will write all parts; if a part inherits from Resource, it calls the Resource.getFilename() method to get a file name, see the getFilename() method. If no file name is found, then this part is written as a "regular" part, not a file, in the content-disposition part of the message.

In your case, you could do the following:

@RequestMapping(method = RequestMethod.POST)
public void uploadFile(@RequestParam("customerId") int customerId, @RequestPart("file") MultipartFile multipartFile) {
  MultiValueMap<String,Object> parts = new LinkedMultiValueMap<>();
  parts.add("customerId", customerId);
  try {
    parts.add("file", new MultipartFileResource(multipartFile.getInputStream()));
  } catch (IOException e) {
    throw new RuntimeException(e);
  }
  HttpHeaders headers = new HttpHeaders();
  headers.setContentType(MediaType.MULTIPART_FORM_DATA);
  HttpEntity<MultiValueMap<String,Object>> request = new HttpEntity<>(parts, headers);
  restTemplate.exchange("http://localhost:8080", HttpMethod.POST, request, Void.class);
}

private class MultipartFileResource extends InputStreamResource {

  public MultipartFileResource(InputStream inputStream, String filename) {
    super(inputStream);
    this.filename = filename;
  }
  @Override
  public String getFilename() {
    return this.filename;
  }
}

There are a few ways to improve this situation in the framework.

1. try to "guess" the filename for all parts extending Resource; the problem is, we don't have that information and we can only try to guess. This can also lead to new issues, since we'd be considering parts as file whereas those weren't in the past
2. provide something that looks like a MultipartFileResource implementation and add it in the reference documentation

Could you try the workaround I told you about and confirm this works?
Let me know what you think about the solutions listed here.

Thanks!

[spring-projects-issues]

Greg Adams commented

I've tried with code nearly identical to your comment

@RestController
public class FileForwardController {
	
	private RestTemplate restTemplate;
	
	public FileForwardController() {
		SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
		requestFactory.setBufferRequestBody(false);
		this.restTemplate = new RestTemplate(requestFactory);
	}

	@RequestMapping(method = RequestMethod.POST)
	public void uploadFile(@RequestParam("customerId") int customerId, @RequestPart("file") MultipartFile multipartFile) {
		MultiValueMap<String,Object> parts = new LinkedMultiValueMap<>();
		parts.add("customerId", customerId);
		try {
			parts.add("file", new MultipartFileResource(multipartFile.getInputStream(), "test"));
		} catch (IOException e) {
			throw new RuntimeException(e);
		}
		HttpHeaders headers = new HttpHeaders();
		headers.setContentType(MediaType.MULTIPART_FORM_DATA);
		HttpEntity<MultiValueMap<String,Object>> request = new HttpEntity<>(parts, headers);
		restTemplate.exchange("http://localhost:8080", HttpMethod.POST, request, Void.class);
	}
}

public class MultipartFileResource extends InputStreamResource {

	private String filename;

	public MultipartFileResource(InputStream inputStream, String filename) {
		super(inputStream);
		this.filename = filename;
	}

	@Override
	public String getFilename() {
		return this.filename;
	}
}

the restTemplate.exchange call throws the following exception:

java.lang.IllegalStateException: InputStream has already been read - do not use InputStreamResource if a stream needs to be read multiple times
	at org.springframework.core.io.InputStreamResource.getInputStream(InputStreamResource.java:96) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.ResourceHttpMessageConverter.writeInternal(ResourceHttpMessageConverter.java:100) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.ResourceHttpMessageConverter.writeInternal(ResourceHttpMessageConverter.java:47) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.AbstractHttpMessageConverter.write(AbstractHttpMessageConverter.java:195) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.FormHttpMessageConverter.writePart(FormHttpMessageConverter.java:349) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.FormHttpMessageConverter.writeParts(FormHttpMessageConverter.java:329) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.FormHttpMessageConverter.writeMultipart(FormHttpMessageConverter.java:318) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.FormHttpMessageConverter.write(FormHttpMessageConverter.java:233) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.http.converter.FormHttpMessageConverter.write(FormHttpMessageConverter.java:88) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
	at org.springframework.web.client.RestTemplate$HttpEntityRequestCallback.doWithRequest(RestTemplate.java:800) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]

[spring-projects-issues]

Greg Adams commented

It looks like the InputStreamResource.getInputStream() method is called twice, once from MultipartFileResource.contentLength() (via ResourceHttpMessageConverter.getContentLength(Resource, MediaType)) and again from ResourceHttpMessageConverter.writeInternal(Resource, HttpOutputMessage), and InputStreamResource doesn't allow multiple calls to getInputStream.

[spring-projects-issues]

Greg Adams commented

Using something like this works:

public class MultipartFileResource extends ByteArrayResource {

	private String filename;

	public MultipartFileResource(MultipartFile multipartFile) throws IOException {
		super(multipartFile.getBytes());
		this.filename = multipartFile.getOriginalFilename();
	}

	@Override
	public String getFilename() {
		return this.filename;
	}
}

I'm still not sure it's a good idea to call multipartFile.getBytes() like this. Doesn't that read the entire file into heap? Wouldn't that expose you to the possibility of OOM for very large files?

[spring-projects-issues]

Marcus Schulte commented

Actually ResourceHttpMessageConverter seems a bit broken to me, when it checks whether it should ask a resource for its content-length by comparing its type to a constant:

@Override
protected Long getContentLength(Resource resource, MediaType contentType) throws IOException {
     // Don't try to determine contentLength on InputStreamResource - cannot be read afterwards...
     // Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
     return (InputStreamResource.class == resource.getClass() ? null : resource.contentLength());
}

This gives anyone inheriting from InputStreamResource a hard time conforming to the Liskov principle.

For me, a combination of a custom-named resource and a slightly altered version of ResourceHttpMessageConverter does the trick:

/**
 * This class leaves it to the resource-implementation to decide whether it can reasonably supply a
 * content-length. It does so by assuming that returning {@code null} or a negative number indicates
 * its unwillingness to provide a content-length.
 *
 */

public class ResourceHttpMessageConverterHandlingInputStreams extends ResourceHttpMessageConverter {

    @Override
    protected Long getContentLength(Resource resource, MediaType contentType) throws IOException {
        Long contentLength = super.getContentLength(resource, contentType);

        return contentLength == null || contentLength < 0 ? null : contentLength;
    }
}

Then, a resource like the following works

 /**
     * Works with {@link ResourceHttpMessageConverterHandlingInputStreams} to forward input stream from
     * file-uploads without reading everything into memory.
     *
     */
    private class MultipartInputStreamFileResource extends InputStreamResource {

        private final String filename;

        public MultipartInputStreamFileResource(InputStream inputStream, String filename) {
            super(inputStream);
            this.filename = filename;
        }
        @Override
        public String getFilename() {
            return this.filename;
        }

        @Override
        public long contentLength() throws IOException {
            return -1; // we do not want to generally read the whole stream into memory ...
        }
    }

Of course, the broken Converter needs to be replaced in the RestTemplate:

List<HttpMessageConverter<?>> messageConverters = this.restTemplate.getMessageConverters();
       for (int i = 0; i < messageConverters.size(); i++) {
           HttpMessageConverter<?> messageConverter = messageConverters.get(i);
           if ( messageConverter.getClass().equals(ResourceHttpMessageConverter.class) )
               messageConverters.set(i, new ResourceHttpMessageConverterHandlingInputStreams());
       }

A method like Resource.isContentLengthAvailable() along with corresponding code to support it would be nice to have ...

[spring-projects-issues]

Juergen Hoeller commented

At this point, a dedicated isContentLengthAvailable() method doesn't seem to be worth it. After all, this is primarily a limitation of InputStreamResource itself which unfortunately - for backwards compatibility reasons - needs to support contentLength() calls that just need to determine the body length, never actually reading the body with the same resource afterwards.

So effectively, all that our check there does is to bypass a standard implementation that is known to be broken for that particular purpose: If we receive a plain InputStreamResource, we know we can't determine the content length if we intend to read the body as well. For any custom subclass of InputStreamResource, we optimistically expect it to provide a proper content length... since we do not reliably know that it is broken for the given use case, we'd rather not expect so upfront. Such a subclass should be able to override the contentLength() implementation without having to switch any extra flag to make it work.

That said, for any custom subclass of InputStreamResource, there should be a way to bypass content length determination the same way at least. Your suggestion of letting ResourceHttpMessageConverter check for negative content length values and bypass content length handling the same way sounds good there; I've rolled this into 4.3.

Juergen