Merge remote-tracking branch 'eclipse/jetty-9.4.x-1027-Multipart' into jetty-9.4.x-1027-Multipart

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.http;
 
 import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.EnumSet;
 
@@ -140,6 +141,7 @@ public class MultiPartParser
         DELIMITER_PADDING,
         DELIMITER_CLOSE, 
         BODY_PART,
+        FIRST_OCTETS,       
         OCTETS,       
         EPILOGUE,
         END
@@ -167,8 +169,10 @@ public class MultiPartParser
     public MultiPartParser(Handler handler, String boundary)
     {
         _handler = handler;
-        _delimiterSearch = SearchPattern.compile("\r\n--"+boundary);
+
-        _patternBuffer = ByteBuffer.wrap(boundary.getBytes());
+        String delimiter = "\r\n--"+boundary;
+        _patternBuffer = ByteBuffer.wrap(delimiter.getBytes(StandardCharsets.US_ASCII));
+        _delimiterSearch = SearchPattern.compile(_patternBuffer.array());
     }
 
     /* ------------------------------------------------------------------------------- */
@@ -323,6 +327,7 @@ public class MultiPartParser
                     handle = parseMimePartHeaders(buffer);
                     break;
                     
+                case FIRST_OCTETS:
                 case OCTETS:
                     handle = parseOctetContent(buffer);
                     break;
@@ -474,7 +479,8 @@ public class MultiPartParser
                         case LINE_FEED:
                         {
                             handleField();
-                            setState(State.OCTETS);
+                            setState(State.FIRST_OCTETS);
+                            _partialBoundary = 2; // CRLF is option for empty parts
                             if (_handler.headerComplete())
                                 return true;
                             break;
@@ -606,7 +612,6 @@ public class MultiPartParser
 
     protected boolean parseOctetContent(ByteBuffer buffer)
     {
-        
         //Starts With
         if (_partialBoundary>0)
         {
@@ -629,9 +634,16 @@ public class MultiPartParser
             {
                 //print up to _partialBoundary of the search pattern
                 ByteBuffer content = _patternBuffer.slice();
+                if (_state==State.FIRST_OCTETS)
+                {
+                    setState(State.OCTETS);
+                    content.position(2);
+                }
                 content.limit(_partialBoundary);
                 _partialBoundary = 0;
-                return _handler.content(BufferUtil.EMPTY_BUFFER, false);
+                
+                if (_handler.content(content, false))
+                    return true;
             }
         }
         
@@ -728,29 +740,4 @@ public class MultiPartParser
             LOG.warn(String.format("Illegal character 0x%X in state=%s for buffer %s",ch,state,BufferUtil.toDetailString(buffer)));
         }
     }
-    
-    
-    
-    public static void main(String[] args) {
-        String s = "hello world";
-        
-        ByteBuffer bb = ByteBuffer.wrap(s.getBytes());
-        System.out.println("bb position: "+bb.position());
-        for(int i=0; i<4; i++) 
-            System.out.print((char)bb.get()+", ");
-        System.out.println();
-        System.out.println("bb position: "+bb.position());
-        
-        //split
-        ByteBuffer bb2 = bb.slice();
-        bb2.limit(bb2.limit()-3);
-        
-        System.out.println("bb2 array offset: "+bb2.arrayOffset());
-        System.out.println("bb2 position: "+bb2.position());
-        System.out.println((char)bb2.get(0));
-        System.out.println((char)bb2.array()[0]);
-        while(bb2.hasRemaining()) 
-            System.out.print((char)bb2.get()+", ");
-    }
-    
 }

jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartParserTest.java

@@ -28,7 +28,6 @@ import java.util.List;
 import org.eclipse.jetty.http.MultiPartParser.State;
 import org.eclipse.jetty.util.BufferUtil;
 import org.hamcrest.Matchers;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class MultiPartParserTest
@@ -41,7 +40,7 @@ public class MultiPartParserTest
         ByteBuffer data = BufferUtil.toBuffer("");
         
         parser.parse(data,false);
-        assertTrue(parser.isState(State.PREAMBLE));
+        assertThat(parser.getState(),is(State.PREAMBLE));
     }
 
     @Test
@@ -144,30 +143,24 @@ public class MultiPartParserTest
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n\r\n");
         parser.parse(data,false);
