486394 - Restore MultiPartFilter behavior with regards to temp file access

+ Adding HttpServletRequest.getParts() demonstration of duplicate name="" entries
+ Adding 2 new testcases in MultipartFilterTest demonstrating
  location/temp file access issue (currently @Ignored)

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestGetPartsTest.java

@@ -0,0 +1,145 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+package org.eclipse.jetty.servlet;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.file.Path;
+
+import javax.servlet.MultipartConfigElement;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.Part;
+
+import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class RequestGetPartsTest
+{
+    @SuppressWarnings("serial")
+    public static class DumpPartInfoServlet extends HttpServlet
+    {
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            resp.setContentType("text/plain");
+            PrintWriter out = resp.getWriter();
+            
+            for(Part part: req.getParts())
+            {
+                out.printf("Got part: name=%s, size=%,d, filename=%s%n",part.getName(), part.getSize(), part.getSubmittedFileName());
+            }
+        }
+    }
+    
+    private static Server server;
+    private static LocalConnector connector;
+    private static File locationDir;
+    
+    @BeforeClass
+    public static void startServer() throws Exception
+    {
+        Path tmpDir = MavenTestingUtils.getTargetTestingPath("testrequest_getparts");
+        FS.ensureEmpty(tmpDir);
+        
+        locationDir = tmpDir.toFile();
+        
+        server = new Server();
+        connector = new LocalConnector(server);
+        server.addConnector(connector);
+        
+        ServletContextHandler context = new ServletContextHandler();
+        context.setContextPath("/");
+        server.setHandler(context);
+        
+        ServletHolder holder = context.addServlet(DumpPartInfoServlet.class,"/dump/*");
+        String location = locationDir.getAbsolutePath();
+        long maxFileSize = 1024*1024*5;
+        long maxRequestSize = 1024*1024*10;
+        int fileSizeThreshold = 1;
+        MultipartConfigElement multipartConfig = new MultipartConfigElement(location,maxFileSize,maxRequestSize,fileSizeThreshold);
+        ((ServletHolder.Registration) holder.getRegistration()).setMultipartConfig(multipartConfig);
+        
+        server.start();
+    }
+    
+    @AfterClass
+    public static void stopServer() throws Exception
+    {
+        server.stop();
+    }
+    
+    @Test
+    public void testMultiFileUpload_SameName() throws Exception
+    {
+        // generated and parsed test
+        HttpTester.Request request = HttpTester.newRequest();
+        HttpTester.Response response;
+
+        // test GET
+        request.setMethod("POST");
+        request.setURI("/dump/");
+        request.setVersion("HTTP/1.1");
+        request.setHeader("Host","tester");
+        request.setHeader("Connection","close");
+
+        String boundary="XyXyXy";
+        request.setHeader("Content-Type","multipart/form-data; boundary=" + boundary);
+        
+        String crocMsg = "See ya later, aligator.";
+        String aligMsg = "In a while, crocodile.";
+
+        StringBuilder content = new StringBuilder();
+        content.append("--").append(boundary).append("\r\n");
+        content.append("Content-Disposition: form-data; name=\"same\"; filename=\"crocodile.dat\"\r\n");
+        content.append("Content-Type: application/octet-stream\r\n");
+        content.append("\r\n");
+        content.append(crocMsg).append("\r\n");
+        content.append("--").append(boundary).append("\r\n");
+        content.append("Content-Disposition: form-data; name=\"same\"; filename=\"aligator.dat\"\r\n");
+        content.append("Content-Type: application/octet-stream\r\n");
+        content.append("\r\n");
+        content.append(aligMsg).append("\r\n");
+        content.append("--").append(boundary).append("--\r\n");
+        content.append("\r\n");
+        
+        request.setContent(content.toString());
+
+        response = HttpTester.parseResponse(connector.getResponses(request.generate()));
+        assertThat("Response status", response.getStatus(), is(HttpServletResponse.SC_OK));
+        assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+        
+        String responseContents = response.getContent();
+        assertThat("response.contents", responseContents, containsString(String.format("Got part: name=same, size=%d, filename=crocodile.dat",crocMsg.length())));
+        assertThat("response.contents", responseContents, containsString(String.format("Got part: name=same, size=%d, filename=aligator.dat",aligMsg.length())));
+    }
+}

jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java

@@ -18,9 +18,10 @@
 
 package org.eclipse.jetty.servlets;
 
-
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -30,10 +31,15 @@ import static org.junit.Assert.assertTrue;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.io.Reader;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.util.EnumSet;
+import java.util.Enumeration;
 import java.util.Map;
 
 import javax.servlet.DispatcherType;
@@ -45,9 +51,12 @@ import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlet.ServletTester;
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.IO;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class MultipartFilterTest
@@ -56,16 +65,45 @@ public class MultipartFilterTest
     private ServletTester tester;
     FilterHolder multipartFilter;
     
+    @SuppressWarnings("serial")
     public static class FilenameServlet extends TestServlet
     {
         @Override
         protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
         {
-            assertNotNull(req.getAttribute("fileup"));
+            assertThat(req.getAttribute("fileup"), notNullValue());
             super.doPost(req, resp);
         }
     }
