Skip to content

Commit

Permalink
Issue #5451 - Cleanup of temp file cleanup.
Browse files Browse the repository at this point in the history 
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@joakime
joakime committed Oct 15, 2020
1 parent 7b06c0d commit 7a2a614
Show file tree
Hide file tree
Showing 17 changed files with 390 additions and 387 deletions.
7 changes: 4 additions & 3 deletions examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java
View file
Expand Up @@ -18,8 +18,9 @@

package org.eclipse.jetty.embedded;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Map;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -120,8 +121,8 @@ public static Server createServer(int port) throws IOException
gzipHandler.addIncludedMimeTypes("text/html");

// configure request logging
File requestLogFile = File.createTempFile("demo", "log");
CustomRequestLog ncsaLog = new CustomRequestLog(requestLogFile.getAbsolutePath());
Path requestLogFile = Files.createTempFile("demo", "log");
CustomRequestLog ncsaLog = new CustomRequestLog(requestLogFile.toString());
server.setRequestLog(ncsaLog);

// create the handler collections
Expand Down
31 changes: 16 additions & 15 deletions jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java
View file
Expand Up @@ -31,6 +31,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -41,6 +42,7 @@

import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.ByteArrayOutputStream2;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.LazyList;
import org.eclipse.jetty.util.MultiException;
import org.eclipse.jetty.util.MultiMap;
Expand All @@ -67,7 +69,6 @@ public class MultiPartFormInputStream
private Throwable _err;
private File _tmpDir;
private File _contextTmpDir;
private boolean _deleteOnExit;
private boolean _writeFilesWithFilenames;
private boolean _parsed;
private int _bufferSize = 16 * 1024;
Expand Down Expand Up @@ -151,19 +152,11 @@ protected void write(byte[] bytes, int offset, int length) throws IOException

protected void createFile() throws IOException
{
/*
* Some statics just to make the code below easier to understand This get optimized away during the compile anyway
*/
final boolean USER = true;
final boolean WORLD = false;
Path parent = MultiPartFormInputStream.this._tmpDir.toPath();
Path tempFile = Files.createTempFile(parent, "MultiPart", "", IO.getUserOnlyFileAttribute(parent));
_file = tempFile.toFile();

_file = File.createTempFile("MultiPart", "", MultiPartFormInputStream.this._tmpDir);
_file.setReadable(false, WORLD); // (reset) disable it for everyone first
_file.setReadable(true, USER); // enable for user only

if (_deleteOnExit)
_file.deleteOnExit();
FileOutputStream fos = new FileOutputStream(_file);
OutputStream fos = Files.newOutputStream(tempFile, StandardOpenOption.WRITE);
BufferedOutputStream bos = new BufferedOutputStream(fos);

if (_size > 0 && _out != null)
Expand Down Expand Up @@ -757,9 +750,13 @@ public void reset()
}
}

/**
* @deprecated no replacement provided.
*/
@Deprecated
public void setDeleteOnExit(boolean deleteOnExit)
{
_deleteOnExit = deleteOnExit;
// does nothing.
}

public void setWriteFilesWithFilenames(boolean writeFilesWithFilenames)
Expand All @@ -772,9 +769,13 @@ public boolean isWriteFilesWithFilenames()
return _writeFilesWithFilenames;
}

/**
* @deprecated no replacement provided
*/
@Deprecated
public boolean isDeleteOnExit()
{
return _deleteOnExit;
return false;
}

private static String value(String nameEqualsValue)
Expand Down
22 changes: 11 additions & 11 deletions jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
View file
Expand Up @@ -20,7 +20,6 @@

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
Expand All @@ -35,17 +34,21 @@
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
Expand All @@ -54,10 +57,12 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

