Repository: square/retrofit
Issue: Multipart request subtype

As discussed in #1063 multipart upload upload still has some issues. For one, it defaults to multipart/mixed, when in [Retrofit 1.9 it used multipart/form-data](https://github.com/square/retrofit/blob/2f4caa6f5df8e7f47a881266bbfc723f537b0b47/retrofit/src/main/java/retrofit/mime/MultipartTypedOutput.java).

I suggest we use the form-data subtype as it seems to be the standard for HTTP file upload. To use it, the following needs to be changed:
- Set Content-Type header of request to `multipart/form-data; boundary=...`
- Set Content-Type header of parts to `multipart/form-data`
- Add `form-data` value to Content-Disposition header and optionally also add the filename parameter


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/RequestBuilder.java b/retrofit/src/main/java/retrofit/RequestBuilder.java
index 7828dab06f..36195f0c0f 100644
--- a/retrofit/src/main/java/retrofit/RequestBuilder.java
+++ b/retrofit/src/main/java/retrofit/RequestBuilder.java
@@ -61,6 +61,7 @@ final class RequestBuilder {
     } else if (isMultipart) {
       // Will be set to 'body' in 'build'.
       multipartBuilder = new MultipartBuilder();
+      multipartBuilder.type(MultipartBuilder.FORM);
     }
   }
 
diff --git a/retrofit/src/main/java/retrofit/RequestBuilderAction.java b/retrofit/src/main/java/retrofit/RequestBuilderAction.java
index ba302c4830..c4898e93f7 100644
--- a/retrofit/src/main/java/retrofit/RequestBuilderAction.java
+++ b/retrofit/src/main/java/retrofit/RequestBuilderAction.java
@@ -241,7 +241,7 @@ static final class PartMap extends RequestBuilderAction {
         }
 
         Headers headers = Headers.of(
-            "Content-Disposition", "name=\"" + entryKey + "\"",
+            "Content-Disposition", "form-data; name=\"" + entryKey + "\"",
             "Content-Transfer-Encoding", transferEncoding);
 
         Class<?> entryClass = entryValue.getClass();
diff --git a/retrofit/src/main/java/retrofit/RequestFactoryParser.java b/retrofit/src/main/java/retrofit/RequestFactoryParser.java
index 7d30171153..50fe0d96a5 100644
--- a/retrofit/src/main/java/retrofit/RequestFactoryParser.java
+++ b/retrofit/src/main/java/retrofit/RequestFactoryParser.java
@@ -290,7 +290,7 @@ private void parseParameters(List<Converter.Factory> converterFactories) {
             }
             Part part = (Part) methodParameterAnnotation;
             com.squareup.okhttp.Headers headers = com.squareup.okhttp.Headers.of(
-                "Content-Disposition", "name=\"" + part.value() + "\"",
+                "Content-Disposition", "form-data; name=\"" + part.value() + "\"",
                 "Content-Transfer-Encoding", part.encoding());
             Converter<?, RequestBody> converter;
             try {
diff --git a/retrofit/src/main/java/retrofit/http/PartMap.java b/retrofit/src/main/java/retrofit/http/PartMap.java
index a8d788447b..338b308bd1 100644
--- a/retrofit/src/main/java/retrofit/http/PartMap.java
+++ b/retrofit/src/main/java/retrofit/http/PartMap.java
@@ -47,6 +47,6 @@
 @Target(PARAMETER)
 @Retention(RUNTIME)
 public @interface PartMap {
-  /** The {@code Content-Transfer-Encoding} of this part. */
+  /** The {@code Content-Transfer-Encoding} of the parts. */
   String encoding() default "binary";
 }


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RequestBuilderTest.java b/retrofit/src/test/java/retrofit/RequestBuilderTest.java
index 826b274726..ee6d58e518 100644
--- a/retrofit/src/test/java/retrofit/RequestBuilderTest.java
+++ b/retrofit/src/test/java/retrofit/RequestBuilderTest.java
@@ -1179,10 +1179,12 @@ Call<Object> method(@Part("ping") String ping, @Part("kit") RequestBody kit) {
     String bodyString = buffer.readUtf8();
 
     assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
         .contains("name=\"ping\"\r\n")
         .contains("\r\npong\r\n--");
 
     assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
         .contains("name=\"kit\"")
         .contains("\r\nkat\r\n--");
   }
@@ -1208,11 +1210,15 @@ Call<Object> method(@Part(value = "ping", encoding = "8-bit") String ping,
     body.writeTo(buffer);
     String bodyString = buffer.readUtf8();
 
-    assertThat(bodyString).contains("name=\"ping\"\r\n")
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"ping\"\r\n")
         .contains("Content-Transfer-Encoding: 8-bit")
         .contains("\r\npong\r\n--");
 
-    assertThat(bodyString).contains("name=\"kit\"")
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"kit\"")
         .contains("Content-Transfer-Encoding: 7-bit")
         .contains("\r\nkat\r\n--");
   }
@@ -1242,10 +1248,12 @@ Call<Object> method(@PartMap Map<String, Object> parts) {
     String bodyString = buffer.readUtf8();
 
     assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
         .contains("name=\"ping\"\r\n")
         .contains("\r\npong\r\n--");
 
     assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
         .contains("name=\"kit\"")
         .contains("\r\nkat\r\n--");
 
@@ -1276,11 +1284,15 @@ Call<Object> method(@PartMap(encoding = "8-bit") Map<String, Object> parts) {
     body.writeTo(buffer);
     String bodyString = buffer.readUtf8();
 
-    assertThat(bodyString).contains("name=\"ping\"\r\n")
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"ping\"\r\n")
         .contains("Content-Transfer-Encoding: 8-bit")
         .contains("\r\npong\r\n--");
 
-    assertThat(bodyString).contains("name=\"kit\"")
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"kit\"")
         .contains("Content-Transfer-Encoding: 8-bit")
         .contains("\r\nkat\r\n--");
 
@@ -1345,6 +1357,7 @@ Call<Object> method(@Part("ping") String ping, @Part("fizz") String fizz) {
     String bodyString = buffer.readUtf8();
 
     assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
         .contains("name=\"ping\"")
         .contains("\r\npong\r\n--");
   }