diff --git a/gradle/changelog/ahc_multipart.yaml b/gradle/changelog/ahc_multipart.yaml new file mode 100644 index 0000000000..fa87f220ad --- /dev/null +++ b/gradle/changelog/ahc_multipart.yaml @@ -0,0 +1,2 @@ +- type: added + description: Support for multipart form data to AdvancedHttpClient ([#1856](https://github.com/scm-manager/scm-manager/pull/1856)) diff --git a/scm-core/src/main/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBody.java b/scm-core/src/main/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBody.java index 8b23185b0d..679ee72e37 100644 --- a/scm-core/src/main/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBody.java +++ b/scm-core/src/main/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBody.java @@ -26,7 +26,6 @@ package sonia.scm.net.ahc; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.base.Charsets; import com.google.common.io.ByteSource; //~--- JDK imports ------------------------------------------------------------ @@ -34,6 +33,7 @@ import com.google.common.io.ByteSource; import java.io.File; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; /** * Http request with body. @@ -170,7 +170,7 @@ public class AdvancedHttpRequestWithBody */ public AdvancedHttpRequestWithBody stringContent(String content) { - return stringContent(content, Charsets.UTF_8); + return stringContent(content, StandardCharsets.UTF_8); } /** @@ -215,6 +215,17 @@ public class AdvancedHttpRequestWithBody return rawContent(value); } + /** + * Use custom implementation of {@link Content} to create request content. + * @param content content implementation + * @return {@code this} + * @since 2.27.0 + */ + AdvancedHttpRequestWithBody content(Content content) { + this.content = content; + return this; + } + /** * Transforms the given object to a xml string and set this string as request * content. diff --git a/scm-core/src/main/java/sonia/scm/net/ahc/FormContentBuilder.java b/scm-core/src/main/java/sonia/scm/net/ahc/FormContentBuilder.java index 6e9a2542a8..a4ee56834f 100644 --- a/scm-core/src/main/java/sonia/scm/net/ahc/FormContentBuilder.java +++ b/scm-core/src/main/java/sonia/scm/net/ahc/FormContentBuilder.java @@ -24,109 +24,206 @@ package sonia.scm.net.ahc; -//~--- non-JDK imports -------------------------------------------------------- - +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; - +import com.google.common.io.ByteStreams; +import lombok.RequiredArgsConstructor; import sonia.scm.util.HttpUtil; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.function.Supplier; + /** * The form builder is able to add form parameters to a request. * * @author Sebastian Sdorra * @since 1.46 */ -public class FormContentBuilder -{ +public class FormContentBuilder { + + private final AdvancedHttpRequestWithBody request; + private final List stringEntries = new ArrayList<>(); + private final List fileEntries = new ArrayList<>(); + private final Supplier boundaryFactory; /** * Constructs a new {@link FormContentBuilder}. * - * * @param request request */ - public FormContentBuilder(AdvancedHttpRequestWithBody request) - { - this.request = request; + public FormContentBuilder(AdvancedHttpRequestWithBody request) { + this(request, () -> "------------------------" + System.currentTimeMillis()); } - //~--- methods -------------------------------------------------------------- + @VisibleForTesting + FormContentBuilder(AdvancedHttpRequestWithBody request, Supplier boundaryFactory) { + this.request = request; + this.boundaryFactory = boundaryFactory; + } /** - * Build the formular content and append it to the request. + * Adds a form parameter. * + * @param name parameter name + * @param values parameter values + * + * @return {@code this} + */ + public FormContentBuilder fields(String name, Iterable values) { + for (Object v : values) { + append(name, v); + } + return this; + } + + /** + * Adds a formular parameter. + * + * @param name parameter name + * @param values parameter values + * + * @return {@code this} + */ + public FormContentBuilder field(String name, Object... values) { + for (Object v : values) { + append(name, v); + } + + return this; + } + + private void append(String name, Object value) { + if (!Strings.isNullOrEmpty(name)) { + stringEntries.add(new StringFormEntry(name, value != null ? value.toString() : "")); + } + } + + /** + * Upload a file as part of the form request. + * Whenever a file is part of the request, + * the request is sent as multipart form request instead of an url encoded form. + * + * @param name parameter name + * @param filename name of the file + * @param content input stream of the file content + * + * @return {@code this} + * @since 2.27.0 + */ + public FormContentBuilder file(String name, String filename, InputStream content) { + fileEntries.add(new FileFormEntry(name, filename, content)); + return this; + } + + /** + * Build the form content and append it to the request. * * @return request instance */ - public AdvancedHttpRequestWithBody build() - { - request.contentType("application/x-www-form-urlencoded"); - request.stringContent(builder.toString()); - + public AdvancedHttpRequestWithBody build() { + if (fileEntries.isEmpty()) { + request.contentType("application/x-www-form-urlencoded"); + request.stringContent(urlEncoded()); + } else { + request.content(new MultipartFormContent(boundaryFactory.get())); + } return request; } - /** - * Adds a formular parameter. - * - * - * @param name parameter name - * @param values parameter values - * - * @return {@code this} - */ - public FormContentBuilder fields(String name, Iterable values) - { - for (Object v : values) - { - append(name, v); - } - - return this; - } - - /** - * Adds a formular parameter. - * - * - * @param name parameter name - * @param values parameter values - * - * @return {@code this} - */ - public FormContentBuilder field(String name, Object... values) - { - for (Object v : values) - { - append(name, v); - } - - return this; - } - - private void append(String name, Object value) - { - if (!Strings.isNullOrEmpty(name)) - { - if (builder.length() > 0) - { + private String urlEncoded() { + StringBuilder builder = new StringBuilder(); + Iterator it = stringEntries.iterator(); + while (it.hasNext()) { + StringFormEntry e = it.next(); + builder.append(HttpUtil.encode(e.name)).append("=").append(HttpUtil.encode(e.value)); + if (it.hasNext()) { builder.append("&"); } - - builder.append(HttpUtil.encode(name)).append("="); - - if (value != null) - { - builder.append(HttpUtil.encode(value.toString())); - } } + return builder.toString(); } - //~--- fields --------------------------------------------------------------- + @RequiredArgsConstructor + private static class StringFormEntry { + private final String name; + private final String value; + } - /** content builder */ - private final StringBuilder builder = new StringBuilder(); + @RequiredArgsConstructor + private static class FileFormEntry { + private final String name; + private final String filename; + private final InputStream stream; + } - /** request */ - private final AdvancedHttpRequestWithBody request; + class MultipartFormContent implements Content { + + private final String boundary; + + private MultipartFormContent(String boundary) { + this.boundary = boundary; + } + + @Override + public void prepare(AdvancedHttpRequestWithBody request) throws IOException { + request.contentType("multipart/form-data; boundary=" + boundary); + } + + @Override + public void process(OutputStream output) throws IOException { + PrintWriter writer = new PrintWriter(output); + + writeBoundary(writer); + for (StringFormEntry entry : stringEntries) { + field(writer, entry); + } + + for (FileFormEntry entry : fileEntries) { + file(output, writer, entry); + } + + writer.flush(); + } + + private void field(PrintWriter writer, StringFormEntry entry) { + writeContentDisposition(writer, entry.name); + writer.println(); + writer.println(entry.value); + writeBoundary(writer); + } + + private void file(OutputStream output, PrintWriter writer, FileFormEntry entry) throws IOException { + writeContentDisposition(writer, entry.name, entry.filename); + writer.println("Content-Transfer-Encoding: binary"); + writer.println(); + + writer.flush(); + ByteStreams.copy(entry.stream, output); + + writer.println(); + writeBoundary(writer); + } + + private void writeBoundary(PrintWriter writer) { + writer.append("--").println(boundary); + } + + private void writeContentDisposition(PrintWriter writer, String name) { + writeContentDisposition(writer, name, null); + } + + private void writeContentDisposition(PrintWriter writer, String name, String filename) { + writer.append("Content-Disposition: form-data; name=\"").append(name).append("\""); + if (filename != null) { + writer.append("; filename=\"").append(filename).append("\""); + } + writer.println(); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBodyTest.java b/scm-core/src/test/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBodyTest.java index 9ac599581a..e9622d92c3 100644 --- a/scm-core/src/test/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBodyTest.java +++ b/scm-core/src/test/java/sonia/scm/net/ahc/AdvancedHttpRequestWithBodyTest.java @@ -26,27 +26,27 @@ package sonia.scm.net.ahc; import com.google.common.base.Charsets; import com.google.common.io.ByteSource; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; - -import org.junit.Test; -import static org.junit.Assert.*; -import static org.hamcrest.Matchers.*; -import static org.mockito.Mockito.*; -import org.junit.Before; -import org.junit.Rule; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; /** * * @author Sebastian Sdorra */ -@RunWith(MockitoJUnitRunner.class) -public class AdvancedHttpRequestWithBodyTest { +@ExtendWith(MockitoExtension.class) +class AdvancedHttpRequestWithBodyTest { @Mock private AdvancedHttpClient ahc; @@ -55,108 +55,129 @@ public class AdvancedHttpRequestWithBodyTest { private ContentTransformer transformer; private AdvancedHttpRequestWithBody request; - - @Rule - public TemporaryFolder tempFolder = new TemporaryFolder(); - - @Before - public void before(){ + + @BeforeEach + void before(){ request = new AdvancedHttpRequestWithBody(ahc, HttpMethod.PUT, "https://www.scm-manager.org"); } @Test - public void testContentLength() - { - request.contentLength(12l); - assertEquals("12", request.getHeaders().get("Content-Length").iterator().next()); + void shouldReturnContentLength() { + request.contentLength(12L); + assertThat(request.getHeaders().get("Content-Length").iterator().next()).isEqualTo("12"); } @Test - public void testContentType(){ + void shouldReturnContentType(){ request.contentType("text/plain"); - assertEquals("text/plain", request.getHeaders().get("Content-Type").iterator().next()); + assertThat(request.getHeaders().get("Content-Type").iterator().next()).isEqualTo("text/plain"); } @Test - public void testFileContent() throws IOException{ - File file = tempFolder.newFile(); - request.fileContent(file); - assertThat(request.getContent(), instanceOf(FileContent.class)); + void shouldReturnFileContent(@TempDir Path path) { + request.fileContent(path.toFile()); + assertThat(request.getContent()).isInstanceOf(FileContent.class); } @Test - public void testRawContent() throws IOException { + void shouldReturnRawContent() { request.rawContent("test".getBytes(Charsets.UTF_8)); - assertThat(request.getContent(), instanceOf(RawContent.class)); + assertThat(request.getContent()).isInstanceOf(RawContent.class); } @Test - public void testRawContentWithByteSource() throws IOException { + void shouldReturnRawContentFromByteSource() { ByteSource bs = ByteSource.wrap("test".getBytes(Charsets.UTF_8)); request.rawContent(bs); - assertThat(request.getContent(), instanceOf(ByteSourceContent.class)); + assertThat(request.getContent()).isInstanceOf(ByteSourceContent.class); } @Test - public void testFormContent(){ + void shouldApplyFormContent(){ FormContentBuilder builder = request.formContent(); - assertNotNull(builder); + assertThat(builder).isNotNull(); builder.build(); - assertThat(request.getContent(), instanceOf(StringContent.class)); + assertThat(request.getContent()).isInstanceOf(StringContent.class); } @Test - public void testStringContent(){ + void shouldReturnStringContent(){ request.stringContent("test"); - assertThat(request.getContent(), instanceOf(StringContent.class)); + assertThat(request.getContent()).isInstanceOf(StringContent.class); } @Test - public void testStringContentWithCharset(){ + void shouldReturnStringContentFormStringContentWithCharset(){ request.stringContent("test", Charsets.UTF_8); - assertThat(request.getContent(), instanceOf(StringContent.class)); + assertThat(request.getContent()).isInstanceOf(StringContent.class); + } + + @Test + void shouldReturnCustomContent(){ + request.content(new CustomContent()); + assertThat(request.getContent()).isInstanceOf(CustomContent.class); } @Test - public void testXmlContent() throws IOException{ + void shouldReturnXmlContent() throws IOException { when(ahc.createTransformer(String.class, ContentType.XML)).thenReturn(transformer); when(transformer.marshall("")).thenReturn(ByteSource.wrap("".getBytes(Charsets.UTF_8))); + Content content = request.xmlContent("").getContent(); - assertThat(content, instanceOf(ByteSourceContent.class)); + assertThat(content).isInstanceOf(ByteSourceContent.class); + ByteSourceContent bsc = (ByteSourceContent) content; ByteArrayOutputStream baos = new ByteArrayOutputStream(); bsc.process(baos); - assertEquals("", baos.toString("UTF-8")); + + assertThat(baos.toString("UTF-8")).isEqualTo(""); } @Test - public void testJsonContent() throws IOException{ + void shouldReturnJsonContent() throws IOException{ when(ahc.createTransformer(String.class, ContentType.JSON)).thenReturn(transformer); when(transformer.marshall("{}")).thenReturn(ByteSource.wrap("{'root': {}}".getBytes(Charsets.UTF_8))); + Content content = request.jsonContent("{}").getContent(); - assertThat(content, instanceOf(ByteSourceContent.class)); + assertThat(content).isInstanceOf(ByteSourceContent.class); + ByteSourceContent bsc = (ByteSourceContent) content; ByteArrayOutputStream baos = new ByteArrayOutputStream(); bsc.process(baos); - assertEquals("{'root': {}}", baos.toString("UTF-8")); - } - - @Test - public void testTransformedContent() throws IOException{ - when(ahc.createTransformer(String.class, "text/plain")).thenReturn(transformer); - when(transformer.marshall("hello")).thenReturn(ByteSource.wrap("hello world".getBytes(Charsets.UTF_8))); - Content content = request.transformedContent("text/plain", "hello").getContent(); - assertThat(content, instanceOf(ByteSourceContent.class)); - ByteSourceContent bsc = (ByteSourceContent) content; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - bsc.process(baos); - assertEquals("hello world", baos.toString("UTF-8")); + + assertThat(baos.toString("UTF-8")).isEqualTo("{'root': {}}"); } @Test - public void testSelf() - { - assertEquals(AdvancedHttpRequestWithBody.class, request.self().getClass()); + void shouldReturnTransformedContent() throws IOException{ + when(ahc.createTransformer(String.class, "text/plain")).thenReturn(transformer); + when(transformer.marshall("hello")).thenReturn(ByteSource.wrap("hello world".getBytes(Charsets.UTF_8))); + + Content content = request.transformedContent("text/plain", "hello").getContent(); + assertThat(content).isInstanceOf(ByteSourceContent.class); + + ByteSourceContent bsc = (ByteSourceContent) content; + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + bsc.process(baos); + + assertThat(baos.toString("UTF-8")).isEqualTo("hello world"); + } + + @Test + void shouldReturnSelf() { + assertThat(request.self().getClass()).isEqualTo(AdvancedHttpRequestWithBody.class); + } + + private static class CustomContent implements Content { + @Override + public void prepare(AdvancedHttpRequestWithBody request) throws IOException { + + } + + @Override + public void process(OutputStream output) throws IOException { + + } } } diff --git a/scm-core/src/test/java/sonia/scm/net/ahc/FormContentBuilderTest.java b/scm-core/src/test/java/sonia/scm/net/ahc/FormContentBuilderTest.java index ae9849625c..f5cdf0366a 100644 --- a/scm-core/src/test/java/sonia/scm/net/ahc/FormContentBuilderTest.java +++ b/scm-core/src/test/java/sonia/scm/net/ahc/FormContentBuilderTest.java @@ -21,68 +21,159 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.net.ahc; import com.google.common.collect.Lists; -import org.junit.Test; -import static org.junit.Assert.*; -import org.junit.runner.RunWith; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; import org.mockito.Mock; -import static org.mockito.Mockito.*; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; /** - * * @author Sebastian Sdorra */ -@RunWith(MockitoJUnitRunner.class) -public class FormContentBuilderTest { +@ExtendWith(MockitoExtension.class) +class FormContentBuilderTest { @Mock private AdvancedHttpRequestWithBody request; - - @InjectMocks + private FormContentBuilder builder; - @Test - public void testFieldEncoding() - { - builder.field("a", "ü", "ä", "ö").build(); - assertContent("a=%C3%BC&a=%C3%A4&a=%C3%B6"); - } - - @Test - public void testBuild() - { - builder.field("a", "b").build(); - assertContent("a=b"); - verify(request).contentType("application/x-www-form-urlencoded"); + @Nested + class Default { + + @BeforeEach + void setUpObjectUnderTest() { + // required because InjectMocks uses the wrong constructor + builder = new FormContentBuilder(request); + } + + @Test + void shouldEncodeFieldValues() { + builder.field("a", "ü", "ä", "ö").build(); + assertUrlEncodedContent("a=%C3%BC&a=%C3%A4&a=%C3%B6"); + } + + @Test + void shouldApplySimpleUrlEncoded() { + builder.field("a", "b").build(); + + assertUrlEncodedContent("a=b"); + + verify(request).contentType("application/x-www-form-urlencoded"); + } + + @Test + void shouldUseMultipartFormData() throws IOException { + builder.file("file", "test.txt", stream("hello")); + buildMultipart(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(request).contentType(captor.capture()); + + assertThat(captor.getValue()).startsWith("multipart/form-data; boundary="); + } + + @Test + void shouldCreateValuesFromVarargs() { + builder.field("a", "b").field("c", "d", "e").build(); + assertUrlEncodedContent("a=b&c=d&c=e"); + } + + @Test + void shouldCreateValuesFromIterables() { + Iterable i1 = Lists.newArrayList("b"); + builder.fields("a", i1) + .fields("c", Lists.newArrayList("d", "e")) + .build(); + assertUrlEncodedContent("a=b&c=d&c=e"); + } + } - @Test - public void testFieldWithArray() - { - builder.field("a", "b").field("c", "d", "e").build(); - assertContent("a=b&c=d&c=e"); + @Nested + class WithBoundary { + + @BeforeEach + void setUpObjectUnderTest() { + // required because InjectMocks uses the wrong constructor + builder = new FormContentBuilder(request, () -> "boundary"); + } + + @Test + void shouldAppendBoundaryToContentType() throws IOException { + builder.file("file", "test.txt", stream("hello")); + buildMultipart(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(request).contentType(captor.capture()); + + assertThat(captor.getValue()).startsWith("multipart/form-data; boundary=boundary"); + } + + @Test + void shouldSendMultipartContent() throws IOException { + builder.field("title", "Readme"); + builder.file("content", "README.md", stream("# hello")); + + String expected = String.join(System.lineSeparator(), + "--boundary", + "Content-Disposition: form-data; name=\"title\"", + "", + "Readme", + "--boundary", + "Content-Disposition: form-data; name=\"content\"; filename=\"README.md\"", + "Content-Transfer-Encoding: binary", + "", + "# hello", + "--boundary", + "" + ); + + assertThat(buildMultipart()).isEqualTo(expected); + } + } - - @Test - public void testFieldWithIterable() - { - Iterable i1 = Lists.newArrayList("b"); - builder.fields("a", i1) - .fields("c", Lists.newArrayList("d", "e")) - .build(); - assertContent("a=b&c=d&c=e"); + + private InputStream stream(String content) { + return new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)); } - - private void assertContent(String content){ + + + private void assertUrlEncodedContent(String content) { ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(request).stringContent(captor.capture()); - assertEquals(content, captor.getValue()); + assertThat(captor.getValue()).isEqualTo(content); + } + + private String buildMultipart() throws IOException { + builder.build(); + + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(Content.class); + verify(request).content(contentCaptor.capture()); + + Content content = contentCaptor.getValue(); + content.prepare(request); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + content.process(baos); + + return baos.toString(); } }