Fix multipart support for HTTP functions. (#27)

Jetty requires a servlet to be configured explicitly before it will accept multipart input.

Author: eamonnmcmanus

invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java

@@ -55,6 +55,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
+import javax.servlet.MultipartConfigElement;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -284,7 +285,9 @@ public void startServer() throws Exception {
           functionSignatureType);
       throw new RuntimeException(error);
     }
-    servletContextHandler.addServlet(new ServletHolder(servlet), "/*");
+    ServletHolder servletHolder = new ServletHolder(servlet);
+    servletHolder.getRegistration().setMultipartConfig(new MultipartConfigElement(""));
+    servletContextHandler.addServlet(servletHolder, "/*");
 
     server.start();
     logServerInfo();

invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java

@@ -59,9 +59,13 @@
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentProvider;
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.client.util.BytesContentProvider;
+import org.eclipse.jetty.client.util.MultiPartContentProvider;
 import org.eclipse.jetty.client.util.StringContentProvider;
+import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
 import org.junit.BeforeClass;
@@ -150,7 +154,7 @@ abstract static class TestCase {
 
     abstract String url();
 
-    abstract String requestText();
+    abstract ContentProvider requestContent();
 
     abstract int expectedResponseCode();
 
@@ -162,7 +166,7 @@ abstract static class TestCase {
 
     abstract Optional<String> expectedOutput();
 
-    abstract String httpContentType();
+    abstract Optional<String> httpContentType();
 
     abstract ImmutableMap<String, String> httpHeaders();
 
@@ -183,7 +187,11 @@ abstract static class Builder {
 
       abstract Builder setUrl(String x);
 
-      abstract Builder setRequestText(String x);
+      abstract Builder setRequestContent(ContentProvider x);
+
+      Builder setRequestText(String text) {
+        return setRequestContent(new StringContentProvider(text));
+      }
 
       abstract Builder setExpectedResponseCode(int x);
 
@@ -199,6 +207,8 @@ abstract static class Builder {
 
       abstract Builder setHttpContentType(String x);
 
+      abstract Builder setHttpContentType(Optional<String> x);
+
       abstract Builder setHttpHeaders(ImmutableMap<String, String> x);
 
       abstract Builder setSnoopFile(File x);
@@ -408,6 +418,25 @@ public void packageless() throws Exception {
         ImmutableList.of(TestCase.builder().setExpectedResponseText("hello, world\n").build()));
   }
 
+  @Test
+  public void multipart() throws Exception {
+    MultiPartContentProvider multiPartProvider = new MultiPartContentProvider();
+    byte[] bytes = new byte[17];
+    multiPartProvider.addFieldPart("bytes", new BytesContentProvider(bytes), new HttpFields());
+    String string = "1234567890";
+    multiPartProvider.addFieldPart("string", new StringContentProvider(string), new HttpFields());
+    String expectedResponse = "part bytes type application/octet-stream length 17\n"
+        + "part string type text/plain;charset=UTF-8 length 10\n";
+    testHttpFunction(
+        fullTarget("Multipart"),
+        ImmutableList.of(
+            TestCase.builder()
+                .setHttpContentType(Optional.empty())
+                .setRequestContent(multiPartProvider)
+                .setExpectedResponseText(expectedResponse)
+                .build()));
+  }
+
   private File snoopFile() throws IOException {
     return temporaryFolder.newFile(testName.getMethodName() + ".txt");
   }
@@ -522,9 +551,10 @@ private void testFunction(
         testCase.snoopFile().ifPresent(File::delete);
         String uri = "http://localhost:" + serverPort + testCase.url();
         Request request = httpClient.POST(uri);
-        request.header(HttpHeader.CONTENT_TYPE, testCase.httpContentType());
+        testCase.httpContentType().ifPresent(
+            contentType -> request.header(HttpHeader.CONTENT_TYPE, contentType));
         testCase.httpHeaders().forEach((header, value) -> request.header(header, value));
-        request.content(new StringContentProvider(testCase.requestText()));
+        request.content(testCase.requestContent());
         ContentResponse response = request.send();
         expect
             .withMessage("Response to %s is %s %s", uri, response.getStatus(), response.getReason())

invoker/core/src/test/java/com/google/cloud/functions/invoker/testfunctions/Multipart.java

@@ -0,0 +1,36 @@
+package com.google.cloud.functions.invoker.testfunctions;
+
+import com.google.cloud.functions.HttpFunction;
+import com.google.cloud.functions.HttpRequest;
+import com.google.cloud.functions.HttpRequest.HttpPart;
+import com.google.cloud.functions.HttpResponse;
+import java.io.PrintWriter;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+
+/**
+ * A simple proof-of-concept function for multipart handling.
+ *
+ * {@code HttpFunctionTest} contains more detailed testing, but this function is part of the
+ * integration test that shows that we can indeed access the multipart API from a function.
+ */
+public class Multipart implements HttpFunction {
+  @Override
+  public void service(HttpRequest request, HttpResponse response) throws Exception {
+    response.setContentType("text/plain");
+    String contentType = request.getContentType().orElse("<unknown>");
+    if (!contentType.startsWith("multipart/form-data")) {
+      response.getWriter().write("Content-Type is " + contentType + " not multipart/form-data");
+      return;
+    }
+    PrintWriter writer = new PrintWriter(response.getWriter());
+    NavigableMap<String, HttpPart> parts = new TreeMap<>(request.getParts());
+    parts.forEach(
+        (name, contents) -> {
+          writer.printf(
+              "part %s type %s length %d\n",
+              name, contents.getContentType().get(), contents.getContentLength());
+        }
+    );
+  }
+}