-        assertTrue(parser.isState(State.OCTETS));
+        assertThat(parser.getState(),is(State.FIRST_OCTETS));
         assertThat(data.remaining(),is(0));
     }
     
     @Test
     public void testFirstPartFields()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler()
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
         {
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
             @Override
             public boolean headerComplete()
             {
-                fields.add("<<COMPLETE>>");
+                super.headerComplete();
                 return true;
             }
+        };
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
         
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -178,43 +171,17 @@ public class MultiPartParserTest
                 + "\r\n"
                 + "Content");
         parser.parse(data,false);
-        assertTrue(parser.isState(State.OCTETS));
+        assertThat(parser.getState(),is(State.FIRST_OCTETS));
         assertThat(data.remaining(),is(7));
-        assertThat(fields,Matchers.contains("name0: value0","name1: value1", "name2: value 2", "<<COMPLETE>>"));
+        assertThat(handler.fields,Matchers.contains("name0: value0","name1: value1", "name2: value 2", "<<COMPLETE>>"));
     }
     
     @Test
     public void testFirstPartNoContent()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler();
-        List<String> content = new ArrayList<>();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
-        {
         
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
-            @Override
-            public boolean headerComplete()
-            {
-                fields.add("<<COMPLETE>>");
-                return false;
-            }
-
-            @Override
-            public boolean content(ByteBuffer buffer, boolean last)
-            {
-                if (BufferUtil.hasContent(buffer))
-                    content.add(BufferUtil.toString(buffer));
-                if (last)
-                    content.add("<<LAST>>");
-                return last;
-            }
-            
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -223,47 +190,18 @@ public class MultiPartParserTest
                 + "\r\n"
                 + "--BOUNDARY");
         parser.parse(data,false);
-        //assertThat(parser.getState(), is(State.BODY_PART));
+        assertThat(parser.getState(), is(State.DELIMITER));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
-        assertThat(content,Matchers.contains("<<LAST>>"));        
+        assertThat(handler.content,Matchers.contains("<<LAST>>"));        
     }    
     
-    
-    
     @Test
-    @Ignore
     public void testFirstPartNoContentNoCRLF()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler();
-        List<String> content = new ArrayList<>();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
-        {
         
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
-            @Override
-            public boolean headerComplete()
-            {
-                fields.add("<<COMPLETE>>");
-                return false;
-            }
-
-            @Override
-            public boolean content(ByteBuffer buffer, boolean last)
-            {
-                if (BufferUtil.hasContent(buffer))
-                    content.add(BufferUtil.toString(buffer));
-                if (last)
-                    content.add("<<LAST>>");
-                return last;
-            }
-            
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -271,45 +209,42 @@ public class MultiPartParserTest
                 + "\r\n"
                 + "--BOUNDARY");
         parser.parse(data,false);
-        //assertThat(parser.getState(), is(State.BODY_PART));
+        assertThat(parser.getState(), is(State.DELIMITER));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
-        assertThat(content,Matchers.contains("<<LAST>>"));        
+        assertThat(handler.content,Matchers.contains("<<LAST>>"));  
+        
     }
 
+    @Test
+    public void testFirstPartContentLookingLikeNoCRLF()
+    {
+        TestHandler handler = new TestHandler();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
+        
+        ByteBuffer data = BufferUtil.toBuffer("");
+        
+        data = BufferUtil.toBuffer("--BOUNDARY\r\n"
+                + "name: value\r\n"
+                + "\r\n"
+                + "-");
+        parser.parse(data,false);
+        data = BufferUtil.toBuffer("Content!");
+        parser.parse(data,false);
+        
+        
+        assertThat(parser.getState(), is(State.OCTETS));
+        assertThat(data.remaining(),is(0));
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
+        assertThat(handler.content,Matchers.contains("-","Content!"));  
+    }
 
     @Test
     public void testFirstPartPartialContent()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler();
-        List<String> content = new ArrayList<>();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
-        {
         
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
-            @Override
-            public boolean headerComplete()
-            {
-                fields.add("<<COMPLETE>>");
-                return false;
-            }
-
-            @Override
-            public boolean content(ByteBuffer buffer, boolean last)
-            {
-                if (BufferUtil.hasContent(buffer))
-                    content.add(BufferUtil.toString(buffer));
-                if (last)
-                    content.add("<<LAST>>");
-                return last;
-            }
-            
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -317,10 +252,10 @@ public class MultiPartParserTest
                 + "\r\n"
                 + "Hello\r\n");
         parser.parse(data,false);
