Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #3683 - ensure multipart files are still cleaned up after errors 

- do not parse MultiParts in constructor so the attribute can be set
- deleteParts in MultiPartFormInputStream from _parts MultiMap
- only add the MultiPartCleanerListener once per context

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2019-06-03 13:52:50 +10:00
parent 970b030c4a
commit 17b8eb8401
7 changed files with 84 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java
View File

@ -299,9 +299,8 @@ public class MultiPartFormInputStream
*/
public void cleanUp() throws IOException
{
if (_temporary && _file != null && _file.exists())
if (!_file.delete())
throw new IOException("Could Not Delete File");
if (_temporary)
delete();
}
/**
@ -398,31 +397,21 @@ public class MultiPartFormInputStream
*/
public void deleteParts()
{
if (!_parsed)
return;
Collection<Part> parts;
try
{
parts = getParts();
}
catch (IOException e)
{
throw new RuntimeException(e);
}
MultiException err = null;
for (Part p : parts)
for (List<Part> parts : _parts.values())
{
try
for (Part p : parts)
{
((MultiPart)p).cleanUp();
}
catch (Exception e)
{
if (err == null)
err = new MultiException();
err.add(e);
try
{
((MultiPart)p).cleanUp();
}
catch (Exception e)
{
if (err == null)
err = new MultiException();
err.add(e);
}
}
}
_parts.clear();
@ -477,6 +466,9 @@ public class MultiPartFormInputStream
{
if (_err != null)
{
if (LOG.isDebugEnabled())
LOG.debug("MultiPart latched exception ", _err);
_err.addSuppressed(new Throwable());
if (_err instanceof IOException)
throw (IOException)_err;

15
jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java
View File

@ -131,12 +131,8 @@ public class MultiPartParser
private HttpTokens.Token next(ByteBuffer buffer)
{
byte ch = buffer.get();
HttpTokens.Token t = HttpTokens.TOKENS[0xff & ch];
if (DEBUG)
LOG.debug("token={}",t);
switch(t.getType())
{
case CNTL:
@ -271,6 +267,9 @@ public class MultiPartParser
/* ------------------------------------------------------------------------------- */
private void parsePreamble(ByteBuffer buffer)
{
if (LOG.isDebugEnabled())
LOG.debug("parsePreamble({})", BufferUtil.toDetailString(buffer));
if (_partialBoundary > 0)
{
int partial = _delimiterSearch.startsWith(buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.remaining(), _partialBoundary);
@ -307,6 +306,9 @@ public class MultiPartParser
/* ------------------------------------------------------------------------------- */
private void parseDelimiter(ByteBuffer buffer)
{
if (LOG.isDebugEnabled())
LOG.debug("parseDelimiter({})", BufferUtil.toDetailString(buffer));
while (__delimiterStates.contains(_state) && hasNextByte(buffer))
{
HttpTokens.Token t = next(buffer);
@ -354,6 +356,9 @@ public class MultiPartParser
*/
protected boolean parseMimePartHeaders(ByteBuffer buffer)
{
if (LOG.isDebugEnabled())
LOG.debug("parseMimePartHeaders({})", BufferUtil.toDetailString(buffer));
// Process headers
while (_state == State.BODY_PART && hasNextByte(buffer))
{
@ -575,6 +580,8 @@ public class MultiPartParser
protected boolean parseOctetContent(ByteBuffer buffer)
{
if (LOG.isDebugEnabled())
LOG.debug("parseOctetContent({})", BufferUtil.toDetailString(buffer));
// Starts With
if (_partialBoundary > 0)

9
jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormInputStreamTest.java
View File

@ -217,6 +217,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testNoBody()
throws Exception
{
String body = "";
@ -277,6 +278,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testWhitespaceBodyWithCRLF()
throws Exception
{
String whitespace = " \n\n\n\r\n\r\n\r\n\r\n";
@ -292,6 +294,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testWhitespaceBody()
throws Exception
{
String whitespace = " ";
@ -400,6 +403,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testRequestTooBig ()
throws Exception
{
MultipartConfigElement config = new MultipartConfigElement(_dirname, 60, 100, 50);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@ -415,6 +419,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testRequestTooBigThrowsErrorOnGetParts ()
throws Exception
{
MultipartConfigElement config = new MultipartConfigElement(_dirname, 60, 100, 50);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@ -434,6 +439,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testFileTooBig()
throws Exception
{
MultipartConfigElement config = new MultipartConfigElement(_dirname, 40, 1024, 30);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@ -449,6 +455,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testFileTooBigThrowsErrorOnGetParts()
throws Exception
{
MultipartConfigElement config = new MultipartConfigElement(_dirname, 40, 1024, 30);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(new ByteArrayInputStream(_multi.getBytes()),
@ -550,6 +557,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testCROnlyRequest()
throws Exception
{
String str = "--AaB03x\r" +
"content-disposition: form-data; name=\"field1\"\r" +
@ -576,6 +584,7 @@ public class MultiPartFormInputStreamTest
@Test
public void testCRandLFMixRequest()
throws Exception
{
String str = "--AaB03x\r" +
"content-disposition: form-data; name=\"field1\"\r" +

105
jetty-server/src/main/java/org/eclipse/jetty/server/MultiParts.java
View File

@ -26,7 +26,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import javax.servlet.MultipartConfigElement;
import javax.servlet.http.Part;
@ -44,13 +43,12 @@ import org.eclipse.jetty.util.MultiPartInputStreamParser.NonCompliance;
*/
public interface MultiParts extends Closeable
{
public Collection<Part> getParts();
public Part getPart(String name);
public boolean isEmpty();
public ContextHandler.Context getContext();
public class MultiPartsHttpParser implements MultiParts
Collection<Part> getParts() throws IOException;
Part getPart(String name) throws IOException;
boolean isEmpty();
ContextHandler.Context getContext();
class MultiPartsHttpParser implements MultiParts
{
private final MultiPartFormInputStream _httpParser;
private final ContextHandler.Context _context;
@ -59,32 +57,18 @@ public interface MultiParts extends Closeable
{
_httpParser = new MultiPartFormInputStream(in, contentType, config, contextTmpDir);
_context = request.getContext();
_httpParser.getParts();
}
@Override
public Collection<Part> getParts()
public Collection<Part> getParts() throws IOException
{
try
{
return _httpParser.getParts();
}
catch (IOException e)
{
throw new RuntimeException(e);
}
return _httpParser.getParts();
}
@Override
public Part getPart(String name) {
try
{
return _httpParser.getPart(name);
}
catch (IOException e)
{
throw new RuntimeException(e);
}
public Part getPart(String name) throws IOException
{
return _httpParser.getPart(name);
}
@Override
@ -104,65 +88,36 @@ public interface MultiParts extends Closeable
{
return _context;
}
}
@SuppressWarnings("deprecation")
public class MultiPartsUtilParser implements MultiParts
@SuppressWarnings("deprecation")
class MultiPartsUtilParser implements MultiParts
{
private final MultiPartInputStreamParser _utilParser;
private final ContextHandler.Context _context;
private final Request _request;
public MultiPartsUtilParser(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, Request request) throws IOException
{
_utilParser = new MultiPartInputStreamParser(in, contentType, config, contextTmpDir);
_context = request.getContext();
_utilParser.getParts();
EnumSet<NonCompliance> nonComplianceWarnings = _utilParser.getNonComplianceWarnings();
if (!nonComplianceWarnings.isEmpty())
{
@SuppressWarnings("unchecked")
List<String> violations = (List<String>)request.getAttribute(HttpCompliance.VIOLATIONS_ATTR);
if (violations==null)
{
violations = new ArrayList<>();
request.setAttribute(HttpCompliance.VIOLATIONS_ATTR,violations);
}
for(NonCompliance nc : nonComplianceWarnings)
violations.add(nc.name()+": "+nc.getURL());
System.out.println(violations);
}
_request = request;
}
@Override
public Collection<Part> getParts()
public Collection<Part> getParts() throws IOException
{
try
{
return _utilParser.getParts();
}
catch (IOException e)
{
throw new RuntimeException(e);
}
Collection<Part> parts = _utilParser.getParts();
setNonComplianceViolationsOnRequest();
return parts;
}
@Override
public Part getPart(String name)
public Part getPart(String name) throws IOException
{
try
{
return _utilParser.getPart(name);
}
catch (IOException e)
{
throw new RuntimeException(e);
}
Part part = _utilParser.getPart(name);
setNonComplianceViolationsOnRequest();
return part;
}
@Override
@ -182,5 +137,19 @@ public interface MultiParts extends Closeable
{
return _context;
}
private void setNonComplianceViolationsOnRequest()
{
@SuppressWarnings("unchecked")
List<String> violations = (List<String>)_request.getAttribute(HttpCompliance.VIOLATIONS_ATTR);
if (violations != null)
return;
EnumSet<NonCompliance> nonComplianceWarnings = _utilParser.getNonComplianceWarnings();
violations = new ArrayList<>();
for (NonCompliance nc : nonComplianceWarnings)
violations.add(nc.name() + ": " + nc.getURL());
_request.setAttribute(HttpCompliance.VIOLATIONS_ATTR, violations);
}
}
}

14
jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View File

@ -487,7 +487,7 @@ public class Request implements HttpServletRequest
throw new BadMessageException(HttpStatus.NOT_IMPLEMENTED_501,"Unsupported Content-Encoding");
getParts(_contentParameters);
}
catch (IOException | ServletException e)
catch (IOException e)
{
LOG.debug(e);
throw new RuntimeIOException(e);
@ -2320,7 +2320,6 @@ public class Request implements HttpServletRequest
public Part getPart(String name) throws IOException, ServletException
{
getParts();
return _multiParts.getPart(name);
}
@ -2334,7 +2333,7 @@ public class Request implements HttpServletRequest
return getParts(null);
}
private Collection<Part> getParts(MultiMap<String> params) throws IOException, ServletException
private Collection<Part> getParts(MultiMap<String> params) throws IOException
{
if (_multiParts == null)
_multiParts = (MultiParts)getAttribute(__MULTIPARTS);
@ -2345,12 +2344,9 @@ public class Request implements HttpServletRequest
if (config == null)
throw new IllegalStateException("No multipart config for servlet");
_multiParts = newMultiParts(getInputStream(),
_contentType, config,
(_context != null?(File)_context.getAttribute("javax.servlet.context.tempdir"):null));
_multiParts = newMultiParts(config);
setAttribute(__MULTIPARTS, _multiParts);
Collection<Part> parts = _multiParts.getParts(); //causes parsing
Collection<Part> parts = _multiParts.getParts();
String _charset_ = null;
Part charsetPart = _multiParts.getPart("_charset_");
@ -2412,7 +2408,7 @@ public class Request implements HttpServletRequest
}
private MultiParts newMultiParts(ServletInputStream inputStream, String contentType, MultipartConfigElement config, Object object) throws IOException
private MultiParts newMultiParts(MultipartConfigElement config) throws IOException
{
MultiPartFormDataCompliance compliance = getHttpChannel().getHttpConfiguration().getMultipartFormDataCompliance();
if(LOG.isDebugEnabled())

7
jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java
View File

@ -747,11 +747,14 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
//cleaned up correctly
if (((Registration)getRegistration()).getMultipartConfig() != null)
{
if (LOG.isDebugEnabled())
LOG.debug("multipart cleanup listener added for {}", this);
//Register a listener to delete tmp files that are created as a result of this
//servlet calling Request.getPart() or Request.getParts()
ContextHandler ch = ContextHandler.getContextHandler(getServletHandler().getServletContext());
ch.addEventListener(MultiPartCleanerListener.INSTANCE);
if(!Arrays.asList(ch.getEventListeners()).contains(MultiPartCleanerListener.INSTANCE))
ch.addEventListener(MultiPartCleanerListener.INSTANCE);
}
}

2
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/MultiPartServletTest.java
View File

@ -61,7 +61,6 @@ public class MultiPartServletTest
private Server server;
private ServerConnector connector;
private HttpClient client;
private Path tmpDir;
private static final int MAX_FILE_SIZE = 512 * 1024;
@ -92,7 +91,6 @@ public class MultiPartServletTest
public void start() throws Exception
{
tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
System.err.println(tmpDir);
server = new Server();
connector = new ServerConnector(server);