Issue #1337 Use either absolute or relative multipart file location (#5119)

* Issue #1337 Use either absolute or relative multipart file location

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java

@@ -28,6 +28,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
@@ -99,7 +100,7 @@ public class MultiPartFormInputStream
     private final File _contextTmpDir;
     private final String _contentType;
     private volatile Throwable _err;
-    private volatile File _tmpDir;
+    private volatile Path _tmpDir;
     private volatile boolean _deleteOnExit;
     private volatile boolean _writeFilesWithFilenames;
     private volatile int _bufferSize = 16 * 1024;
@@ -185,12 +186,16 @@ public class MultiPartFormInputStream
         @Override
         public void write(String fileName) throws IOException
         {
+            Path p = Path.of(fileName);
+            if (!p.isAbsolute())
+                p = _tmpDir.resolve(p);
+            
             if (_file == null)
             {
                 _temporary = false;
 
                 // part data is only in the ByteArrayOutputStream and never been written to disk
-                _file = new File(_tmpDir, fileName);
+                _file = Files.createFile(p).toFile();
 
                 try (BufferedOutputStream bos = new BufferedOutputStream(new FileOutputStream(_file)))
                 {
@@ -208,9 +213,8 @@ public class MultiPartFormInputStream
                 _temporary = false;
 
                 Path src = _file.toPath();
-                Path target = src.resolveSibling(fileName);
+                Files.move(src, p, StandardCopyOption.REPLACE_EXISTING);
-                Files.move(src, target, StandardCopyOption.REPLACE_EXISTING);
+                _file = p.toFile();
-                _file = target.toFile();
             }
         }
 
@@ -222,7 +226,7 @@ public class MultiPartFormInputStream
             final boolean USER = true;
             final boolean WORLD = false;
 
-            _file = File.createTempFile("MultiPart", "", MultiPartFormInputStream.this._tmpDir);
+            _file = Files.createTempFile(MultiPartFormInputStream.this._tmpDir, "MultiPart", "").toFile();
             _file.setReadable(false, WORLD); // (reset) disable it for everyone first
             _file.setReadable(true, USER); // enable for user only
 
@@ -533,22 +537,21 @@ public class MultiPartFormInputStream
         MultiPartParser parser = null;
         try
         {
-            // sort out the location to which to write the files
+            // Sort out the location to which to write files:
-            if (_config.getLocation() == null)
+            // If there is a MultiPartConfigElement.location, use it
-                _tmpDir = _contextTmpDir;
+            // otherwise default to the context tmp dir
-            else if ("".equals(_config.getLocation()))
+            if (StringUtil.isBlank(_config.getLocation()))
-                _tmpDir = _contextTmpDir;
+                _tmpDir = _contextTmpDir.toPath();
             else
             {
-                File f = new File(_config.getLocation());
+                // If the MultiPartConfigElement.location is
-                if (f.isAbsolute())
+                // relative, make it relative to the context tmp dir
-                    _tmpDir = f;
+                Path location = FileSystems.getDefault().getPath(_config.getLocation());
-                else
+                _tmpDir = (location.isAbsolute() ? location : _contextTmpDir.toPath().resolve(location));
-                    _tmpDir = new File(_contextTmpDir, _config.getLocation());
             }
 
-            if (!_tmpDir.exists())
+            if (!Files.exists(_tmpDir))
-                _tmpDir.mkdirs();
+                Files.createDirectories(_tmpDir);
 
             String contentTypeBoundary = "";
             int bstart = _contentType.indexOf("boundary=");

jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java

@@ -24,6 +24,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.util.Base64;
 import java.util.Collection;
 import java.util.concurrent.CompletableFuture;
@@ -453,7 +454,7 @@ public class MultiPartFormInputStreamTest
     }
 
     @Test
-    public void testPartFileNotDeleted() throws Exception
+    public void testPartFileRelative() throws Exception
     {
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(createMultipartRequestString("tptfd").getBytes()),
@@ -465,7 +466,7 @@ public class MultiPartFormInputStreamTest
 
         MultiPart part = (MultiPart)mpis.getPart("stuff");
         File stuff = part.getFile();
-        assertThat(stuff, notNullValue()); // longer than 100 bytes, should already be a tmp file
+        assertThat(stuff, notNullValue()); // longer than 50 bytes, should already be a tmp file
         part.write("tptfd.txt");
         File tptfd = new File(_dirname + File.separator + "tptfd.txt");
         assertThat(tptfd.exists(), is(true));
@@ -475,6 +476,77 @@ public class MultiPartFormInputStreamTest
         tptfd.deleteOnExit(); //clean up test
     }
     
+    @Test
+    public void testPartFileAbsolute() throws Exception
+    {
+        MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50);
+        MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(createMultipartRequestString("tpfa").getBytes()),
+            _contentType,
+            config,
+            _tmpDir);
+        mpis.setDeleteOnExit(true);
+        mpis.getParts();
+
+        MultiPart part = (MultiPart)mpis.getPart("stuff");
+        File stuff = part.getFile();
+        assertThat(stuff, notNullValue()); // longer than 50 bytes, should already be a tmp file
+        Path path = MavenTestingUtils.getTargetTestingPath().resolve("tpfa.txt");
+        part.write(path.toFile().getAbsolutePath());
+        File tpfa = path.toFile();
+        assertThat(tpfa.exists(), is(true));
+        assertThat(stuff.exists(), is(false)); //got renamed
+        part.cleanUp();
+        assertThat(tpfa.exists(), is(true));  //explicitly written file did not get removed after cleanup
+        tpfa.deleteOnExit(); //clean up test 
+    }
+    
+    @Test
+    public void testPartFileAbsoluteFromBuffer() throws Exception
+    {
+        MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 5000);
+        MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(createMultipartRequestString("tpfafb").getBytes()),
+            _contentType,
+            config,
+            _tmpDir);
+        mpis.setDeleteOnExit(true);
+        mpis.getParts();
+
+        MultiPart part = (MultiPart)mpis.getPart("stuff");
+        //Content should still be in the buffer, because the length is < 5000, 
+        assertNull(part.getFile());
+        //test writing to an absolute filename
+        Path path = MavenTestingUtils.getTargetTestingPath().resolve("tpfafb.txt");
+        part.write(path.toFile().getAbsolutePath());
+        File tpfafb = path.toFile();
+        assertThat(tpfafb.exists(), is(true));
+        part.cleanUp();
+        assertThat(tpfafb.exists(), is(true));  //explicitly written file did not get removed after cleanup
+        tpfafb.deleteOnExit(); //clean up test 
+    }
+    
+    @Test
+    public void testPartFileRelativeFromBuffer() throws Exception
+    {
+        MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 5000);
+        MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(createMultipartRequestString("tpfrfb").getBytes()),
+            _contentType,
+            config,
+            _tmpDir);
+        mpis.setDeleteOnExit(true);
+        mpis.getParts();
+
+        MultiPart part = (MultiPart)mpis.getPart("stuff");
+        //Content should still be in the buffer, because the length is < 5000, 
+        assertNull(part.getFile());
+        //test writing to a relative filename
+        part.write("tpfrfb.txt");
+        File tpfrfb = new File(_tmpDir, "tpfrfb.txt");
+        assertThat(tpfrfb.exists(), is(true));
+        part.cleanUp();
+        assertThat(tpfrfb.exists(), is(true));  //explicitly written file did not get removed after cleanup
+        tpfrfb.deleteOnExit(); //clean up test 
+    }
+
     @Test
     public void testPartTmpFileDeletion() throws Exception
     {
@@ -488,7 +560,7 @@ public class MultiPartFormInputStreamTest
 
         MultiPart part = (MultiPart)mpis.getPart("stuff");
         File stuff = part.getFile();
-        assertThat(stuff, notNullValue()); // longer than 100 bytes, should already be a tmp file
+        assertThat(stuff, notNullValue()); // longer than 50 bytes, should already be a tmp file
         assertThat(stuff.exists(), is(true));
         part.cleanUp();
         assertThat(stuff.exists(), is(false));  //tmp file was removed after cleanup