-        assertTrue(parser.isState(State.OCTETS));
+        assertThat(parser.getState(),is(State.OCTETS));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));
-        assertThat(content,Matchers.contains("Hello"));
+        assertThat(handler.content,Matchers.contains("Hello"));
 
         data = BufferUtil.toBuffer(
                 "Now is the time for all good ment to come to the aid of the party.\r\n"
@@ -328,52 +263,21 @@ public class MultiPartParserTest
                 + "The quick brown fox jumped over the lazy dog.\r\n"
                 + "this is not a --BOUNDARY\r\n");
         parser.parse(data,false);
-        assertTrue(parser.isState(State.OCTETS));
+        assertThat(parser.getState(),is(State.OCTETS));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));
-        assertThat(content,Matchers.contains("Hello","Now is the time for all good ment to come to the aid of the party.\r\n"
+        assertThat(handler.content,Matchers.contains("Hello","\r\n","Now is the time for all good ment to come to the aid of the party.\r\n"
                 + "How now brown cow.\r\n"
                 + "The quick brown fox jumped over the lazy dog.\r\n"
                 + "this is not a --BOUNDARY"));
-        
-
-            
     }
 
-    
-
     @Test
     public void testFirstPartShortContent()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler();
-        List<String> content = new ArrayList<>();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
-        {
         
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
-            @Override
-            public boolean headerComplete()
-            {
-                fields.add("<<COMPLETE>>");
-                return false;
-            }
-
-            @Override
-            public boolean content(ByteBuffer buffer, boolean last)
-            {
-                if (BufferUtil.hasContent(buffer))
-                    content.add(BufferUtil.toString(buffer));
-                if (last)
-                    content.add("<<LAST>>");
-                return last;
-            }
-            
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -384,8 +288,8 @@ public class MultiPartParserTest
         parser.parse(data,false);
         assertThat(parser.getState(), is(State.DELIMITER));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));
-        assertThat(content,Matchers.contains("Hello","<<LAST>>"));
+        assertThat(handler.content,Matchers.contains("Hello","<<LAST>>"));
         
         
     }
