From e117fbe8281bd43ec5601a30d39f50a09cbcef22 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 10 Aug 2020 10:31:16 +0200 Subject: [PATCH] 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 --- .../server/MultiPartFormInputStream.java | 39 +++++----- .../server/MultiPartFormInputStreamTest.java | 78 ++++++++++++++++++- 2 files changed, 96 insertions(+), 21 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java b/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java index 5af0c8913fc..bdc9fb1bcf4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java +++ b/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, target, StandardCopyOption.REPLACE_EXISTING); - _file = target.toFile(); + Files.move(src, p, StandardCopyOption.REPLACE_EXISTING); + _file = p.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 - if (_config.getLocation() == null) - _tmpDir = _contextTmpDir; - else if ("".equals(_config.getLocation())) - _tmpDir = _contextTmpDir; + // Sort out the location to which to write files: + // If there is a MultiPartConfigElement.location, use it + // otherwise default to the context tmp dir + if (StringUtil.isBlank(_config.getLocation())) + _tmpDir = _contextTmpDir.toPath(); else { - File f = new File(_config.getLocation()); - if (f.isAbsolute()) - _tmpDir = f; - else - _tmpDir = new File(_contextTmpDir, _config.getLocation()); + // If the MultiPartConfigElement.location is + // relative, make it relative to the context tmp dir + Path location = FileSystems.getDefault().getPath(_config.getLocation()); + _tmpDir = (location.isAbsolute() ? location : _contextTmpDir.toPath().resolve(location)); } - if (!_tmpDir.exists()) - _tmpDir.mkdirs(); + if (!Files.exists(_tmpDir)) + Files.createDirectories(_tmpDir); String contentTypeBoundary = ""; int bstart = _contentType.indexOf("boundary="); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java index cf2d11bd61d..0c37219c375 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java +++ b/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)); @@ -474,6 +475,77 @@ public class MultiPartFormInputStreamTest assertThat(tptfd.exists(), is(true)); //explicitly written file did not get removed after cleanup 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