Repository: netty/netty
Issue: HttpPostMultipartRequestDecoder IndexOutOfBoundsException error with netty 4.1.65.Final

The problem happens when using latests vert.x version. see https://github.com/eclipse-vertx/vert.x/issues/3949
But I guess is a problem with netty. 

### Expected behavior
Uploading multiple form datas/files will success. 

### Actual behavior
Fails ramdomly. 

### Steps to reproduce
See original issue. I have a project test case, but not have a minimal test case with netty yet. 

### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.65.Final . After  I change to 4.1.59.Final, this problem is gone. 

### JVM version (e.g. `java -version`)

### OS version (e.g. `uname -a`)


Issue comments:
@liubao68 I think I have found the reason.

@normanmaurer except a check that was missing in the multipartDecoder, i've found a possible issue while debugging within `UnpooledUnsafeDirectByteBuf` (at least there).
The reason we did not get this issue before in the Junit test was because the underlying check of access to the `undecodedChunk` while using `buffer.getByte(fullPosition)` is only checking the following the following in MathUtil:

    /**
     * Determine if the requested {@code index} and {@code length} will fit within {@code capacity}.
     * @param index The starting index.
     * @param length The length which will be utilized (starting from {@code index}).
     * @param capacity The capacity that {@code index + length} is allowed to be within.
     * @return {@code false} if the requested {@code index} and {@code length} will fit within {@code capacity}.
     * {@code true} if this would result in an index out of bounds exception.
     */
    public static boolean isOutOfBounds(int index, int length, int capacity) {
        return (index | length | capacity | (index + length) | (capacity - (index + length))) < 0;
    }

But the underlying buffer has a capacity bigger than writerIndex, so accessing `buffer.getByte(writerIndex)` does not thrown an `IndexOutOfBoundsException` (index = writerIndex, length = 1, capacity = 2048).

So when I try to reproduce this issue, I am in fact unable to do so, since there is never such an issue but it should.

I will propose first a fix for the multipart decoder (simple).
But I don't know how to adress this second issue (it should raized an `IndexOutOfBoundsException` if it is a `read` operation). The implication could be very high (since used everywhere).