+    
+    @SuppressWarnings("serial")
+    public static class ParameterListServlet extends TestServlet
+    {
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            resp.setContentType("text/plain");
+            PrintWriter out = resp.getWriter();
+            
+            Enumeration<String> pnames = req.getParameterNames();
+            while (pnames.hasMoreElements())
+            {
+                String pname = pnames.nextElement();
+                Object param = req.getParameter(pname);
+                out.printf("Parameter[%s] = ",pname);
+                if (param == null)
+                {
+                    out.println(" <null>");
+                }
+                else
+                {
+                    out.printf("(%s) %s%n",param.getClass().getName(),param);
+                }
+            }
+        }
+    }
 
+    @SuppressWarnings("serial")
     public static class BoundaryServlet extends TestServlet
     {
         @Override
@@ -73,9 +111,11 @@ public class MultipartFilterTest
         {
             //we have configured the multipart filter to always store to disk (file size threshold == 1)
             //but fileName has no filename param, so only the attribute should be set
-            assertNull(req.getParameter("fileName"));
+            assertThat("getParameter('fileName')", req.getParameter("fileName"), nullValue());
-            assertNotNull(req.getAttribute("fileName"));
+            assertThat("getAttribute('fileName')", req.getAttribute("fileName"), notNullValue());
+            
             File f = (File)req.getAttribute("fileName");
+            assertThat("File exists", f.exists(), is(true));
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             IO.copy(new FileInputStream(f), baos);
             assertEquals(getServletContext().getAttribute("fileName"), baos.toString());
@@ -93,34 +133,36 @@ public class MultipartFilterTest
         }
     }
     
+    @SuppressWarnings("serial")
     public static class TestServlet extends DumpServlet
     {
 
+        @SuppressWarnings("deprecation")
         @Override
         protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
         {
-            assertNotNull(req.getParameter("fileup"));
+            String fileup = req.getParameter("fileup");
-            System.err.println("Fileup="+req.getParameter("fileup"));
+            assertThat("getParameter('fileup')",fileup,notNullValue());
-            assertNotNull(req.getParameter("fileup"+MultiPartFilter.CONTENT_TYPE_SUFFIX));
+            
-            assertEquals(req.getParameter("fileup"+MultiPartFilter.CONTENT_TYPE_SUFFIX), "application/octet-stream");
+            System.err.println("Fileup=" + req.getParameter("fileup"));
+            
+            String fileupType = req.getParameter("fileup" + MultiPartFilter.CONTENT_TYPE_SUFFIX);
+            assertThat("req.getParameter('fileup'+CONTENT_TYPE_SUFFIX)",fileupType,is("application/octet-stream"));
             super.doPost(req, resp);
         }
-
     }
 
-
+    @SuppressWarnings("deprecation")
-
     @Before
     public void setUp() throws Exception
     {
-        _dir = File.createTempFile("testmultupart",null);
+        Path tmpDir = MavenTestingUtils.getTargetTestingPath("testmultupart");
-        assertTrue(_dir.delete());
+        FS.ensureEmpty(tmpDir);
-        assertTrue(_dir.mkdir());
+        
-        _dir.deleteOnExit();
+        _dir = tmpDir.toFile();
-        assertTrue(_dir.isDirectory());
 
         tester=new ServletTester("/context");
-        tester.getContext().setResourceBase(_dir.getCanonicalPath());
+        tester.getContext().setResourceBase(tmpDir.toString());
         tester.getContext().addServlet(TestServlet.class, "/");
         tester.getContext().setAttribute("javax.servlet.context.tempdir", _dir);
         multipartFilter = tester.getContext().addFilter(MultiPartFilter.class,"/*", EnumSet.of(DispatcherType.REQUEST));
@@ -790,6 +832,7 @@ public class MultipartFilterTest
         assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus());
     }
 
+    @SuppressWarnings("serial")
     public static class TestServletParameterMap extends DumpServlet
     {
         @Override
@@ -800,7 +843,7 @@ public class MultipartFilterTest
             super.doPost(req, resp);
         }
     }
