finagle-http: Allow for multipart FileElement without Content-Transfe…

…r-Encoding: binary

Problem

We hard-code all FileElements as binary so an appropriate `Content-Transfer-Encoding` head is
added for them. In some cases, we don't want to set any `Content-Transfer-Encoding` (i.e.,
when FileElement is text/plain).

Solution

Expose `isText` attribute for FileElement (default is `false`, which doesn't change today's behavior).
When set to `true`, no `Content-Transfer-Encoding` header is added to the request.

Differential Revision: https://phabricator.twitter.biz/D969859

CHANGELOG.rst

@@ -7,6 +7,13 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
 Unreleased
 ----------
 
+New Features
+~~~~~~~~~~~~
+
+* finagle-http: RequestBuilder's `c.t.f.http.FileElement` now has `isText` (default: false)
+  attribute. When set to `true`, no `Content-Transfer-Encoding` header is populated on the request.
+  ``PHAB_ID=D969859``
+
 Runtime Behavior Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

finagle-base-http/src/main/scala/com/twitter/finagle/http/FormElement.scala

@@ -19,5 +19,6 @@ final case class FileElement(
   name: String,
   content: Buf,
   contentType: Option[String] = None,
-  filename: Option[String] = None)
+  filename: Option[String] = None,
+  isText: Boolean = false)
     extends FormElement

finagle-http/src/test/scala/com/twitter/finagle/http/RequestBuilderTest.scala

@@ -37,6 +37,16 @@ content-type: text/plain; charset=UTF-8
 
 v33
 --Boundary--
+""".replace("\r\n", "\n")
+
+  val MULTIPART1 =
+    """--Boundary
+content-disposition: form-data; name="foo"; filename="file-name"
+content-length: 6
+content-type: application/json; charset=UTF-8
+
+foobar
+--Boundary--
 """.replace("\r\n", "\n")
 
   val FORMPOST0 = "k1=v&k2=v2&k3=v33"
@@ -326,4 +336,18 @@ v33
 
     assert(content == MULTIPART0)
   }
+
+  test("build multipart form with files") {
+    val builder0 = RequestBuilder()
+      .url(URL0)
+      .add(
+        FileElement("foo", Buf.Utf8("foobar"), Some("application/json"), Some("file-name"), true))
+
+    val req0 = builder0.buildFormPost(true)
+    val content = "--[^-\r\n]+".r
+      .replaceAllIn(req0.contentString, "--Boundary")
+      .replace("\r\n", "\n")
+
+    assert(content == MULTIPART1)
+  }
 }

finagle-netty4-http/src/main/scala/com/twitter/finagle/netty4/http/Netty4FormPostEncoder.scala

@@ -3,14 +3,18 @@ package com.twitter.finagle.netty4.http
 import com.twitter.finagle.http._
 import com.twitter.finagle.http.exp.FormPostEncoder
 import com.twitter.finagle.netty4.ByteBufConversion
-import com.twitter.io.{Buf, Reader}
-import io.netty.buffer.{ByteBufAllocator, PooledByteBufAllocator, UnpooledByteBufAllocator}
-import io.netty.handler.codec.http.multipart.{
-  DefaultHttpDataFactory,
-  HttpDataFactory,
-  HttpPostRequestEncoder
-}
-import io.netty.handler.codec.http.{DefaultHttpRequest, FullHttpRequest, HttpRequest, HttpUtil}
+import com.twitter.io.Buf
+import com.twitter.io.Reader
+import io.netty.buffer.ByteBufAllocator
+import io.netty.buffer.PooledByteBufAllocator
+import io.netty.buffer.UnpooledByteBufAllocator
+import io.netty.handler.codec.http.multipart.DefaultHttpDataFactory
+import io.netty.handler.codec.http.multipart.HttpDataFactory
+import io.netty.handler.codec.http.multipart.HttpPostRequestEncoder
+import io.netty.handler.codec.http.DefaultHttpRequest
+import io.netty.handler.codec.http.FullHttpRequest
+import io.netty.handler.codec.http.HttpRequest
+import io.netty.handler.codec.http.HttpUtil
 import java.net.InetSocketAddress
 
 private[finagle] object Netty4FormPostEncoder extends FormPostEncoder {
@@ -50,7 +54,7 @@ private[finagle] object Netty4FormPostEncoder extends FormPostEncoder {
     val encoder = new HttpPostRequestEncoder(dataFactory, netty4Request, multipart)
     try {
       config.formElements.foreach {
-        case FileElement(name, content, contentType, filename) =>
+        case FileElement(name, content, contentType, filename, isText) =>
           addBodyFileUpload(
             encoder = encoder,
             factory = dataFactory,
@@ -59,7 +63,7 @@ private[finagle] object Netty4FormPostEncoder extends FormPostEncoder {
             filename = filename.getOrElse(""),
             content = content,
             contentType = contentType.orNull,
-            isText = false
+            isText = isText
           )
 
         case SimpleElement(name, value) =>
@@ -161,7 +165,7 @@ private[finagle] object Netty4FormPostEncoder extends FormPostEncoder {
       }
     val contentTransferEncoding =
       if (!isText) BinaryTransferEncodingMechanism
-      else SevenBitTransferEncodingMechanism
+      else null // netty is happy with the null
 
     val fileUpload = factory.createFileUpload(
       request,
@@ -178,5 +182,4 @@ private[finagle] object Netty4FormPostEncoder extends FormPostEncoder {
 
   /** Fields defined in the Netty private class HttpPostBodyUtil. */
   private val BinaryTransferEncodingMechanism = "binary"
-  private val SevenBitTransferEncodingMechanism = "7bit"
 }