The stacktrace to understand where it comes:

    isOutOfBounds:64, MathUtil (io.netty.util.internal)
    checkRangeBounds:1389, AbstractByteBuf (io.netty.buffer)
    checkIndex0:1397, AbstractByteBuf (io.netty.buffer)
    checkIndex:1384, AbstractByteBuf (io.netty.buffer)
    checkIndex:1379, AbstractByteBuf (io.netty.buffer)
    getByte:85, UnpooledUnsafeDirectByteBuf (io.netty.buffer)
    findDelimiter:239, HttpPostBodyUtil (io.netty.handler.codec.http.multipart)
    loadDataMultipartOptimized:1172, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)
    getFileUpload:926, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)
    decodeMultipart:572, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)
    parseBodyMultipart:463, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)
    parseBody:432, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)
    offer:347, HttpPostMultipartRequestDecoder (io.netty.handler.codec.http.multipart)



Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostBodyUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostBodyUtil.java
index f174327d912..e93dfaf216a 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostBodyUtil.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostBodyUtil.java
@@ -233,13 +233,15 @@ static int findDelimiter(ByteBuf buffer, int index, byte[] delimiter, boolean pr
             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 (toRead >= delimiterLength) {
+                delimiterNotFound = false;
+                for (int i = 0; i < delimiterLength; i++) {
+                    if (buffer.getByte(newOffset + i) != delimiter[i]) {
+                        newOffset++;
+                        toRead--;
+                        delimiterNotFound = true;
+                        break;
+                    }
                 }
             }
             if (!delimiterNotFound) {


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
index d7d3ab8342b..fb7e08202dd 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
@@ -95,6 +95,63 @@ public void testDecodeFullHttpRequestWithInvalidPayloadReleaseBuffer() {
         }
     }
 
+    @Test
+    public void testDelimiterExceedLeftSpaceInCurrentBuffer() {
+        String delimiter = "--861fbeab-cd20-470c-9609-d40a0f704466";
+        String suffix = '\n' + delimiter + "--\n";
+        byte[] bsuffix = suffix.getBytes(CharsetUtil.UTF_8);
+        int partOfDelimiter = bsuffix.length / 2;
+        int bytesLastChunk = 355 - partOfDelimiter; // to try to have an out of bound since content is > delimiter
+        byte[] bsuffix1 = Arrays.copyOf(bsuffix, partOfDelimiter);
+        byte[] bsuffix2 = Arrays.copyOfRange(bsuffix, partOfDelimiter, bsuffix.length);
+        String prefix = delimiter + "\n" +
+                        "Content-Disposition: form-data; name=\"image\"; filename=\"guangzhou.jpeg\"\n" +
+                        "Content-Type: image/jpeg\n" +
+                        "Content-Length: " + bytesLastChunk + "\n\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", prefix.length() + bytesLastChunk + suffix.length());
+
+        // Factory using Memory mode
+        HttpDataFactory factory = new DefaultHttpDataFactory(false);
+        HttpPostMultipartRequestDecoder decoder = new HttpPostMultipartRequestDecoder(factory, request);
+        ByteBuf buf = Unpooled.wrappedBuffer(prefix.getBytes(CharsetUtil.UTF_8));
+        decoder.offer(new DefaultHttpContent(buf));
+        assertNotNull((HttpData) decoder.currentPartialHttpData());
+        buf.release();
+        // Chunk less than Delimiter size but containing part of delimiter
+        byte[] body = new byte[bytesLastChunk + bsuffix1.length];
+        Arrays.fill(body, (byte) 2);
+        for (int i = 0; i < bsuffix1.length; i++) {
+            body[bytesLastChunk + i] = bsuffix1[i];
+        }
+        ByteBuf content = Unpooled.wrappedBuffer(body);
+        decoder.offer(new DefaultHttpContent(content)); // Ouf of range before here
+        assertNotNull(((HttpData) decoder.currentPartialHttpData()).content());
+        content.release();
+        content = Unpooled.wrappedBuffer(bsuffix2);
+        decoder.offer(new DefaultHttpContent(content));
+        assertNull((HttpData) decoder.currentPartialHttpData());
+        content.release();
+        decoder.offer(new DefaultLastHttpContent());
+        FileUpload data = (FileUpload) decoder.getBodyHttpDatas().get(0);
+        assertEquals(data.length(), bytesLastChunk);
+        assertEquals(true, data.isInMemory());
+
+        InterfaceHttpData[] httpDatas = decoder.getBodyHttpDatas().toArray(new InterfaceHttpData[0]);
+        for (InterfaceHttpData httpData : httpDatas) {
+            assertEquals(1, httpData.refCnt(), "Before cleanAllHttpData should be 1");
+        }
+        factory.cleanAllHttpData();
+        for (InterfaceHttpData httpData : httpDatas) {
+            assertEquals(1, httpData.refCnt(), "After cleanAllHttpData should be 1 if in Memory");
+        }
+        decoder.destroy();
+        for (InterfaceHttpData httpData : httpDatas) {
+            assertEquals(0, httpData.refCnt(), "RefCnt should be 0");
+        }
+    }
+
     private void commonTestBigFileDelimiterInMiddleChunk(HttpDataFactory factory, boolean inMemory)
             throws IOException {
         int nbChunks = 100;
@@ -184,7 +241,7 @@ private void commonTestBigFileDelimiterInMiddleChunk(HttpDataFactory factory, bo
         }
         factory.cleanAllHttpData();
         for (InterfaceHttpData httpData : httpDatas) {
-            assertEquals(inMemory? 1 : 0, httpData.refCnt(), "Before cleanAllHttpData should be 1 if in Memory");
+            assertEquals(inMemory? 1 : 0, httpData.refCnt(), "After cleanAllHttpData should be 1 if in Memory");
         }
         decoder.destroy();
         for (InterfaceHttpData httpData : httpDatas) {