Minimize get byte multipart and fix buffer reuse (#11001)

Motivation:
- Underlying buffer usages might be erroneous when releasing them internaly
in HttpPostMultipartRequestDecoder.

2 bugs occurs:
1) Final File upload seems not to be of the right size.
2) Memory, even in Disk mode, is increasing continuously, while it shouldn't.

- Method `getByte(position)` is too often called within the current implementation
of the HttpPostMultipartRequestDecoder.
This implies too much activities which is visible when PARANOID mode is active.
This is also true in standard mode.

Apply the same fix on buffer from HttpPostMultipartRequestDecoder to HttpPostStandardRequestDecoder
made previously.

Finally in order to ensure we do not rewrite already decoded HttpData when decoding
next ones within multipart, we must ensure the buffers are copied and not a retained slice.

Modification:
- Add some tests to check consistency for HttpPostMultipartRequestDecoder.
Add a package protected method for testing purpose only.

- Use the `bytesBefore(...)` method instead of `getByte(pos)` in order to limit the external
access to the underlying buffer by retrieving iteratively the beginning of a correct start
position.
It is used to find both LF/CRLF and delimiter.
2 methods in HttpPostBodyUtil were created for that.

The undecodedChunk is copied when adding a chunk to a DataMultipart is loaded.
The same buffer is also rewritten in order to release the copied memory part.

Result:

Just for note, for both Memory or Disk or Mixed mode factories, the release has to be done as:

      for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
          httpData.release();
          factory.removeHttpDataFromClean(request, httpData);
      }
      factory.cleanAllHttpData();
      decoder.destroy();

The memory used is minimal in Disk or Mixed mode. In Memory mode, a big file is still
in memory but not more in the undecodedChunk but its own buffer (copied).

In terms of benchmarking, the results are:

Original code Benchmark                                                             Mode  Cnt  Score    Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  0,152 ±  0,100  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  0,543 ±  0,218  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,001 ±  0,001  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  0,615 ±  0,070  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  0,114 ±  0,063  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  0,664 ±  0,034  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,001 ±  0,001  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  0,620 ±  0,140  ops/ms

New code Benchmark                                                                  Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  4,037 ± 0,358  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  4,226 ± 0,471  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,875 ± 0,029  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  4,346 ± 0,275  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,044 ± 0,020  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  2,278 ± 0,159  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,174 ± 0,004  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,370 ± 0,065  ops/ms

In short, using big file transfers, this is about 7 times faster with new code, while
using high number of HttpData, this is about 4 times faster with new code when using Simple Level.
When using Paranoid Level, using big file transfers, this is about 800 times faster with new code, while
using high number of HttpData, this is about 170 times faster with new code.

codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java

@@ -105,6 +105,9 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData {
             size += localsize;
             if (byteBuf == null) {
                 byteBuf = buffer;
+            } else if (localsize == 0) {
+                // Nothing to add and byteBuf already exists
+                buffer.release();
             } else if (byteBuf instanceof CompositeByteBuf) {
                 CompositeByteBuf cbb = (CompositeByteBuf) byteBuf;
                 cbb.addComponent(true, buffer);

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostBodyUtil.java

@@ -16,6 +16,7 @@
 package io.netty.handler.codec.http.multipart;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.handler.codec.http.HttpConstants;
 
 /**
  * Shared Static object between HttpMessageDecoder, HttpPostRequestDecoder and HttpPostRequestEncoder
@@ -156,4 +157,85 @@ final class HttpPostBodyUtil {
         return result;
     }
 
+    /**
+     * Try to find LF or CRLF as Line Breaking
+     *
+     * @param buffer the buffer to search in
+     * @param index the index to start from in the buffer
+     * @return a relative position from index > 0 if LF or CRLF is found
+     *         or < 0 if not found
+     */
+    static int findLineBreak(ByteBuf buffer, int index) {
+        int toRead = buffer.readableBytes() - (index - buffer.readerIndex());
+        int posFirstChar = buffer.bytesBefore(index, toRead, HttpConstants.LF);
+        if (posFirstChar == -1) {
+            // No LF, so neither CRLF
+            return  -1;
+        }
+        if (posFirstChar > 0 && buffer.getByte(index + posFirstChar - 1) == HttpConstants.CR) {
+            posFirstChar--;
+        }
+        return posFirstChar;
+    }
+
+    /**
+     * Try to find the delimiter, with LF or CRLF in front of it (added as delimiters) if needed
+     *
+     * @param buffer the buffer to search in
+     * @param index the index to start from in the buffer
+     * @param delimiter the delimiter as byte array
+     * @param precededByLineBreak true if it must be preceded by LF or CRLF, else false
+     * @return a relative position from index > 0 if delimiter found designing the start of it
+     *         (including LF or CRLF is asked)
+     *         or a number < 0 if delimiter is not found
+     * @throws IndexOutOfBoundsException
+     *         if {@code offset + delimiter.length} is greater than {@code buffer.capacity}
+     */
+    static int findDelimiter(ByteBuf buffer, int index, byte[] delimiter, boolean precededByLineBreak) {
+        final int delimiterLength = delimiter.length;
+        final int readerIndex = buffer.readerIndex();
+        final int writerIndex = buffer.writerIndex();
+        int toRead = writerIndex - index;
+        int newOffset = index;
+        boolean delimiterNotFound = true;
+        while (delimiterNotFound && delimiterLength <= toRead) {
+            // Find first position: delimiter
+            int posDelimiter = buffer.bytesBefore(newOffset, toRead, delimiter[0]);
+            if (posDelimiter < 0) {
+                return -1;
+            }
+            newOffset += posDelimiter;
+            toRead -= posDelimiter;
+            // Now check for delimiter
+            delimiterNotFound = false;
+            for (int i = 0; i < delimiterLength; i++) {
+                if (buffer.getByte(newOffset + i) != delimiter[i]) {
+                    newOffset++;
+                    toRead--;
+                    delimiterNotFound = true;
+                    break;
+                }
+            }
+            if (!delimiterNotFound) {
+                // Delimiter found, find if necessary: LF or CRLF
+                if (precededByLineBreak && newOffset > readerIndex) {
+                    if (buffer.getByte(newOffset - 1) == HttpConstants.LF) {
+                        newOffset--;
+                        // Check if CR before: not mandatory to be there
+                        if (newOffset > readerIndex && buffer.getByte(newOffset - 1) == HttpConstants.CR) {
+                            newOffset--;
+                        }
+                    } else {
+                        // Delimiter with Line Break could be further: iterate after first char of delimiter
+                        newOffset++;
+                        toRead--;
+                        delimiterNotFound = true;
+                        continue;
+                    }
+                }
+                return newOffset - readerIndex;
+            }
+        }
+        return -1;
+    }
 }

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java

@@ -550,7 +550,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
                 }
             }
             // load data
