Multipart form support for AdvancedHttpClient (#1856)

Add support for mutlipart form data to the AdvancedHttpClient API.
2026-03-04 11:20:53 +01:00 · 2021-11-11 15:48:16 +01:00
parent 25ed55753a
commit 42d79bad63
5 changed files with 401 additions and 179 deletions
--- a/gradle/changelog/ahc_multipart.yaml
+++ 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))
--- 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.
--- 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<StringFormEntry> stringEntries = new ArrayList<>();
+  private final List<FileFormEntry> fileEntries = new ArrayList<>();
+  private final Supplier<String> 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<String> 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<? extends Object> 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<? extends Object> 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<StringFormEntry> 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();
+    }
+  }
 }
--- 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("<root />")).thenReturn(ByteSource.wrap("<root></root>".getBytes(Charsets.UTF_8)));
+
    Content content = request.xmlContent("<root />").getContent();
-    assertThat(content, instanceOf(ByteSourceContent.class));
+    assertThat(content).isInstanceOf(ByteSourceContent.class);
+
    ByteSourceContent bsc = (ByteSourceContent) content;
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    bsc.process(baos);
-    assertEquals("<root></root>", baos.toString("UTF-8"));
+
+    assertThat(baos.toString("UTF-8")).isEqualTo("<root></root>");
  }
  
  @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 {
+
+    }
  }
  
 }
--- 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<String> 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<Object> 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<String> 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<? extends Object> 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<String> 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<Content> 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();
  }

 }