From ca882c8deebc354d4de59d9a3ab4ffc4e8c40b79 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 2 Nov 2016 15:40:59 +1100 Subject: [PATCH] Issue #240 No error if no parts because input stream already consumed. --- .../jetty/servlets/MultiPartFilter.java | 5 +- .../jetty/servlets/MultipartFilterTest.java | 72 +++++++++++++++++++ .../util/MultiPartInputStreamParser.java | 29 +++++--- .../jetty/util/MultiPartInputStreamTest.java | 46 ++++++++++++ 4 files changed, 141 insertions(+), 11 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java index d68e016d704..6232a1f9dd7 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java @@ -149,7 +149,6 @@ public class MultiPartFilter implements Filter return; } - InputStream in = new BufferedInputStream(request.getInputStream()); String content_type=srequest.getContentType(); //Get current parameters so we can merge into them @@ -164,12 +163,12 @@ public class MultiPartFilter implements Filter } MultipartConfigElement config = new MultipartConfigElement(tempdir.getCanonicalPath(), _maxFileSize, _maxRequestSize, _fileOutputBuffer); - MultiPartInputStreamParser mpis = new MultiPartInputStreamParser(in, content_type, config, tempdir); + MultiPartInputStreamParser mpis = new MultiPartInputStreamParser(request.getInputStream(), content_type, config, tempdir); mpis.setDeleteOnExit(_deleteFiles); mpis.setWriteFilesWithFilenames(_writeFilesWithFilenames); request.setAttribute(MULTIPART, mpis); try - { + { Collection parts = mpis.getParts(); if (parts != null) { diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java index 41bb58e0d0b..c5c744893fc 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java @@ -31,6 +31,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.PrintWriter; import java.nio.charset.StandardCharsets; @@ -38,7 +39,13 @@ import java.util.EnumSet; import java.util.Map; import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -46,10 +53,12 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.FilterMapping; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletTester; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiPartInputStreamParser; +import org.eclipse.jetty.util.ReadLineInputStream; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StacklessLogging; @@ -64,6 +73,33 @@ public class MultipartFilterTest private ServletTester tester; FilterHolder multipartFilter; + public static class ReadAllFilter implements Filter + { + + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + ServletInputStream is = request.getInputStream(); + ReadLineInputStream rlis = new ReadLineInputStream(request.getInputStream()); + String line = ""; + while (line != null) + { + line = rlis.readLine(); + } + chain.doFilter(request, response); + } + + @Override + public void destroy() + { + } + } + public static class NullServlet extends HttpServlet { @@ -705,6 +741,42 @@ public class MultipartFilterTest assertTrue(response.getContent().indexOf("Missing content")>=0); } } + + + @Test + public void testBodyAlreadyConsumed() + throws Exception + { + tester.addServlet(NullServlet.class,"/null"); + + FilterHolder holder = new FilterHolder(); + holder.setName("reader"); + holder.setFilter(new ReadAllFilter()); + tester.getContext().getServletHandler().addFilter(holder); + FilterMapping mapping = new FilterMapping(); + mapping.setFilterName("reader"); + mapping.setPathSpec("/*"); + tester.getContext().getServletHandler().prependFilterMapping(mapping); + String boundary="XyXyXy"; + // generated and parsed test + HttpTester.Request request = HttpTester.newRequest(); + HttpTester.Response response; + + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/null"); + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + request.setContent("How now brown cow"); + + try(StacklessLogging stackless = new StacklessLogging(ServletHandler.class)) + { + response = HttpTester.parseResponse(tester.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + } + } + + @Test public void testWhitespaceBodyWithCRLF() diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java index cecf895461c..f3a26808b8e 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java @@ -38,8 +38,10 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Map; import javax.servlet.MultipartConfigElement; +import javax.servlet.ServletInputStream; import javax.servlet.http.Part; import org.eclipse.jetty.util.log.Log; @@ -56,6 +58,7 @@ public class MultiPartInputStreamParser { private static final Logger LOG = Log.getLogger(MultiPartInputStreamParser.class); public static final MultipartConfigElement __DEFAULT_MULTIPART_CONFIG = new MultipartConfigElement(System.getProperty("java.io.tmpdir")); + public static final MultiMap EMPTY_MAP = new MultiMap(Collections.emptyMap()); protected InputStream _in; protected MultipartConfigElement _config; protected String _contentType; @@ -353,15 +356,24 @@ public class MultiPartInputStreamParser */ public MultiPartInputStreamParser (InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir) { - _in = new ReadLineInputStream(in); - _contentType = contentType; - _config = config; - _contextTmpDir = contextTmpDir; - if (_contextTmpDir == null) - _contextTmpDir = new File (System.getProperty("java.io.tmpdir")); + _contentType = contentType; + _config = config; + _contextTmpDir = contextTmpDir; + if (_contextTmpDir == null) + _contextTmpDir = new File (System.getProperty("java.io.tmpdir")); - if (_config == null) - _config = new MultipartConfigElement(_contextTmpDir.getAbsolutePath()); + if (_config == null) + _config = new MultipartConfigElement(_contextTmpDir.getAbsolutePath()); + + if (in instanceof ServletInputStream) + { + if (((ServletInputStream)in).isFinished()) + { + _parts = EMPTY_MAP; + return; + } + } + _in = new ReadLineInputStream(in); } /** @@ -477,6 +489,7 @@ public class MultiPartInputStreamParser if (_parts != null || _err != null) return; + //initialize long total = 0; //keep running total of size of bytes read from input and throw an exception if exceeds MultipartConfigElement._maxRequestSize _parts = new MultiMap<>(); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java index 5496450243f..0ae86bf46a1 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java @@ -38,7 +38,9 @@ import java.io.InputStream; import java.util.Collection; import javax.servlet.MultipartConfigElement; +import javax.servlet.ReadListener; import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; import javax.servlet.http.Part; import org.eclipse.jetty.util.MultiPartInputStreamParser.MultiPart; @@ -237,6 +239,50 @@ public class MultiPartInputStreamTest } } + + @Test + public void testBodyAlreadyConsumed() + throws Exception + { + ServletInputStream is = new ServletInputStream() { + + @Override + public boolean isFinished() + { + return true; + } + + @Override + public boolean isReady() + { + return false; + } + + @Override + public void setReadListener(ReadListener readListener) + { + } + + @Override + public int read() throws IOException + { + return 0; + } + + }; + + MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50); + MultiPartInputStreamParser mpis = new MultiPartInputStreamParser(is, + _contentType, + config, + _tmpDir); + mpis.setDeleteOnExit(true); + Collection parts = mpis.getParts(); + assertEquals(0, parts.size()); + } + + + @Test public void testWhitespaceBodyWithCRLF() throws Exception