From 103f1d269ed0e4cb335241286e53973703574626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Br=C3=A9gier?= Date: Mon, 31 May 2021 08:43:39 +0200 Subject: [PATCH] HttpPostMultipartRequestDecoder IndexOutOfBoundsException error (#11335) Motivation: When searching for the delimiter, the decoder part within HttpPostBodyUtil was not checking the left space to check if it could be included or not, while it should. Modifications: Add a check on toRead being greater or equal than delimiterLength before going within the loop. If the check is wrong, the delimiter is obviously not found. Add a Junit test to preserve regression. Result: No more IndexOutOfBoundsException Fixes #11334 --- .../http/multipart/HttpPostBodyUtil.java | 16 ++--- .../HttpPostMultiPartRequestDecoderTest.java | 59 ++++++++++++++++++- 2 files changed, 67 insertions(+), 8 deletions(-) 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 f174327d91..e93dfaf216 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 @@ final class HttpPostBodyUtil { 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) { 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 d7d3ab8342..fb7e08202d 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 class HttpPostMultiPartRequestDecoderTest { } } + @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 @@ public class HttpPostMultiPartRequestDecoderTest { } 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) {