Fixes for #11, #13

pom.xml

@@ -177,7 +177,7 @@
         <dependency>
             <groupId>commons-fileupload</groupId>
             <artifactId>commons-fileupload</artifactId>
-            <version>1.2.1</version>
+            <version>1.3.2</version>
         </dependency>
         <dependency>
             <groupId>com.hp.hpl.jena</groupId>
@@ -189,5 +189,31 @@
             <artifactId>xom</artifactId>
             <version>1.1</version>
         </dependency>
+
+        <!-- Test dependencies -->
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.12</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-all</artifactId>
+            <version>1.10.19</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <version>3.4.1</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>commons-io</groupId>
+            <artifactId>commons-io</artifactId>
+            <version>2.5</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

src/main/java/org/swordapp/server/SwordAPIEndpoint.java

@@ -6,9 +6,7 @@
 import org.apache.abdera.model.Entry;
 import org.apache.abdera.parser.Parser;
 import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.fileupload.FileItemFactory;
-import org.apache.commons.fileupload.FileUploadException;
-import org.apache.commons.fileupload.ParameterParser;
+import org.apache.commons.fileupload.*;
 import org.apache.commons.fileupload.disk.DiskFileItem;
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
@@ -254,8 +252,8 @@ protected void addDepositPropertiesFromMultipart(Deposit deposit, HttpServletReq
 			throws ServletException, IOException, SwordError
 	{
 		// Parse the request for files (using the fileupload commons library)
-		List<DiskFileItem> items = this.getPartsFromRequest(req);
-		for (DiskFileItem item : items)
+		List<FileItem> items = this.getPartsFromRequest(req);
+		for (FileItem item : items)
 		{
 			// find out which part we are looking at
 			String contentDisposition = item.getHeaders().getHeader("Content-Disposition");
@@ -453,7 +451,7 @@ protected String getContentDispositionValue(String contentDisposition, String ke
 		return parameters.get(key);
 	}
 
-	protected List<DiskFileItem> getPartsFromRequest(HttpServletRequest request)
+	protected List<FileItem> getPartsFromRequest(HttpServletRequest request)
 			throws ServletException
 	{
 		try
@@ -462,11 +460,26 @@ protected List<DiskFileItem> getPartsFromRequest(HttpServletRequest request)
 			FileItemFactory factory = new DiskFileItemFactory();
 
 			// Create a new file upload handler
-			ServletFileUpload upload = new ServletFileUpload(factory);
+			ServletFileUpload upload = new ServletFileUpload(factory) {
+				// Work around WONTFIX https://issues.apache.org/jira/browse/FILEUPLOAD-239
+				@Override
+				protected String getFieldName(FileItemHeaders headers) {
+					String contentDisposition = headers.getHeader(CONTENT_DISPOSITION);
+					if (contentDisposition != null) {
+						ParameterParser parser = new ParameterParser();
+						parser.setLowerCaseNames(true);
+						Map params = parser.parse(contentDisposition, ';');
+						String fieldName = (String) params.get("name");
+						if (fieldName != null) {
+							return fieldName.trim();
+						}
+					}
+					return null;
+				}
+			};
 
 			// Parse the request
-			List<DiskFileItem> items = upload.parseRequest(request);
-
+			List<FileItem> items = upload.parseRequest(request);
 			return items;
 		}
 		catch (FileUploadException e)

src/test/java/org/swordapp/server/SwordAPIEndpointTest.java

@@ -0,0 +1,110 @@
+package org.swordapp.server;
+
+import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.FileItemHeaders;
+import org.apache.commons.io.Charsets;
+import org.apache.commons.io.IOUtils;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.servlet.ServletInputStream;
+import javax.servlet.http.HttpServletRequest;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class SwordAPIEndpointTest {
+
+    private SwordAPIEndpoint endpoint;
+
+    @Before
+    public void setUp() {
+        SwordConfiguration config = mock(SwordConfiguration.class);
+        endpoint = new SwordAPIEndpoint(config);
+    }
+
+    private String getResourceAsString(String resource) throws IOException {
+        return IOUtils.toString(getClass().getResourceAsStream(resource), Charsets.UTF_8);
+    }
+
+    private String ensureCRLF(String data) {
+        return data.replaceAll("\\r?\\n", "\r\n");
+    }
+
+    private ServletInputStream makeServletInputStream(byte[] bodyBytes) throws IOException {
+        final InputStream in = new ByteArrayInputStream(bodyBytes);
+        return new ServletInputStream() {
+            @Override
+            public int read() throws IOException {
+                return in.read();
+            }
+        };
+    }
+
+    private HttpServletRequest makeRequest(String bodyResource) throws IOException {
+        byte[] bodyBytes = ensureCRLF(getResourceAsString(bodyResource)).getBytes();
+        int contentLength = bodyBytes.length;
+
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        when(request.getContentLength()).thenReturn(contentLength);
+        when(request.getContentType()).thenReturn("multipart/related;\r\n" +
+                "            boundary=\"===============0670350989==\";\r\n" +
+                "            type=\"application/atom+xml\"");
+        when(request.getCharacterEncoding()).thenReturn(Charsets.UTF_8.name());
+
+        ServletInputStream sis = makeServletInputStream(bodyBytes);
+        when(request.getInputStream()).thenReturn(sis);
+        return request;
+    }
+
+    /** Make sure we don't break the current behavior */
+    @Test
+    public void getPartsFromRequestHandlesFormData() throws Exception {
+        HttpServletRequest request = makeRequest("/form-data-body.dat");
+        List<FileItem> parts = endpoint.getPartsFromRequest(request);
+        assertThat(parts).hasSize(2);
+
+        FileItem entry = parts.get(0);
+        assertThat(entry.getContentType()).isEqualTo("application/atom+xml");
+        FileItemHeaders entryHeaders = entry.getHeaders();
+        assertThat(entryHeaders).isNotNull();
+        assertThat(entryHeaders.getHeader("MIME-Version")).isEqualTo("1.0");
+        assertThat(entryHeaders.getHeader("Content-Disposition")).isEqualTo("form-data; name=\"atom\"");
+
+        FileItem payload = parts.get(1);
+        assertThat(payload.getContentType()).isEqualTo("application/zip");
+        FileItemHeaders payloadHeaders = payload.getHeaders();
+        assertThat(payloadHeaders).isNotNull();
+        assertThat(payloadHeaders.getHeader("MIME-Version")).isEqualTo("1.0");
+        assertThat(payloadHeaders.getHeader("Content-Disposition")).isEqualTo("form-data; name=\"payload\"; filename=\"example.zip\"");
+        assertThat(payloadHeaders.getHeader("Content-Transfer-Encoding")).isEqualTo("base64");
+    }
+
+    /** Make sure can handle requests that follow the spec */
+    @Test
+    public void getPartsFromRequestHandlesAttachment() throws Exception {
+        HttpServletRequest request = makeRequest("/attachment-body.dat");
+        List<FileItem> parts =  endpoint.getPartsFromRequest(request);
+        assertThat(parts).hasSize(2);
+
+        FileItem entry = parts.get(0);
+        assertThat(entry.getContentType()).isEqualTo("application/atom+xml");
+        FileItemHeaders entryHeaders = entry.getHeaders();
+        assertThat(entryHeaders).isNotNull();
+        assertThat(entryHeaders.getHeader("MIME-Version")).isEqualTo("1.0");
+        assertThat(entryHeaders.getHeader("Content-Disposition")).isEqualTo("attachment; name=\"atom\"");
+
+        FileItem payload = parts.get(1);
+        assertThat(payload.getContentType()).isEqualTo("application/zip");
+        FileItemHeaders payloadHeaders = payload.getHeaders();
+        assertThat(payloadHeaders).isNotNull();
+        assertThat(payloadHeaders.getHeader("MIME-Version")).isEqualTo("1.0");
+        assertThat(payloadHeaders.getHeader("Content-Disposition")).isEqualTo("attachment; name=\"payload\"; filename=\"example.zip\"");
+        assertThat(payloadHeaders.getHeader("Content-Transfer-Encoding")).isEqualTo("base64");
+    }
+}