-
+    
     /**
      * Validate that the getParameterMap() call is correctly unencoding the parameters in the
      * map that it returns.
@@ -830,7 +873,7 @@ public class MultipartFilterTest
         "Content-Type: application/octet-stream\r\n\r\n"+
         "How now brown cow."+
         "\r\n--" + boundary + "\r\n"+
-        "Content-Disposition: form-data; name=\"strup\""+
+        "Content-Disposition: form-data; name=\"strup\""+ // FIXME: this is missing a "\r\n"??
         "Content-Type: application/octet-stream\r\n\r\n"+
         "How now brown cow."+
         "\r\n--" + boundary + "--\r\n\r\n";
@@ -841,6 +884,119 @@ public class MultipartFilterTest
         assertEquals(HttpServletResponse.SC_OK,response.getStatus());
         assertTrue(response.getContent().indexOf("brown cow")>=0);
     }
+    
+    /**
+     * Validate that the uploaded file can be accessed on the name it was given
+     * @throws Exception on test failure
+     */
+    @Test
+    @Ignore("Fails for Bug #486394")
+    public void testFileUpload_AccessViaFilename() throws Exception
+    {
+        tester.addServlet(ParameterListServlet.class,"/paramlist");
+        
+        multipartFilter.setInitParameter("fileOutputBuffer", "1");
+        multipartFilter.setInitParameter("deleteFiles", "false");
+        multipartFilter.setInitParameter("writeFilesWithFilenames", "true");
+
+        // generated and parsed test
+        HttpTester.Request request = HttpTester.newRequest();
+        HttpTester.Response response;
+
+        // test GET
+        request.setMethod("POST");
+        request.setURI("/context/paramlist");
+        request.setVersion("HTTP/1.1");
+        request.setHeader("Host","tester");
+        request.setHeader("Connection","close");
+        String boundary="XyXyXy";
+        request.setHeader("Content-Type","multipart/form-data; boundary=" + boundary);
+
+        StringBuilder content = new StringBuilder();
+        content.append("--").append(boundary).append("\r\n");
+        content.append("Content-Disposition: form-data; name=\"file\"; filename=\"tiny.dat\"\r\n");
+        content.append("Content-Type: application/octet-stream\r\n");
+        content.append("\r\n");
+        content.append("How now brown cow.\r\n");
+        content.append("--").append(boundary).append("--\r\n");
+        content.append("\r\n");
+        
+        request.setContent(content.toString());
+
+        response = HttpTester.parseResponse(tester.getResponses(request.generate()));
+        assertThat("Response status", response.getStatus(), is(HttpServletResponse.SC_OK));
+        assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+        
+        String contents = assertUploadedFileExists("tiny.dat");
+        assertThat("contents", contents, containsString("How now brown cow."));
+    }
+    
+    /**
+     * Validate that the two upload files, with the same name="" on two different parts,
+     * can be accessed with the filename="" portions.
+     * @throws Exception on test failure
+     */
+    @Test
+    @Ignore("Fails for Bug #486394")
+    public void testTwoFileUploads_AccessViaFilename() throws Exception
+    {
+        tester.addServlet(ParameterListServlet.class,"/paramlist");
+        
+        multipartFilter.setInitParameter("fileOutputBuffer", "1");
+        multipartFilter.setInitParameter("deleteFiles", "false");
+        multipartFilter.setInitParameter("writeFilesWithFilenames", "true");
+
+        // generated and parsed test
+        HttpTester.Request request = HttpTester.newRequest();
+        HttpTester.Response response;
+
+        // test GET
+        request.setMethod("POST");
+        request.setURI("/context/paramlist");
+        request.setVersion("HTTP/1.1");
+        request.setHeader("Host","tester");
+        request.setHeader("Connection","close");
+
+        String boundary="XyXyXy";
+        request.setHeader("Content-Type","multipart/form-data; boundary=" + boundary);
+
+        StringBuilder content = new StringBuilder();
+        content.append("--").append(boundary).append("\r\n");
+        content.append("Content-Disposition: form-data; name=\"same\"; filename=\"crocodile.dat\"\r\n");
+        content.append("Content-Type: application/octet-stream\r\n");
+        content.append("\r\n");
+        content.append("See ya later, aligator.\r\n");
+        content.append("--").append(boundary).append("\r\n");
+        content.append("Content-Disposition: form-data; name=\"same\"; filename=\"aligator.dat\"\r\n");
+        content.append("Content-Type: application/octet-stream\r\n");
+        content.append("\r\n");
+        content.append("In a while, crocodile.\r\n");
+        content.append("--").append(boundary).append("--\r\n");
+        content.append("\r\n");
+        
+        request.setContent(content.toString());
+
+        response = HttpTester.parseResponse(tester.getResponses(request.generate()));
+        assertThat("Response status", response.getStatus(), is(HttpServletResponse.SC_OK));
+        assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+        
+        String contents = assertUploadedFileExists("crocodile.dat");
+        assertThat("contents", contents, containsString("See ya later, aligator."));
+        
+        contents = assertUploadedFileExists("aligator.dat");
+        assertThat("contents", contents, containsString("In a while, crocodile."));
+    }
+    
+    private String assertUploadedFileExists(String filename) throws IOException
+    {
+        File uploadedFile = new File(_dir,filename);
+        assertThat("Uploaded File[" + uploadedFile + "].exists",uploadedFile.exists(),is(true));
+        
+        try (Reader reader = new FileReader(uploadedFile))
+        {
+            return IO.toString(reader);
+        }
+    }
 
     public static class TestServletCharSet extends HttpServlet
     {
@@ -947,13 +1103,9 @@ public class MultipartFilterTest
         assertTrue(response.getContent().indexOf("000")>=0);
     }
     
-    
+    @SuppressWarnings("serial")
-    
     public static class DumpServlet extends HttpServlet
     {
-        private static final long serialVersionUID = 201012011130L;
-
-        /* ------------------------------------------------------------ */
         /**
          * @see javax.servlet.http.HttpServlet#doPost(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
          */