Issue #4383 - Updates behavior to eliminate ISE

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Jan 15, 2020 · eba24b6 · eba24b6
1 parent 37f8503
commit eba24b6
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 20 deletions.
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormInputStream.java
@@ -60,10 +60,10 @@ public class MultiPartFormInputStream
 {
     private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
     private static final MultiMap<Part> EMPTY_MAP = new MultiMap<>(Collections.emptyMap());
+    private final MultiMap<Part> _parts;
     private InputStream _in;
     private MultipartConfigElement _config;
     private String _contentType;
-    private MultiMap<Part> _parts;
     private Throwable _err;
     private File _tmpDir;
     private File _contextTmpDir;
@@ -341,16 +341,19 @@ public MultiPartFormInputStream(InputStream in, String contentType, MultipartCon
         if (_config == null)
             _config = new MultipartConfigElement(_contextTmpDir.getAbsolutePath());
 
+        MultiMap parts = new MultiMap();
+
         if (in instanceof ServletInputStream)
         {
             if (((ServletInputStream)in).isFinished())
             {
-                _parts = EMPTY_MAP;
+                parts = EMPTY_MAP;
                 _parsed = true;
-                return;
             }
         }
-        _in = new BufferedInputStream(in);
+        if (!_parsed)
+            _in = new BufferedInputStream(in);
+        _parts = parts;
     }
 
     /**
@@ -397,13 +400,6 @@ public Collection<Part> getParsedParts()
      */
     public void deleteParts()
     {
-        if (_parts == null)
-        {
-            // If we call deleteParts at this point, we are considered CLOSED
-            _err = new IllegalStateException("CLOSED via call to deleteParts()");
-            return;
-        }
-
         MultiException err = null;
         for (List<Part> parts : _parts.values())
         {
@@ -439,6 +435,9 @@ public Collection<Part> getParts() throws IOException
             parse();
         throwIfError();
 
+        if (_parts.isEmpty())
+            return Collections.emptyList();
+
         Collection<List<Part>> values = _parts.values();
         List<Part> parts = new ArrayList<>();
         for (List<Part> o : values)
@@ -499,9 +498,6 @@ protected void parse()
         Handler handler = new Handler();
         try
         {
-            // initialize
-            _parts = new MultiMap<>();
-
             // if its not a multipart request, don't parse it
             if (_contentType == null || !_contentType.startsWith("multipart/form-data"))
                 return;

diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormInputStreamTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormInputStreamTest.java
@@ -510,17 +510,13 @@ public void testPartTmpFileDeletion() throws Exception
     }
 
     @Test
-    public void testParseAfterCleanUp() throws Exception
+    public void testDeleteNPE()
     {
         final InputStream input = new ByteArrayInputStream(createMultipartRequestString("myFile").getBytes());
         MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 1024, 50);
         MultiPartFormInputStream mpis = new MultiPartFormInputStream(input, _contentType, config, _tmpDir);
 
-        mpis.deleteParts();
-
-        // The call to getParts should throw because we have already cleaned up the parts.
-        Throwable error = assertThrows(IllegalStateException.class, mpis::getParts);
-        assertThat(error.getMessage(), containsString("CLOSED"));
+        mpis.deleteParts(); // this should not be an NPE
     }
 
     @Test