Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #5451 - Cleanup of temp file usages. #5453

Merged
merged 4 commits into from Oct 16, 2020
Merged

Issue #5451 - Cleanup of temp file usages. #5453

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdd880b
Issue #5451 - Cleanup of temp file cleanup.
joakime Oct 15, 2020
6726a50
fix test
olamy Oct 16, 2020
6f98b13
fix tests
olamy Oct 16, 2020
369940c
Issue #5451 - Proper use of WorkDir
joakime Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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