@@ -394,35 +298,9 @@ public class MultiPartParserTest
     @Test
     public void testFirstPartLongContent()
     {
-        List<String> fields = new ArrayList<>();
+        TestHandler handler = new TestHandler();
-        List<String> content = new ArrayList<>();
+        MultiPartParser parser = new MultiPartParser(handler,"BOUNDARY");
-        MultiPartParser parser = new MultiPartParser(new MultiPartParser.Handler()
-        {
         
-            @Override
-            public void parsedHeader(String name, String value)
-            {
-                fields.add(name+": "+value);
-            }
-
-            @Override
-            public boolean headerComplete()
-            {
-                fields.add("<<COMPLETE>>");
-                return false;
-            }
-
-            @Override
-            public boolean content(ByteBuffer buffer, boolean last)
-            {
-                if (BufferUtil.hasContent(buffer))
-                    content.add(BufferUtil.toString(buffer));
-                if (last)
-                    content.add("<<LAST>>");
-                return last;
-            }
-            
-        },"BOUNDARY");
         ByteBuffer data = BufferUtil.toBuffer("");
         
         data = BufferUtil.toBuffer("--BOUNDARY\r\n"
@@ -436,8 +314,8 @@ public class MultiPartParserTest
         parser.parse(data,false);
         assertThat(parser.getState(), is(State.DELIMITER));
         assertThat(data.remaining(),is(0));
-        assertThat(fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
+        assertThat(handler.fields,Matchers.contains("name: value", "<<COMPLETE>>"));        
-        assertThat(content,Matchers.contains("Now is the time for all good ment to come to the aid of the party.\r\n"
+        assertThat(handler.content,Matchers.contains("Now is the time for all good ment to come to the aid of the party.\r\n"
                 + "How now brown cow.\r\n"
                 + "The quick brown fox jumped over the lazy dog.\r\n","<<LAST>>"));
     }
@@ -571,4 +449,34 @@ public class MultiPartParserTest
         assertThat(data.remaining(),is(0));
     }
     
+
+    static class TestHandler implements MultiPartParser.Handler
+    {
+        List<String> fields = new ArrayList<>();
+        List<String> content = new ArrayList<>();
+
+        @Override
+        public void parsedHeader(String name, String value)
+        {
+            fields.add(name+": "+value);
+        }
+
+        @Override
+        public boolean headerComplete()
+        {
+            fields.add("<<COMPLETE>>");
+            return false;
+        }
+
+        @Override
+        public boolean content(ByteBuffer buffer, boolean last)
+        {
+            if (BufferUtil.hasContent(buffer))
+                content.add(BufferUtil.toString(buffer));
+            if (last)
+                content.add("<<LAST>>");
+            return last;
+        }
+    }
+
 }

jetty-util/src/main/java/org/eclipse/jetty/util/SearchPattern.java

@@ -148,9 +148,9 @@ public class SearchPattern
         
         int matchedCount = 0;
         
-        for(int i=0; i<pattern.length-matched && i < offset+length; i++)
+        for(int i=0; i<pattern.length-matched && i < length; i++)
         {
-            if(data[i] == pattern[i+matched])
+            if(data[offset+i] == pattern[i+matched])
                 matchedCount++;
             else
                 return 0;

jetty-util/src/test/java/org/eclipse/jetty/util/SearchPatternTest.java

@@ -25,7 +25,6 @@ import org.junit.Assert;
 import org.junit.Test;
 
 public class SearchPatternTest
-
 {
 
     
@@ -134,43 +133,54 @@ public class SearchPatternTest
         Assert.assertEquals(0,sp.endsWith(d,0,d.length));
     }
     
+    @Test
+    public void testStartsWithNoOffset()
+    {
+        testStartsWith("");
+    }
     
     @Test
-    public void testStartsWith()
+    public void testStartsWithOffset()
+    {
+        testStartsWith("abcdef");
+    }
+    
+    private void testStartsWith(String offset)
     {
         byte[] p = new String("abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
-        byte[] d = new String("ijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
+        byte[] d = new String(offset+"ijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
         SearchPattern sp = SearchPattern.compile(p);
-        Assert.assertEquals(18,sp.match(d,0,d.length));
+        Assert.assertEquals(18+offset.length(),sp.match(d, offset.length(), d.length-offset.length()));
-        Assert.assertEquals(-1,sp.match(d,19,d.length-19));
+        Assert.assertEquals(-1,sp.match(d, offset.length()+19, d.length-19-offset.length()));
-        Assert.assertEquals(26,sp.startsWith(d,0,d.length,8));
+        Assert.assertEquals(26,sp.startsWith(d, offset.length(), d.length-offset.length(),8));
         
         p = new String("abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
-        d = new String("ijklmnopqrstuvwxyNOMATCH").getBytes(StandardCharsets.US_ASCII);
+        d = new String(offset+"ijklmnopqrstuvwxyNOMATCH").getBytes(StandardCharsets.US_ASCII);
         sp = SearchPattern.compile(p);
-        Assert.assertEquals(0,sp.startsWith(d,0,d.length,8));
+        Assert.assertEquals(0,sp.startsWith(d, offset.length(), d.length-offset.length(),8));
         
         p = new String("abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
-        d = new String("abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
+        d = new String(offset+"abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
         sp = SearchPattern.compile(p);
-        Assert.assertEquals(26,sp.startsWith(d,0,d.length,0));
+        Assert.assertEquals(26,sp.startsWith(d, offset.length(), d.length-offset.length(),0));
         
         //test no match
         p = new String("hello world").getBytes(StandardCharsets.US_ASCII);
-        d = new String("there is definitely no match in here").getBytes(StandardCharsets.US_ASCII);
+        d = new String(offset+"there is definitely no match in here").getBytes(StandardCharsets.US_ASCII);
         sp = SearchPattern.compile(p);
-        Assert.assertEquals(0,sp.startsWith(d,0,d.length,0));  
+        Assert.assertEquals(0,sp.startsWith(d, offset.length(), d.length-offset.length(),0));  
         
         //test large pattern small buffer
         p = new String("abcdefghijklmnopqrstuvwxyz").getBytes(StandardCharsets.US_ASCII);
-        d = new String("mnopqrs").getBytes(StandardCharsets.US_ASCII);
+        d = new String(offset+"mnopqrs").getBytes(StandardCharsets.US_ASCII);
         sp = SearchPattern.compile(p);
-        Assert.assertEquals(19,sp.startsWith(d,0,d.length,12));  
+        Assert.assertEquals(19,sp.startsWith(d, offset.length(), d.length-offset.length(),12));  
         
         //partial pattern
         p = new String("abcdef").getBytes(StandardCharsets.US_ASCII);
-        d = new String("cde").getBytes(StandardCharsets.US_ASCII);
+        d = new String(offset+"cde").getBytes(StandardCharsets.US_ASCII);
         sp = SearchPattern.compile(p);
-        Assert.assertEquals(5,sp.startsWith(d,0,d.length,2));
+        Assert.assertEquals(5,sp.startsWith(d,offset.length(),d.length-offset.length(),2));
+        
     }
 }