Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Custom Headers for Multipart Async Data [SPR-16376] #20922

Closed
spring-issuemaster opened this issue Jan 13, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jan 13, 2018

Marc-Christian Schulze opened SPR-16376 and commented

In order to upload a file using the newly introduced method (cf. #20854)

public static <T, P extends Publisher<T>> 
MultipartInserter BodyInserters::fromMultipartAsyncData (
  String key, 
  P publisher, 
  Class<T> elementClass
)

it's necessary to specify the filename and content type along with the publisher.

Right now when streaming asynchronously file content into a multipart request

Publisher<ByteBuffer> filePublisher = ...
WebClient 
  .create(baseUrl) 
  .post() 
  .uri("...") 
  .body(BodyInserters.fromMultipartAsyncData("file", filePublisher, ByteBuffer.class)) 

the resulting http request looks like:

--ZAbh_nuM150m0P4R_zq9ywXiM_pJ0IKJq 
Content-Disposition: form-data; name="file" 
 
 ...... 
--ZAbh_nuM150m0P4R_zq9ywXiM_pJ0IKJq-- 

As you can see the file name and the content type is not specified. For a correct file upload I would expect the http request to look like:

--ZAbh_nuM150m0P4R_zq9ywXiM_pJ0IKJq 
Content-Disposition: form-data; name="file"; filename="myFile.txt"
Content-Type: application/octet-stream 
 
 ...... 
--ZAbh_nuM150m0P4R_zq9ywXiM_pJ0IKJq-- 


Affects: 5.0.2

Issue Links:

  • #20948 MultipartHttpMessageWriter should not subscribe to Publisher multipart data ("depends on")
  • #20854 Support Publishers for multipart data in BodyInserters
  • #20949 DataBufferUtils.read should not take input stream/channel as parameter
  • #20952 MultipartHttpMessageWriter fails when Publisher is provided as Multipart

Referenced from: commits 646fcc5, 283811b

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2018

Arjen Poutsma commented

There are two possible ways to accomplish this:

  • If the data you are sending comes out of a file, and is not processed in some way before sending, it is better to pass that data as a Resource, so a FileSystemResource, PathResource or ClassPathResource for instance. Having a handle to the file itself allows WebFlux to send it over using zero-zopy, which is even more efficient that using a Flux<DataBuffer>, and it will also pick up the file name from the resource, if available.
  • Aside from the above, the recently introduced MultipartBodyBuilder makes it easier to create multipart data maps, which can then be passed on to BodyInserters.fromMultipartData. It also allows you to customise headers for each part, using the header method. Like so:
MultipartBodyBuilder builder = new MultipartBodyBuilder();
builder.part("foo", new ClassPathResource("org/springframework/http/codec/multipart/foo.txt"))
	.header("Custom-Header-For-Foo", "Value");
builder.part("bar", "bar");
MultiValueMap<String, HttpEntity<?>> multipartData = builder.build();

Mono<ClientResponse> result = webClient
		.post()
		.uri("http://example.com")
		.body(BodyInserters.fromMultipartData(multipartData))
		.exchange();
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2018

Marc-Christian Schulze commented

I'm afraid your outlined workaround does not work as expected.
When trying to use the MultipartBodyBuilder as described, e.g.

Publisher<ByteBuffer> publisher = ...
MultipartBodyBuilder builder = new MultipartBodyBuilder();
builder.asyncPart("file", publisher, ByteBuffer.class).header("filename", fileName);
WebClient 
  .create(url) 
  .post() 
  .uri("some-uri")
  .body(BodyInserters.fromMultipartData(builder.build())) 
  .retrieve() 

The HTTP-Header gets written to the netty stack and flushed. Afterwards I can see the boundary to be written as well followed by the custom headers provided but then the actual part's content is not written. Instead the connection times out.

When looking in the Pull Request providing the backing implementation for Publishers the reason becomes clear:
6c3a645#diff-e9234d8569c85ecd276b0259daacc78bR241

if (httpEntity instanceof MultipartBodyBuilder.PublisherEntity<?, ?>) {
  MultipartBodyBuilder.PublisherEntity<?, ?> publisherEntity = (MultipartBodyBuilder.PublisherEntity<?, ?>) httpEntity;
  resolvableType = publisherEntity.getResolvableType();
}

The MultipartHttpMessageWriter expects the Publisher to be wrapped in a PublisherEntity which is the case when the part is created by the BodyInserters.fromMultipartAsyncData() method. But when I use the above-mentioned approach, the part is wrapped in a HttpEntity. I guess the same check for PublisherEntity has to be put in place at DefaultMultipartInserter:

@Override
public MultipartInserter with(MultiValueMap<String, Object> values) {
  Assert.notNull(values, "'values' must not be null");
  for (Map.Entry<String, List<Object>> entry : values.entrySet()) {
    for (Object value : entry.getValue()) {
      // I guess at this point we should check whether we have to call builder.asyncPart() - in case we got a PublisherEntity
      this.builder.part(entry.getKey(), value);
    }
  }
  return this;
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2018

Marc-Christian Schulze commented

The outlined workaround is not working as expected, unfortunately.
The Headers and Boundary of the HTTP-Request gets written but at the point in time the actual Part shall be written the connection times out.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2018

Marc-Christian Schulze commented

Providing the file's content via a Resource is not possible in my case as I receive the file's content asynchronously and I just want to pass the file's content to the http request without caching in-memory or on disk.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 19, 2018

Arjen Poutsma commented

Marc-Christian Schulze Can you tell me where the Flux<DataBuffer> does come from? I think I did find an issue in DataBufferUtils.read, and it might be related.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 20, 2018

Arjen Poutsma commented

Fixed in 646fcc5

We now check if there is no Content-Disposition header present before adding a default one. See the test case included in that commit to see how you can set a custom Content-Disposition.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2018

Marc-Christian Schulze commented

I can confirm that with your change it's now possible to set the Content-Disposition when using a Publisher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.