-            if (!loadDataMultipart(undecodedChunk, multipartDataBoundary, currentAttribute)) {
+            if (!loadDataMultipartOptimized(undecodedChunk, multipartDataBoundary, currentAttribute)) {
                 // Delimiter is not found. Need more chunks.
                 return null;
             }
@@ -647,7 +647,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
         skipOneLine();
         String newline;
         try {
-            newline = readDelimiter(undecodedChunk, delimiter);
+            newline = readDelimiterOptimized(undecodedChunk, delimiter, charset);
         } catch (NotEnoughDataDecoderException ignored) {
             undecodedChunk.readerIndex(readerIndex);
             return null;
@@ -687,7 +687,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
             String newline;
             try {
                 skipControlCharacters(undecodedChunk);
-                newline = readLine(undecodedChunk, charset);
+                newline = readLineOptimized(undecodedChunk, charset);
             } catch (NotEnoughDataDecoderException ignored) {
                 undecodedChunk.readerIndex(readerIndex);
                 return null;
@@ -903,7 +903,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
             }
         }
         // load data as much as possible
-        if (!loadDataMultipart(undecodedChunk, delimiter, currentFileUpload)) {
+        if (!loadDataMultipartOptimized(undecodedChunk, delimiter, currentFileUpload)) {
             // Delimiter is not found. Need more chunks.
             return null;
         }
@@ -983,83 +983,32 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
      *             Need more chunks and reset the {@code readerIndex} to the previous
      *             value
      */
-    private static String readLineStandard(ByteBuf undecodedChunk, Charset charset) {
+    private static String readLineOptimized(ByteBuf undecodedChunk, Charset charset) {
         int readerIndex = undecodedChunk.readerIndex();
-        ByteBuf line = undecodedChunk.alloc().heapBuffer(64);
+        ByteBuf line = null;
         try {
-            while (undecodedChunk.isReadable()) {
+            if (undecodedChunk.isReadable()) {
-                byte nextByte = undecodedChunk.readByte();
+                int posLfOrCrLf = HttpPostBodyUtil.findLineBreak(undecodedChunk, undecodedChunk.readerIndex());
-                if (nextByte == HttpConstants.CR) {
+                if (posLfOrCrLf <= 0) {
-                    // check but do not changed readerIndex
+                    throw new NotEnoughDataDecoderException();
-                    nextByte = undecodedChunk.getByte(undecodedChunk.readerIndex());
-                    if (nextByte == HttpConstants.LF) {
-                        // force read
-                        undecodedChunk.readByte();
-                        return line.toString(charset);
-                    } else {
-                        // Write CR (not followed by LF)
-                        line.writeByte(HttpConstants.CR);
-                    }
-                } else if (nextByte == HttpConstants.LF) {
-                    return line.toString(charset);
-                } else {
-                    line.writeByte(nextByte);
                 }
-            }
+                try {
-        } catch (IndexOutOfBoundsException e) {
+                    line = undecodedChunk.alloc().heapBuffer(posLfOrCrLf);
-            undecodedChunk.readerIndex(readerIndex);
+                    line.writeBytes(undecodedChunk, posLfOrCrLf);
-            throw new NotEnoughDataDecoderException(e);
-        } finally {
-            line.release();
-        }
-        undecodedChunk.readerIndex(readerIndex);
-        throw new NotEnoughDataDecoderException();
-    }
 
-    /**
+                    byte nextByte = undecodedChunk.readByte();
-     * Read one line up to the CRLF or LF
+                    if (nextByte == HttpConstants.CR) {
-     *
+                        // force read next byte since LF is the following one
-     * @return the String from one line
+                        undecodedChunk.readByte();
-     * @throws NotEnoughDataDecoderException
-     *             Need more chunks and reset the {@code readerIndex} to the previous
-     *             value
-     */
-    private static String readLine(ByteBuf undecodedChunk, Charset charset) {
-        if (!undecodedChunk.hasArray()) {
-            return readLineStandard(undecodedChunk, charset);
-        }
-        SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk);
-        int readerIndex = undecodedChunk.readerIndex();
-        ByteBuf line = undecodedChunk.alloc().heapBuffer(64);
-        try {
-            while (sao.pos < sao.limit) {
-                byte nextByte = sao.bytes[sao.pos++];
-                if (nextByte == HttpConstants.CR) {
-                    if (sao.pos < sao.limit) {
-                        nextByte = sao.bytes[sao.pos++];
-                        if (nextByte == HttpConstants.LF) {
-                            sao.setReadPosition(0);
-                            return line.toString(charset);
-                        } else {
-                            // Write CR (not followed by LF)
-                            sao.pos--;
-                            line.writeByte(HttpConstants.CR);
-                        }
-                    } else {
-                        line.writeByte(nextByte);
                     }
-                } else if (nextByte == HttpConstants.LF) {
-                    sao.setReadPosition(0);
                     return line.toString(charset);
-                } else {
+                } finally {
-                    line.writeByte(nextByte);
+                    line.release();
                 }
             }
         } catch (IndexOutOfBoundsException e) {
             undecodedChunk.readerIndex(readerIndex);
             throw new NotEnoughDataDecoderException(e);
-        } finally {
-            line.release();
         }
         undecodedChunk.readerIndex(readerIndex);
         throw new NotEnoughDataDecoderException();
@@ -1080,23 +1029,19 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
      *             Need more chunks and reset the {@code readerIndex} to the previous
      *             value
      */
-    private static String readDelimiterStandard(ByteBuf undecodedChunk, String delimiter) {
+    private static String readDelimiterOptimized(ByteBuf undecodedChunk, String delimiter, Charset charset) {
-        int readerIndex = undecodedChunk.readerIndex();
+        final int readerIndex = undecodedChunk.readerIndex();
+        final byte[] bdelimiter = delimiter.getBytes(charset);
+        final int delimiterLength = bdelimiter.length;
         try {
-            StringBuilder sb = new StringBuilder(64);
+            int delimiterPos = HttpPostBodyUtil.findDelimiter(undecodedChunk, readerIndex, bdelimiter, false);
-            int delimiterPos = 0;
+            if (delimiterPos < 0) {
-            int len = delimiter.length();
+                // delimiter not found so break here !
-            while (undecodedChunk.isReadable() && delimiterPos < len) {
+                undecodedChunk.readerIndex(readerIndex);
-                byte nextByte = undecodedChunk.readByte();
+                throw new NotEnoughDataDecoderException();
-                if (nextByte == delimiter.charAt(delimiterPos)) {
-                    delimiterPos++;
-                    sb.append((char) nextByte);
-                } else {
-                    // delimiter not found so break here !
-                    undecodedChunk.readerIndex(readerIndex);
-                    throw new NotEnoughDataDecoderException();
-                }
             }
+            StringBuilder sb = new StringBuilder(delimiter);
+            undecodedChunk.readerIndex(readerIndex + delimiterPos + delimiterLength);
             // Now check if either opening delimiter or closing delimiter
             if (undecodedChunk.isReadable()) {
                 byte nextByte = undecodedChunk.readByte();
@@ -1161,127 +1106,28 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
     }
 
     /**
-     * Read one line up to --delimiter or --delimiter-- and if existing the CRLF
+     * Rewrite buffer in order to skip lengthToSkip bytes from current readerIndex,
-     * or LF. Note that CRLF or LF are mandatory for opening delimiter
+     * such that any readable bytes available after readerIndex + lengthToSkip (so before writerIndex)
-     * (--delimiter) but not for closing delimiter (--delimiter--) since some
+     * are moved at readerIndex position,
-     * clients does not include CRLF in this case.
+     * therefore decreasing writerIndex of lengthToSkip at the end of the process.
      *
-     * @param delimiter
+     * @param buffer the buffer to rewrite from current readerIndex
-     *            of the form --string, such that '--' is already included
+     * @param lengthToSkip the size to skip from readerIndex
-     * @return the String from one line as the delimiter searched (opening or
-     *         closing)
-     * @throws NotEnoughDataDecoderException
-     *             Need more chunks and reset the readerInder to the previous
-     *             value
      */
-    private static String readDelimiter(ByteBuf undecodedChunk, String delimiter) {
+    private static void rewriteCurrentBuffer(ByteBuf buffer, int lengthToSkip) {
-        if (!undecodedChunk.hasArray()) {
+        if (lengthToSkip == 0) {
-            return readDelimiterStandard(undecodedChunk, delimiter);
+            return;
         }
-        SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk);
+        final int readerIndex = buffer.readerIndex();
-        int readerIndex = undecodedChunk.readerIndex();
+        final int readableBytes = buffer.readableBytes();
-        int delimiterPos = 0;
+        if (readableBytes == lengthToSkip) {
-        int len = delimiter.length();
+            buffer.readerIndex(readerIndex);
-        try {
+            buffer.writerIndex(readerIndex);
-            StringBuilder sb = new StringBuilder(64);
+            return;
-            // check conformity with delimiter
-            while (sao.pos < sao.limit && delimiterPos < len) {
-                byte nextByte = sao.bytes[sao.pos++];
-                if (nextByte == delimiter.charAt(delimiterPos)) {
-                    delimiterPos++;
-                    sb.append((char) nextByte);
-                } else {
-                    // delimiter not found so break here !
-                    undecodedChunk.readerIndex(readerIndex);
-                    throw new NotEnoughDataDecoderException();
-                }
-            }
-            // Now check if either opening delimiter or closing delimiter
-            if (sao.pos < sao.limit) {
-                byte nextByte = sao.bytes[sao.pos++];
-                if (nextByte == HttpConstants.CR) {
-                    // first check for opening delimiter
-                    if (sao.pos < sao.limit) {
-                        nextByte = sao.bytes[sao.pos++];
-                        if (nextByte == HttpConstants.LF) {
-                            sao.setReadPosition(0);
-                            return sb.toString();
-                        } else {
-                            // error CR without LF
-                            // delimiter not found so break here !
-                            undecodedChunk.readerIndex(readerIndex);
-                            throw new NotEnoughDataDecoderException();
-                        }
-                    } else {
-                        // error since CR must be followed by LF
-                        // delimiter not found so break here !
-                        undecodedChunk.readerIndex(readerIndex);
-                        throw new NotEnoughDataDecoderException();
-                    }
-                } else if (nextByte == HttpConstants.LF) {
-                    // same first check for opening delimiter where LF used with
-                    // no CR
-                    sao.setReadPosition(0);
-                    return sb.toString();
-                } else if (nextByte == '-') {
-                    sb.append('-');
-                    // second check for closing delimiter
-                    if (sao.pos < sao.limit) {
-                        nextByte = sao.bytes[sao.pos++];
-                        if (nextByte == '-') {
-                            sb.append('-');
-                            // now try to find if CRLF or LF there
-                            if (sao.pos < sao.limit) {
-                                nextByte = sao.bytes[sao.pos++];
-                                if (nextByte == HttpConstants.CR) {
-                                    if (sao.pos < sao.limit) {
-                                        nextByte = sao.bytes[sao.pos++];
-                                        if (nextByte == HttpConstants.LF) {
-                                            sao.setReadPosition(0);
-                                            return sb.toString();
-                                        } else {
-                                            // error CR without LF
-                                            // delimiter not found so break here !
-                                            undecodedChunk.readerIndex(readerIndex);
-                                            throw new NotEnoughDataDecoderException();
-                                        }
-                                    } else {
-                                        // error CR without LF
-                                        // delimiter not found so break here !
-                                        undecodedChunk.readerIndex(readerIndex);
-                                        throw new NotEnoughDataDecoderException();
-                                    }
-                                } else if (nextByte == HttpConstants.LF) {
-                                    sao.setReadPosition(0);
-                                    return sb.toString();
-                                } else {
-                                    // No CRLF but ok however (Adobe Flash
-                                    // uploader)
-                                    // minus 1 since we read one char ahead but
-                                    // should not
-                                    sao.setReadPosition(1);
-                                    return sb.toString();
-                                }
-                            }
-                            // FIXME what do we do here?
-                            // either considering it is fine, either waiting for
-                            // more data to come?
-                            // lets try considering it is fine...
-                            sao.setReadPosition(0);
-                            return sb.toString();
-                        }
-                        // whatever now => error since incomplete
-                        // only one '-' => not enough or whatever not enough
-                        // element
-                    }
-                }
-            }
-        } catch (IndexOutOfBoundsException e) {
-            undecodedChunk.readerIndex(readerIndex);
-            throw new NotEnoughDataDecoderException(e);
         }
-        undecodedChunk.readerIndex(readerIndex);
+        buffer.setBytes(readerIndex, buffer, readerIndex + lengthToSkip, readableBytes - lengthToSkip);
-        throw new NotEnoughDataDecoderException();
+        buffer.readerIndex(readerIndex);
+        buffer.writerIndex(readerIndex + readableBytes - lengthToSkip);
     }
 
     /**
@@ -1290,91 +1136,50 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
      * @return {@code true} if the last chunk is loaded (boundary delimiter found), {@code false} if need more chunks
      * @throws ErrorDataDecoderException
      */
-    private static boolean loadDataMultipartStandard(ByteBuf undecodedChunk, String delimiter, HttpData httpData) {
+    private static boolean loadDataMultipartOptimized(ByteBuf undecodedChunk, String delimiter, HttpData httpData) {
+        if (!undecodedChunk.isReadable()) {
+            return false;
+        }
         final int startReaderIndex = undecodedChunk.readerIndex();
-        final int delimeterLength = delimiter.length();
+        final byte[] bdelimiter = delimiter.getBytes(httpData.getCharset());
-        int index = 0;
+        int posDelimiter = HttpPostBodyUtil.findDelimiter(undecodedChunk, startReaderIndex, bdelimiter, true);
-        int lastPosition = startReaderIndex;
+        if (posDelimiter < 0) {
-        byte prevByte = HttpConstants.LF;
+            // Not found but however perhaps because incomplete so search LF or CRLF
-        boolean delimiterFound = false;
+            posDelimiter = HttpPostBodyUtil.findLineBreak(undecodedChunk, startReaderIndex);
-        while (undecodedChunk.isReadable()) {
+            if (posDelimiter < 0) {
-            final byte nextByte = undecodedChunk.readByte();
+                // not found so this chunk can be fully added
-            // Check the delimiter
+                ByteBuf content = undecodedChunk.copy();
-            if (prevByte == HttpConstants.LF && nextByte == delimiter.codePointAt(index)) {
+                try {
-                index++;
+                    httpData.addContent(content, false);
-                if (delimeterLength == index) {
+                } catch (IOException e) {
-                    delimiterFound = true;
+                    throw new ErrorDataDecoderException(e);
-                    break;
                 }
-                continue;
+                undecodedChunk.readerIndex(startReaderIndex);
+                undecodedChunk.writerIndex(startReaderIndex);
+                return false;
+            } else if (posDelimiter > 0) {
+                // Not fully but still some bytes to provide: httpData is not yet finished since delimiter not found
+                ByteBuf content = undecodedChunk.copy(startReaderIndex, posDelimiter);
+                try {
+                    httpData.addContent(content, false);
+                } catch (IOException e) {
+                    throw new ErrorDataDecoderException(e);
+                }
+                rewriteCurrentBuffer(undecodedChunk, posDelimiter);
+                return false;
             }
-            lastPosition = undecodedChunk.readerIndex();
+            // Empty chunk or so
-            if (nextByte == HttpConstants.LF) {
+            return false;
-                index = 0;
-                lastPosition -= (prevByte == HttpConstants.CR)? 2 : 1;
-            }
-            prevByte = nextByte;
         }
-        if (prevByte == HttpConstants.CR) {
+        // Delimiter found at posDelimiter, including LF or CRLF, so httpData has its last chunk
-            lastPosition--;
+        ByteBuf content = undecodedChunk.copy(startReaderIndex, posDelimiter);
-        }
-        ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, lastPosition - startReaderIndex);
         try {
-            httpData.addContent(content, delimiterFound);
+            httpData.addContent(content, true);
         } catch (IOException e) {
             throw new ErrorDataDecoderException(e);
         }
-        undecodedChunk.readerIndex(lastPosition);
+        rewriteCurrentBuffer(undecodedChunk, posDelimiter);
-        return delimiterFound;
+        return true;
-    }
-
-    /**
-     * Load the field value from a Multipart request
-     *
-     * @return {@code true} if the last chunk is loaded (boundary delimiter found), {@code false} if need more chunks
-     * @throws ErrorDataDecoderException
-     */
-    private static boolean loadDataMultipart(ByteBuf undecodedChunk, String delimiter, HttpData httpData) {
-        if (!undecodedChunk.hasArray()) {
-            return loadDataMultipartStandard(undecodedChunk, delimiter, httpData);
-        }
-        final SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk);
-        final int startReaderIndex = undecodedChunk.readerIndex();
-        final int delimeterLength = delimiter.length();
-        int index = 0;
-        int lastRealPos = sao.pos;
-        byte prevByte = HttpConstants.LF;
-        boolean delimiterFound = false;
-        while (sao.pos < sao.limit) {
-            final byte nextByte = sao.bytes[sao.pos++];
-            // Check the delimiter
-            if (prevByte == HttpConstants.LF && nextByte == delimiter.codePointAt(index)) {
-                index++;
-                if (delimeterLength == index) {
-                    delimiterFound = true;
-                    break;
-                }
-                continue;
-            }
-            lastRealPos = sao.pos;
-            if (nextByte == HttpConstants.LF) {
-                index = 0;
-                lastRealPos -= (prevByte == HttpConstants.CR)? 2 : 1;
-            }
-            prevByte = nextByte;
-        }
-        if (prevByte == HttpConstants.CR) {
-            lastRealPos--;
-        }
-        final int lastPosition = sao.getReadPosition(lastRealPos);
-        final ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, lastPosition - startReaderIndex);
-        try {
-            httpData.addContent(content, delimiterFound);
-        } catch (IOException e) {
-            throw new ErrorDataDecoderException(e);
-        }
-        undecodedChunk.readerIndex(lastPosition);
-        return delimiterFound;
     }
 
     /**
@@ -1514,4 +1319,15 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
         values.add(svalue.substring(start));
         return values.toArray(new String[0]);
     }
+
+    /**
+     * This method is package private intentionally in order to allow during tests
+     * to access to the amount of memory allocated (capacity) within the private
+     * ByteBuf undecodedChunk
+     *
+     * @return the number of bytes the internal buffer can contain
+     */
+    int getCurrentAllocatedCapacity() {
+        return undecodedChunk.capacity();
+    }
 }

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestEncoder.java

@@ -942,12 +942,12 @@ public class HttpPostRequestEncoder implements ChunkedInput<HttpContent> {
         // Set name=
         if (isKey) {
             String key = currentData.getName();
-            buffer = wrappedBuffer(key.getBytes());
+            buffer = wrappedBuffer(key.getBytes(charset));
             isKey = false;
             if (currentBuffer == null) {
-                currentBuffer = wrappedBuffer(buffer, wrappedBuffer("=".getBytes()));
+                currentBuffer = wrappedBuffer(buffer, wrappedBuffer("=".getBytes(charset)));
             } else {
-                currentBuffer = wrappedBuffer(currentBuffer, buffer, wrappedBuffer("=".getBytes()));
+                currentBuffer = wrappedBuffer(currentBuffer, buffer, wrappedBuffer("=".getBytes(charset)));
             }
             // continue
             size -= buffer.readableBytes() + 1;
@@ -968,7 +968,7 @@ public class HttpPostRequestEncoder implements ChunkedInput<HttpContent> {
         ByteBuf delimiter = null;
         if (buffer.readableBytes() < size) {
             isKey = true;
-            delimiter = iterator.hasNext() ? wrappedBuffer("&".getBytes()) : null;
+            delimiter = iterator.hasNext() ? wrappedBuffer("&".getBytes(charset)) : null;
         }
 
         // End for current InterfaceHttpData, need potentially more data

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java

@@ -304,7 +304,17 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
         }
         parseBody();
         if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
-            undecodedChunk.discardReadBytes();
+            if (undecodedChunk.refCnt() == 1) {
+                // It's safe to call discardBytes() as we are the only owner of the buffer.
+                undecodedChunk.discardReadBytes();
+            } else {
+                // There seems to be multiple references of the buffer. Let's copy the data and release the buffer to
+                // ensure we can give back memory to the system.
+                ByteBuf buffer = undecodedChunk.alloc().buffer(undecodedChunk.readableBytes());
+                buffer.writeBytes(undecodedChunk);
+                undecodedChunk.release();
+                undecodedChunk = buffer;
+            }
         }
         return this;
     }

codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java

@@ -270,9 +270,6 @@ public class MixedAttribute implements Attribute {
 
     @Override
     public void setValue(String value) throws IOException {
-        if (value != null) {
-            checkSize(value.getBytes().length);
-        }
         attribute.setValue(value);
     }
 

codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java

@@ -34,6 +34,7 @@ import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.util.CharsetUtil;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.net.URLEncoder;
 import java.nio.charset.UnsupportedCharsetException;
 import java.util.Arrays;
@@ -961,6 +962,7 @@ public class HttpPostRequestDecoderTest {
 
         try {
             HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(new DefaultHttpDataFactory(false), req);
+            assertEquals(2, decoder.getBodyHttpDatas().size());
             InterfaceHttpData data = decoder.getBodyHttpData("title");
             assertTrue(data instanceof MemoryAttribute);
             assertEquals("bar-stream", ((MemoryAttribute) data).getString());
@@ -976,4 +978,177 @@ public class HttpPostRequestDecoderTest {
             assertTrue(req.release());
         }
     }
+
+    private void commonTestBigFileDelimiterInMiddleChunk(HttpDataFactory factory, boolean inMemory)
+            throws IOException {
+        int nbChunks = 100;
+        int bytesPerChunk = 100000;
+        int bytesLastChunk = 10000;
+        int fileSize = bytesPerChunk * nbChunks + bytesLastChunk; // set Xmx to a number lower than this and it crashes
+
+        String prefix = "--861fbeab-cd20-470c-9609-d40a0f704466\n" +
+                "Content-Disposition: form-data; name=\"image\"; filename=\"guangzhou.jpeg\"\n" +
+                "Content-Type: image/jpeg\n" +
+                "Content-Length: " + fileSize + "\n" +
+                "\n";
+
+        String suffix1 = "\n" +
+                "--861fbeab-";
+        String suffix2 = "cd20-470c-9609-d40a0f704466--\n";
+        String suffix = suffix1 + suffix2;
+
+        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
+        request.headers().set("content-type", "multipart/form-data; boundary=861fbeab-cd20-470c-9609-d40a0f704466");
+        request.headers().set("content-length", prefix.length() + fileSize + suffix.length());
+
+        HttpPostMultipartRequestDecoder decoder = new HttpPostMultipartRequestDecoder(factory, request);
+        decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(prefix.getBytes(CharsetUtil.UTF_8))));
+
+        byte[] body = new byte[bytesPerChunk];
+        Arrays.fill(body, (byte) 1);
+        for (int i = 0; i < nbChunks; i++) {
+            ByteBuf content = Unpooled.wrappedBuffer(body, 0, bytesPerChunk);
+            decoder.offer(new DefaultHttpContent(content)); // **OutOfMemory here**
+            content.release();
+        }
+
+        byte[] bsuffix1 = suffix1.getBytes(CharsetUtil.UTF_8);
+        byte[] lastbody = new byte[bytesLastChunk + bsuffix1.length];
+        Arrays.fill(body, (byte) 1);
+        for (int i = 0; i < bsuffix1.length; i++) {
+            lastbody[bytesLastChunk + i] = bsuffix1[i];
+        }
+
+        ByteBuf content2 = Unpooled.wrappedBuffer(lastbody, 0, lastbody.length);
+        decoder.offer(new DefaultHttpContent(content2));
+        content2.release();
+        content2 = Unpooled.wrappedBuffer(suffix2.getBytes(CharsetUtil.UTF_8));
+        decoder.offer(new DefaultHttpContent(content2));
+        content2.release();
+        decoder.offer(new DefaultLastHttpContent());
+
+        FileUpload data = (FileUpload) decoder.getBodyHttpDatas().get(0);
+        assertEquals(data.length(), fileSize);
+        assertEquals(inMemory, data.isInMemory());
+        if (data.isInMemory()) {
+            // To be done only if not inMemory: assertEquals(data.get().length, fileSize);
+            assertFalse("Capacity should be higher than 1M", data.getByteBuf().capacity()
+                    < 1024 * 1024);
+        }
+        assertTrue("Capacity should be less than 1M", decoder.getCurrentAllocatedCapacity()
+                < 1024 * 1024);
+        for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
+            httpData.release();
+            factory.removeHttpDataFromClean(request, httpData);
+        }
+        factory.cleanAllHttpData();
+        decoder.destroy();
+    }
+
+    @Test
+    public void testBIgFileUploadDelimiterInMiddleChunkDecoderDiskFactory() throws IOException {
+        // Factory using Disk mode
+        HttpDataFactory factory = new DefaultHttpDataFactory(true);
+
+        commonTestBigFileDelimiterInMiddleChunk(factory, false);
+    }
+
+    @Test
+    public void testBIgFileUploadDelimiterInMiddleChunkDecoderMemoryFactory() throws IOException {
+        // Factory using Memory mode
+        HttpDataFactory factory = new DefaultHttpDataFactory(false);
+
+        commonTestBigFileDelimiterInMiddleChunk(factory, true);
+    }
+
+    @Test
+    public void testBIgFileUploadDelimiterInMiddleChunkDecoderMixedFactory() throws IOException {
+        // Factory using Mixed mode, where file shall be on Disk
+        HttpDataFactory factory = new DefaultHttpDataFactory(10000);
+
+        commonTestBigFileDelimiterInMiddleChunk(factory, false);
+    }
+
+    private void commonNotBadReleaseBuffersDuringDecoding(HttpDataFactory factory, boolean inMemory)
+            throws IOException {
+        int nbItems = 20;
+        int bytesPerItem = 1000;
+        int maxMemory = 500;
+
+        String prefix1 = "\n--861fbeab-cd20-470c-9609-d40a0f704466\n" +
+                "Content-Disposition: form-data; name=\"image";
+        String prefix2 =
+                "\"; filename=\"guangzhou.jpeg\"\n" +
+                        "Content-Type: image/jpeg\n" +
+                        "Content-Length: " + bytesPerItem + "\n" + "\n";
+
+        String suffix = "\n--861fbeab-cd20-470c-9609-d40a0f704466--\n";
+
+        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
+        request.headers().set("content-type", "multipart/form-data; boundary=861fbeab-cd20-470c-9609-d40a0f704466");
+        request.headers().set("content-length", nbItems * (prefix1.length() + prefix2.length() + 2 + bytesPerItem)
+                + suffix.length());
+        HttpPostMultipartRequestDecoder decoder = new HttpPostMultipartRequestDecoder(factory, request);
+        decoder.setDiscardThreshold(maxMemory);
+        for (int rank = 0; rank < nbItems; rank++) {
+            byte[] bp1 = prefix1.getBytes(CharsetUtil.UTF_8);
+            byte[] bp2 = prefix2.getBytes(CharsetUtil.UTF_8);
+            byte[] prefix = new byte[bp1.length + 2 + bp2.length];
+            for (int i = 0; i < bp1.length; i++) {
+                prefix[i] = bp1[i];
+            }
+            byte[] brank = Integer.toString(10 + rank).getBytes(CharsetUtil.UTF_8);
+            prefix[bp1.length] = brank[0];
+            prefix[bp1.length + 1] = brank[1];
+            for (int i = 0; i < bp2.length; i++) {
+                prefix[bp1.length + 2 + i] = bp2[i];
+            }
+            decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(prefix)));
+            byte[] body = new byte[bytesPerItem];
+            Arrays.fill(body, (byte) rank);
+            ByteBuf content = Unpooled.wrappedBuffer(body, 0, bytesPerItem);
+            decoder.offer(new DefaultHttpContent(content));
+            content.release();
+        }
+        byte[] lastbody = suffix.getBytes(CharsetUtil.UTF_8);
+        ByteBuf content2 = Unpooled.wrappedBuffer(lastbody, 0, lastbody.length);
+        decoder.offer(new DefaultHttpContent(content2));
+        content2.release();
+        decoder.offer(new DefaultLastHttpContent());
+
+        for (int rank = 0; rank < nbItems; rank++) {
+            FileUpload data = (FileUpload) decoder.getBodyHttpData("image" + (10 + rank));
+            assertEquals(data.length(), bytesPerItem);
+            assertEquals(inMemory, data.isInMemory());
+            byte[] body = new byte[bytesPerItem];
+            Arrays.fill(body, (byte) rank);
+            assertTrue(Arrays.equals(body, data.get()));
+        }
+        // To not be done since will load full file on memory: assertEquals(data.get().length, fileSize);
+        // Not mandatory since implicitely called during destroy of decoder
+        for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
+            httpData.release();
+            factory.removeHttpDataFromClean(request, httpData);
+        }
+        factory.cleanAllHttpData();
+        decoder.destroy();
+    }
+    @Test
+    public void testNotBadReleaseBuffersDuringDecodingDiskFactory() throws IOException {
+        // Using Disk Factory
+        HttpDataFactory factory = new DefaultHttpDataFactory(true);
+        commonNotBadReleaseBuffersDuringDecoding(factory, false);
+    }
+    @Test
+    public void testNotBadReleaseBuffersDuringDecodingMemoryFactory() throws IOException {
+        // Using Memory Factory
+        HttpDataFactory factory = new DefaultHttpDataFactory(false);
+        commonNotBadReleaseBuffersDuringDecoding(factory, true);
+    }
+    @Test
+    public void testNotBadReleaseBuffersDuringDecodingMixedFactory() throws IOException {
+        // Using Mixed Factory
+        HttpDataFactory factory = new DefaultHttpDataFactory(100);
+        commonNotBadReleaseBuffersDuringDecoding(factory, false);
+    }
 }

microbench/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoderBenchmark.java

@@ -24,6 +24,7 @@ import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpVersion;
 import io.netty.microbench.util.AbstractMicrobenchmark;
+import io.netty.util.CharsetUtil;
 import io.netty.util.ResourceLeakDetector;
 import io.netty.util.ResourceLeakDetector.Level;
 import org.openjdk.jmh.annotations.Benchmark;
@@ -54,12 +55,12 @@ public class HttpPostMultipartRequestDecoderBenchmark
                                  "Content-Disposition: form-data; name=\"msg_id\"\n\n15200\n--" +
                                  BOUNDARY +
                                  "\nContent-Disposition: form-data; name=\"msg1\"; filename=\"file1.txt\"\n\n" +
-                                 data).getBytes();
+                                 data).getBytes(CharsetUtil.UTF_8);
-        byte[] bodyPartBigBytes = data.getBytes();
+        byte[] bodyPartBigBytes = data.getBytes(CharsetUtil.UTF_8);
         byte[] intermediaryBytes = ("\n--" + BOUNDARY +
                                     "\nContent-Disposition: form-data; name=\"msg2\"; filename=\"file2.txt\"\n\n" +
-                                    data).getBytes();
+                                    data).getBytes(CharsetUtil.UTF_8);
-        byte[] finalBigBytes = ("\n" + "--" + BOUNDARY + "--\n").getBytes();
+        byte[] finalBigBytes = ("\n" + "--" + BOUNDARY + "--\n").getBytes(CharsetUtil.UTF_8);
         ByteBuf firstBuf = Unpooled.wrappedBuffer(bodyStartBytes);
         ByteBuf finalBuf = Unpooled.wrappedBuffer(finalBigBytes);
         ByteBuf nextBuf;