391080 Multipart temp files can be left on disk with MultiPartFilter

This commit is contained in:
Jan Bartel 2012-10-04 15:24:23 +10:00
parent 23fe83a689
commit 25cdef966c
5 changed files with 131 additions and 41 deletions

View File

@ -1433,6 +1433,24 @@ public class Request implements HttpServletRequest
if (_savedNewSessions != null)
_savedNewSessions.clear();
_savedNewSessions=null;
if (_multiPartInputStream != null)
{
Collection<Part> parts = _multiPartInputStream.getParsedParts();
if (parts != null)
{
for (Part p:parts)
{
try
{
p.delete();
}
catch (IOException e)
{
LOG.warn("Error deleting multipart file", e);
}
}
}
}
_multiPartInputStream = null;
}

View File

@ -84,7 +84,7 @@ import org.eclipse.jetty.util.TypeUtil;
public class MultiPartFilter implements Filter
{
public final static String CONTENT_TYPE_SUFFIX=".org.eclipse.jetty.servlet.contentType";
private final static String FILES ="org.eclipse.jetty.servlet.MultiPartFilter.files";
private final static String MULTIPART = "org.eclipse.jetty.servlet.MultiPartInputStream";
private File tempdir;
private boolean _deleteFiles;
private ServletContext _context;
@ -149,7 +149,8 @@ public class MultiPartFilter implements Filter
MultipartConfigElement config = new MultipartConfigElement(tempdir.getCanonicalPath(), _maxFileSize, _maxRequestSize, _fileOutputBuffer);
MultiPartInputStream mpis = new MultiPartInputStream(in, content_type, config, tempdir);
mpis.setDeleteOnExit(_deleteFiles);
request.setAttribute(MULTIPART, mpis);
try
{
@ -170,18 +171,6 @@ public class MultiPartFilter implements Filter
if (mp.getContentType() != null)
params.add(mp.getName()+CONTENT_TYPE_SUFFIX, mp.getContentType());
}
if (_deleteFiles)
{
mp.getFile().deleteOnExit();
ArrayList files = (ArrayList)request.getAttribute(FILES);
if (files==null)
{
files=new ArrayList();
request.setAttribute(FILES,files);
}
files.add(mp.getFile());
}
}
else
{
@ -205,23 +194,23 @@ public class MultiPartFilter implements Filter
private void deleteFiles(ServletRequest request)
{
ArrayList files = (ArrayList)request.getAttribute(FILES);
if (files!=null)
MultiPartInputStream mpis = (MultiPartInputStream)request.getAttribute(MULTIPART);
if (mpis != null)
{
Iterator iter = files.iterator();
while (iter.hasNext())
Collection<Part> parts = mpis.getParsedParts();
for (Part p:parts)
{
File file=(File)iter.next();
try
{
file.delete();
}
p.delete();
}
catch(Exception e)
{
_context.log("failed to delete "+file,e);
_context.log("Failed to delete "+p.getName(),e);
}
}
}
request.removeAttribute(MULTIPART);
}
/* ------------------------------------------------------------ */

View File

@ -121,7 +121,7 @@ public class MultipartFilterTest
response.parse(tester.getResponses(request.generate()));
assertTrue(response.getMethod()==null);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,response.getStatus());
}

View File

