From 8efb2f76a0ae1e4d7d7a149cb1ff0c5e2888faef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Szynkiewicz?= Date: Mon, 28 Feb 2022 18:44:28 +0100 Subject: [PATCH] Properly support small files in multipart responses in REST Client Reactive fixes #23986 --- .../multipart/MultipartResponseTest.java | 36 +++++++++++++++++-- .../multipart/QuarkusMultipartFormUpload.java | 2 +- .../QuarkusMultipartResponseDataFactory.java | 27 +++----------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java index 4c73daa59c3be..ab9c432c4f473 100644 --- a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java @@ -43,7 +43,7 @@ void shouldParseMultipartResponse() { Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class); MultipartData data = client.getFile(); assertThat(data.file).exists(); - verifyWooHooFile(data.file); + verifyWooHooFile(data.file, 10000); assertThat(data.name).isEqualTo("foo"); assertThat(data.panda.weight).isEqualTo("huge"); assertThat(data.panda.height).isEqualTo("medium"); @@ -52,6 +52,18 @@ void shouldParseMultipartResponse() { assertThat(data.numberz).containsSequence(2008, 2011, 2014); } + @Test + void shouldParseMultipartResponseWithSmallFile() { + Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class); + MultipartData data = client.getSmallFile(); + assertThat(data.file).exists(); + verifyWooHooFile(data.file, 1); + assertThat(data.name).isEqualTo("foo"); + assertThat(data.panda).isNull(); + assertThat(data.number).isEqualTo(1984); + assertThat(data.numberz).isNull(); + } + @Test void shouldParseMultipartResponseWithNulls() { Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class); @@ -93,7 +105,7 @@ void shouldParseMultipartResponseWithClientBuilderApi() { assertThat(data.numbers).containsSequence(2008, 2011, 2014); } - void verifyWooHooFile(File file) { + void verifyWooHooFile(File file, int expectedTimes) { int position = 0; try (FileReader reader = new FileReader(file)) { int read; @@ -101,7 +113,7 @@ void verifyWooHooFile(File file) { assertThat((char) read).isEqualTo(WOO_HOO_WOO_HOO_HOO.charAt(position % WOO_HOO_WOO_HOO_HOO.length())); position++; } - assertThat(position).isEqualTo(WOO_HOO_WOO_HOO_HOO.length() * 10000); + assertThat(position).isEqualTo(WOO_HOO_WOO_HOO_HOO.length() * expectedTimes); } catch (IOException e) { fail("failed to read provided file", e); } @@ -113,6 +125,11 @@ public interface Client { @Produces(MediaType.MULTIPART_FORM_DATA) MultipartData getFile(); + @GET + @Produces(MediaType.MULTIPART_FORM_DATA) + @Path("/small") + MultipartData getSmallFile(); + @GET @Produces(MediaType.MULTIPART_FORM_DATA) @Path("/empty") @@ -146,6 +163,19 @@ public MultipartData getFile() throws IOException { 1984, new int[] { 2008, 2011, 2014 }); } + @GET + @Produces(MediaType.MULTIPART_FORM_DATA) + @Path("/small") + public MultipartData getSmallFile() throws IOException { + File file = File.createTempFile("toDownload", ".txt"); + file.deleteOnExit(); + // let's write Woo hoo, woo hoo hoo 1 time + try (FileOutputStream out = new FileOutputStream(file)) { + out.write(WOO_HOO_WOO_HOO_HOO.getBytes(StandardCharsets.UTF_8)); + } + return new MultipartData("foo", file, null, 1984, null); + } + @GET @Produces(MediaType.MULTIPART_FORM_DATA) @Path("/empty") diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java index 14af23304ae44..c784f8034367e 100644 --- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java +++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java @@ -51,7 +51,7 @@ public QuarkusMultipartFormUpload(Context context, io.netty.handler.codec.http.HttpMethod.POST, "/"); Charset charset = parts.getCharset() != null ? parts.getCharset() : HttpConstants.DEFAULT_CHARSET; - DefaultHttpDataFactory httpDataFactory = new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE, charset) { + DefaultHttpDataFactory httpDataFactory = new DefaultHttpDataFactory(-1, charset) { @Override public FileUpload createFileUpload(HttpRequest request, String name, String filename, String contentType, String contentTransferEncoding, Charset _charset, long size) { diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java index 0770f91f9e9f0..72758422c2950 100644 --- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java +++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java @@ -25,9 +25,7 @@ import io.netty.handler.codec.http.multipart.HttpData; import io.netty.handler.codec.http.multipart.InterfaceHttpData; import io.netty.handler.codec.http.multipart.MemoryAttribute; -import io.netty.handler.codec.http.multipart.MemoryFileUpload; import io.netty.handler.codec.http.multipart.MixedAttribute; -import io.netty.handler.codec.http.multipart.MixedFileUpload; import io.vertx.core.http.HttpClientResponse; import java.io.IOException; import java.nio.charset.Charset; @@ -240,31 +238,16 @@ public Attribute createAttribute(HttpClientResponse response, String name, Strin } // to reuse netty stuff as much as possible, we use FileUpload class to represent the downloaded file + // the difference between this and the original is that we always use DiskFileUpload public FileUpload createFileUpload(HttpClientResponse response, String name, String filename, String contentType, String contentTransferEncoding, Charset charset, long size) { - if (useDisk) { - FileUpload fileUpload = new DiskFileUpload(name, filename, contentType, - contentTransferEncoding, charset, size, baseDir, deleteOnExit); - fileUpload.setMaxSize(maxSize); - checkHttpDataSize(fileUpload); - List list = getList(response); - list.add(fileUpload); - return fileUpload; - } - if (checkSize) { - FileUpload fileUpload = new MixedFileUpload(name, filename, contentType, - contentTransferEncoding, charset, size, minSize, baseDir, deleteOnExit); - fileUpload.setMaxSize(maxSize); - checkHttpDataSize(fileUpload); - List list = getList(response); - list.add(fileUpload); - return fileUpload; - } - MemoryFileUpload fileUpload = new MemoryFileUpload(name, filename, contentType, - contentTransferEncoding, charset, size); + FileUpload fileUpload = new DiskFileUpload(name, filename, contentType, + contentTransferEncoding, charset, size, baseDir, deleteOnExit); fileUpload.setMaxSize(maxSize); checkHttpDataSize(fileUpload); + List list = getList(response); + list.add(fileUpload); return fileUpload; }