@ExtendWith(WorkDirExtension.class)
public class IOTest
{
public WorkDir workDir;

@Test
public void testIO() throws Exception
{
Expand Down Expand Up @@ -96,7 +101,6 @@ public void testHalfClose() throws Exception
// but cannot write
Assertions.assertThrows(SocketException.class, () -> client.getOutputStream().write(1));


// but can still write in opposite direction.
server.getOutputStream().write(1);
assertEquals(1, client.getInputStream().read());
Expand Down Expand Up @@ -419,13 +423,9 @@ public void testAsyncSocketChannel() throws Exception
@Test
public void testGatherWrite() throws Exception
{
File dir = MavenTestingUtils.getTargetTestingDir();
if (!dir.exists())
dir.mkdir();

File file = File.createTempFile("test", ".txt", dir);
file.deleteOnExit();
FileChannel out = FileChannel.open(file.toPath(),
Path dir = workDir.getEmptyPathDir();
Path file = Files.createTempFile(dir, "test", ".txt");
FileChannel out = FileChannel.open(file,
StandardOpenOption.CREATE,
StandardOpenOption.READ,
StandardOpenOption.WRITE,
Expand Down
80 changes: 37 additions & 43 deletions jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View file
Expand Up @@ -27,6 +27,8 @@
import java.io.Reader;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Enumeration;
Expand Down Expand Up @@ -54,6 +56,8 @@
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.log.Log;
Expand All @@ -64,6 +68,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
Expand All @@ -81,9 +86,11 @@
import static org.junit.jupiter.api.Assertions.fail;

// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
@ExtendWith(WorkDirExtension.class)
public class RequestTest
{
private static final Logger LOG = Log.getLogger(RequestTest.class);
public WorkDir workDir;
private Server _server;
private LocalConnector _connector;
private RequestHandler _handler;
Expand Down Expand Up @@ -335,33 +342,26 @@ public boolean check(HttpServletRequest request, HttpServletResponse response) t
@Test
public void testMultiPart() throws Exception
{
final File testTmpDir = File.createTempFile("reqtest", null);
if (testTmpDir.exists())
testTmpDir.delete();
testTmpDir.mkdir();
testTmpDir.deleteOnExit();
assertTrue(testTmpDir.list().length == 0);
Path testTmpDir = workDir.getEmptyPathDir();

ContextHandler contextHandler = new ContextHandler();
contextHandler.setContextPath("/foo");
contextHandler.setResourceBase(".");
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir));
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir.toFile()));
contextHandler.addEventListener(new MultiPartCleanerListener()
{

@Override
public void requestDestroyed(ServletRequestEvent sre)
{
MultiParts m = (MultiParts)sre.getServletRequest().getAttribute(Request.MULTIPARTS);
assertNotNull(m);
ContextHandler.Context c = m.getContext();
assertNotNull(c);
assertTrue(c == sre.getServletContext());
assertTrue(!m.isEmpty());
assertTrue(testTmpDir.list().length == 2);
assertSame(c, sre.getServletContext());
assertFalse(m.isEmpty());
assertThat("File count in temp dir", getFileCount(testTmpDir), is(2L));
super.requestDestroyed(sre);
String[] files = testTmpDir.list();
assertTrue(files.length == 0);
assertThat("File count in temp dir", getFileCount(testTmpDir), is(0L));
}
});
_server.stop();
Expand Down Expand Up @@ -395,33 +395,26 @@ public void requestDestroyed(ServletRequestEvent sre)
@Test
public void testUtilMultiPart() throws Exception
{
final File testTmpDir = File.createTempFile("reqtest", null);
if (testTmpDir.exists())
testTmpDir.delete();
testTmpDir.mkdir();
testTmpDir.deleteOnExit();
assertTrue(testTmpDir.list().length == 0);
Path testTmpDir = workDir.getEmptyPathDir();

ContextHandler contextHandler = new ContextHandler();
contextHandler.setContextPath("/foo");
contextHandler.setResourceBase(".");
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir));
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir.toFile()));
contextHandler.addEventListener(new MultiPartCleanerListener()
{

@Override
public void requestDestroyed(ServletRequestEvent sre)
{
MultiParts m = (MultiParts)sre.getServletRequest().getAttribute(Request.MULTIPARTS);
assertNotNull(m);
ContextHandler.Context c = m.getContext();
assertNotNull(c);
assertTrue(c == sre.getServletContext());
assertTrue(!m.isEmpty());
assertTrue(testTmpDir.list().length == 2);
assertSame(c, sre.getServletContext());
assertFalse(m.isEmpty());
assertThat("File count in temp dir", getFileCount(testTmpDir), is(2L));
super.requestDestroyed(sre);
String[] files = testTmpDir.list();
assertTrue(files.length == 0);
assertThat("File count in temp dir", getFileCount(testTmpDir), is(0L));
}
});
_server.stop();
Expand Down Expand Up @@ -458,17 +451,12 @@ public void requestDestroyed(ServletRequestEvent sre)
@Test
public void testHttpMultiPart() throws Exception
{
final File testTmpDir = File.createTempFile("reqtest", null);
if (testTmpDir.exists())
testTmpDir.delete();
testTmpDir.mkdir();
testTmpDir.deleteOnExit();
assertTrue(testTmpDir.list().length == 0);
Path testTmpDir = workDir.getEmptyPathDir();

ContextHandler contextHandler = new ContextHandler();
contextHandler.setContextPath("/foo");
contextHandler.setResourceBase(".");
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir));
contextHandler.setHandler(new MultiPartRequestHandler(testTmpDir.toFile()));

_server.stop();
_server.setHandler(contextHandler);
Expand Down Expand Up @@ -503,17 +491,12 @@ public void testHttpMultiPart() throws Exception
public void testBadMultiPart() throws Exception
{
//a bad multipart where one of the fields has no name
final File testTmpDir = File.createTempFile("badmptest", null);
if (testTmpDir.exists())
testTmpDir.delete();
testTmpDir.mkdir();
testTmpDir.deleteOnExit();
assertTrue(testTmpDir.list().length == 0);
Path testTmpDir = workDir.getEmptyPathDir();

ContextHandler contextHandler = new ContextHandler();
contextHandler.setContextPath("/foo");
contextHandler.setResourceBase(".");
contextHandler.setHandler(new BadMultiPartRequestHandler(testTmpDir));
contextHandler.setHandler(new BadMultiPartRequestHandler(testTmpDir.toFile()));
contextHandler.addEventListener(new MultiPartCleanerListener()
{

Expand All @@ -524,10 +507,9 @@ public void requestDestroyed(ServletRequestEvent sre)
assertNotNull(m);
ContextHandler.Context c = m.getContext();
assertNotNull(c);
assertTrue(c == sre.getServletContext());
assertSame(c, sre.getServletContext());
super.requestDestroyed(sre);
String[] files = testTmpDir.list();
assertTrue(files.length == 0);
assertThat("File count in temp dir", getFileCount(testTmpDir), is(0L));
}
});
_server.stop();
Expand Down Expand Up @@ -1853,6 +1835,18 @@ public void testGetterSafeFromNullPointerException()
assertEquals(0, request.getParameterMap().size());
}

private static long getFileCount(Path path)
{
try
{
return Files.list(path).count();
}
catch (IOException e)
{
throw new RuntimeException("Unable to get file list count: " + path, e);
}
}

interface RequestTester
{
boolean check(HttpServletRequest request, HttpServletResponse response) throws IOException;
Expand Down

0 comments on commit 7a2a614

Please sign in to comment.