@ -34,6 +34,7 @@ import java.io.InputStreamReader;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -59,7 +60,7 @@ public class MultiPartInputStream
protected MultiMap<String> _parts;
protected File _tmpDir;
protected File _contextTmpDir;
protected boolean _deleteOnExit;
@ -141,6 +142,9 @@ public class MultiPartInputStream
throws IOException
{
_file = File.createTempFile("MultiPart", "", MultiPartInputStream.this._tmpDir);
if (_deleteOnExit)
_file.deleteOnExit();
FileOutputStream fos = new FileOutputStream(_file);
BufferedOutputStream bos = new BufferedOutputStream(fos);
@ -244,6 +248,8 @@ public class MultiPartInputStream
{
//part data is only in the ByteArrayOutputStream and never been written to disk
_file = new File (_tmpDir, fileName);
if (_deleteOnExit)
_file.deleteOnExit();
BufferedOutputStream bos = null;
try
{
@ -262,6 +268,8 @@ public class MultiPartInputStream
{
//the part data is already written to a temporary file, just rename it
File f = new File(_tmpDir, fileName);
if (_deleteOnExit)
f.deleteOnExit();
if (_file.renameTo(f))
_file = f;
}
@ -318,7 +326,21 @@ public class MultiPartInputStream
_config = new MultipartConfigElement(_contextTmpDir.getAbsolutePath());
}
public Collection<Part> getParsedParts()
{
if (_parts == null)
return Collections.emptyList();
Collection<Object> values = _parts.values();
List<Part> parts = new ArrayList<Part>();
for (Object o: values)
{
List<Part> asList = LazyList.getList(o, false);
parts.addAll(asList);
}
return parts;
}
public Collection<Part> getParts()
throws IOException, ServletException
@ -593,7 +615,18 @@ public class MultiPartInputStream
throw new IOException("Incomplete parts");
}
public void setDeleteOnExit(boolean deleteOnExit)
{
_deleteOnExit = deleteOnExit;
}
public boolean isDeleteOnExit()
{
return _deleteOnExit;
}
/* ------------------------------------------------------------ */
private String value(String nameEqualsValue, boolean splitAfterSpace)
{

View File

@ -50,8 +50,42 @@ public class MultiPartInputStreamTest extends TestCase
protected String _contentType = "multipart/form-data, boundary=AaB03x";
protected String _multi = createMultipartRequestString(FILENAME);
protected String _dirname = System.getProperty("java.io.tmpdir")+File.separator+"myfiles-"+System.currentTimeMillis();
protected File _tmpDir = new File(_dirname);
public MultiPartInputStreamTest ()
{
_tmpDir.deleteOnExit();
}
public void testBadMultiPartRequest()
throws Exception
{
String boundary = "X0Y0";
String str = "--" + boundary + "\r\n"+
"Content-Disposition: form-data; name=\"fileup\"; filename=\"test.upload\"\r\n"+
"Content-Type: application/octet-stream\r\n\r\n"+
"How now brown cow."+
"\r\n--" + boundary + "-\r\n\r\n";
MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50);
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(str.getBytes()),
"multipart/form-data, boundary="+boundary,
config,
_tmpDir);
mpis.setDeleteOnExit(true);
try
{
mpis.getParts();
fail ("Multipart incomplete");
}
catch (IOException e)
{
assertTrue(e.getMessage().startsWith("Incomplete"));
}
}
public void testNonMultiPartRequest()
throws Exception
{
@ -59,7 +93,8 @@ public class MultiPartInputStreamTest extends TestCase
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(_multi.getBytes()),
"Content-type: text/plain",
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
assertTrue(mpis.getParts().isEmpty());
}
@ -70,7 +105,8 @@ public class MultiPartInputStreamTest extends TestCase
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(_multi.getBytes()),
_contentType,
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
Collection<Part> parts = mpis.getParts();
assertFalse(parts.isEmpty());
}
@ -82,11 +118,12 @@ public class MultiPartInputStreamTest extends TestCase
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(_multi.getBytes()),
_contentType,
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
Collection<Part> parts = null;
try
{
mpis.getParts();
parts = mpis.getParts();
fail("Request should have exceeded maxRequestSize");
}
catch (IllegalStateException e)
@ -102,11 +139,12 @@ public class MultiPartInputStreamTest extends TestCase
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(_multi.getBytes()),
_contentType,
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
Collection<Part> parts = null;
try
{
mpis.getParts();
parts = mpis.getParts();
fail("stuff.txt should have been larger than maxFileSize");
}
catch (IllegalStateException e)
@ -133,8 +171,8 @@ public class MultiPartInputStreamTest extends TestCase
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(createMultipartRequestString(filename).getBytes()),
_contentType,
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
Collection<Part> parts = mpis.getParts();
assertThat(parts.size(), is(2));
Part field1 = mpis.getPart("field1");
@ -188,15 +226,15 @@ public class MultiPartInputStreamTest extends TestCase
"content-disposition: form-data; name=\"stuff\"; filename=\"stuff2.txt\"\r\n"+
"Content-Type: text/plain\r\n"+
"\r\n"+
"000000000000000000000000000000000000000000000000000\r\n"+
"110000000000000000000000000000000000000000000000000\r\n"+
"--AaB03x--\r\n";
MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50);
MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(sameNames.getBytes()),
_contentType,
config,
new File(_dirname));
_tmpDir);
mpis.setDeleteOnExit(true);
Collection<Part> parts = mpis.getParts();
assertEquals(2, parts.size());
for (Part p:parts)
@ -210,6 +248,18 @@ public class MultiPartInputStreamTest extends TestCase
private String createMultipartRequestString(String filename)
{
int length = filename.length();
String name = filename;
if (length > 10)
name = filename.substring(0,10);
StringBuffer filler = new StringBuffer();
int i = name.length();
while (i < 51)
{
filler.append("0");
i++;
}
return "--AaB03x\r\n"+
"content-disposition: form-data; name=\"field1\"\r\n"+
"\r\n"+
@ -217,8 +267,8 @@ public class MultiPartInputStreamTest extends TestCase
"--AaB03x\r\n"+
"content-disposition: form-data; name=\"stuff\"; filename=\"" + filename + "\"\r\n"+
"Content-Type: text/plain\r\n"+
"\r\n"+
"000000000000000000000000000000000000000000000000000\r\n"+
"\r\n"+name+
filler.toString()+"\r\n" +
"--AaB03x--\r\n";
}
}