Update MimeParser to pospone body if an ambiguous boundary is found. (#…

…3968) (#3971)

I.e. a boundary located at the end of the current buffer that could be a "closing" boundary.

Fixes #3967

media/multipart/src/main/java/io/helidon/media/multipart/MimeParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -83,12 +83,14 @@ abstract static class ParserEvent {
 
         /**
          * Get the event type.
+         *
          * @return EVENT_TYPE
          */
         abstract EventType type();
 
         /**
          * Get this event as a {@link HeaderEvent}.
+         *
          * @return HeaderEvent
          */
         HeaderEvent asHeaderEvent() {
@@ -97,6 +99,7 @@ HeaderEvent asHeaderEvent() {
 
         /**
          * Get this event as a {@link BodyEvent}.
+         *
          * @return HeaderEvent
          */
         BodyEvent asBodyEvent() {
@@ -229,6 +232,7 @@ static final class ParsingException extends RuntimeException {
 
         /**
          * Create a new exception with the specified message.
+         *
          * @param message exception message
          */
         private ParsingException(String message) {
@@ -237,6 +241,7 @@ private ParsingException(String message) {
 
         /**
          * Create a new exception with the specified cause.
+         *
          * @param cause exception cause
          */
         private ParsingException(Throwable cause) {
@@ -344,8 +349,8 @@ private enum STATE {
      * Push new data to the parsing buffer.
      *
      * @param data new data add to the parsing buffer
-     * @throws ParsingException if the parser state is not consistent
      * @return buffer id
+     * @throws ParsingException if the parser state is not consistent
      */
     int offer(ByteBuffer data) throws ParsingException {
         if (closed) {
@@ -358,6 +363,9 @@ int offer(ByteBuffer data) throws ParsingException {
                 break;
             case DATA_REQUIRED:
                 // resume the previous state
+                if (LOGGER.isLoggable(Level.FINER)) {
+                    LOGGER.log(Level.FINER, "Resume state. resumeState={1}", resumeState);
+                }
                 state = resumeState;
                 resumeState = null;
                 id = buf.offer(data, position);
@@ -409,6 +417,7 @@ void cleanup() {
 
     /**
      * Advances parsing.
+     *
      * @throws ParsingException if an error occurs during parsing
      */
     Iterator<ParserEvent> parseIterator() {
@@ -574,9 +583,13 @@ private List<VirtualBuffer.BufferEntry> readBody() {
                 position = bufLen - (bl + 1);
                 return buf.slice(bodyBegin, position);
             }
-            // remaining data can be an complete boundary, force it to be
+            // remaining data can be a complete boundary, force it to be
             // processed during next iteration
             return Collections.emptyList();
+        } else if (bndStart + bl + 1 >= bufLen) {
+            // found a boundary at the very end of the buffer
+            // it may be a "closing" boundary, require more data
+            return Collections.emptyList();
         }
 
         // Found boundary.
@@ -724,7 +737,7 @@ private String readHeaderLine() {
             }
         }
         position = offset + hdrLen + lwsp;
-        if (hdrLen == 0){
+        if (hdrLen == 0) {
             return "";
         }
         return new String(buf.getBytes(offset, hdrLen), HEADER_ENCODING);
@@ -733,7 +746,7 @@ private String readHeaderLine() {
     /**
      * Boyer-Moore search method.
      * Copied from {@link java.util.regex.Pattern}
-     *
+     * <p>
      * Pre calculates arrays needed to generate the bad character shift and the
      * good suffix shift. Only the last seven bits are used to see if chars
      * match; This keeps the tables small and covers the heavily used ASCII
@@ -809,6 +822,7 @@ private int match() {
 
     /**
      * Get the bytes representation of a string.
+     *
      * @param str string to convert
      * @return byte[]
      */

media/multipart/src/test/java/io/helidon/media/multipart/MimeParserTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,11 @@
 package io.helidon.media.multipart;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -32,8 +32,8 @@
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.hasItems;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -59,7 +59,7 @@ public static void after() {
 
     // TODO test mixed with nested boundaries
     @Test
-    public void testSkipPreambule() {
+    public void testSkipPreamble() {
         String boundary = "boundary";
         final byte[] chunk1 = ("--" + boundary
                 + "          \t     \t  \t "
@@ -79,7 +79,7 @@ public void testSkipPreambule() {
     }
 
     @Test
-    public void testNoPreambule() {
+    public void testNoPreamble() {
         String boundary = "boundary";
         final byte[] chunk1 = ("--" + boundary
                 + "Content-Id: part1\n"
@@ -431,6 +431,46 @@ public void testClosingBoundaryAcrossChunks() {
         assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
     }
 
+    @Test
+    public void testIncompleteClosingBoundary() {
+        String boundary = "boundary";
+        final byte[] chunk1 = ("--" + boundary + "\n"
+                + "Content-Id: part1\n"
+                + "\n"
+                + "this-is-the-body-of-part1\n"
+                + "--" + boundary + "-").getBytes();
+        final byte[] chunk2 = "-".getBytes();
+
+        List<MimePart> parts = parse(boundary, List.of(chunk1, chunk2)).parts;
+        assertThat(parts.size(), is(equalTo(1)));
+
+        MimePart part1 = parts.get(0);
+        assertThat(part1.headers.size(), is(equalTo(1)));
+        assertThat(part1.headers.get("Content-Id"), hasItems("part1"));
+        assertThat(part1.content, is(notNullValue()));
+        assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
+    }
+
+    @Test
+    public void testAmbiguousClosingBoundary() {
+        String boundary = "boundary";
+        final byte[] chunk1 = ("--" + boundary + "\n"
+                + "Content-Id: part1\n"
+                + "\n"
+                + "this-is-the-body-of-part1\n"
+                + "--" + boundary).getBytes();
+        final byte[] chunk2 = "--".getBytes();
+
+        List<MimePart> parts = parse(boundary, List.of(chunk1, chunk2)).parts;
+        assertThat(parts.size(), is(equalTo(1)));
+
+        MimePart part1 = parts.get(0);
+        assertThat(part1.headers.size(), is(equalTo(1)));
+        assertThat(part1.headers.get("Content-Id"), hasItems("part1"));
+        assertThat(part1.content, is(notNullValue()));
+        assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1")));
+    }
+
     @Test
     public void testPreamble() {
         String boundary = "boundary";
@@ -688,12 +728,8 @@ static ParserResult parse(String boundary, List<byte[]> data) {
                         assertThat(name, notNullValue());
                         assertThat(name.length(), not(equalTo(0)));
                         assertThat(value, notNullValue());
-                        List<String> values = partHeaders.get(name);
-                        if (values == null) {
-                            values = new ArrayList<>();
-                            partHeaders.put(name, values);
-                        }
-                        values.add(value);
+                        partHeaders.computeIfAbsent(name, k -> new ArrayList<>())
+                                   .add(value);
                         break;
                     case BODY:
                         for (VirtualBuffer.BufferEntry content : event.asBodyEvent().body()) {