diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java index a366bd3160..b125dfae0e 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java @@ -123,6 +123,11 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest */ private Attribute currentAttribute; + /** + * The current Data position before finding delimiter + */ + private int lastDataPosition; + private boolean destroyed; private int discardThreshold = HttpPostRequestDecoder.DEFAULT_DISCARD_THRESHOLD; @@ -337,12 +342,15 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest // which is not really usable for us as we may exceed it once we add more bytes. buf.alloc().buffer(buf.readableBytes()).writeBytes(buf); } else { + int readPos = undecodedChunk.readerIndex(); + int writable = undecodedChunk.writableBytes(); + int toWrite = buf.readableBytes(); + if (undecodedChunk.refCnt() == 1 && writable < toWrite && readPos + writable >= toWrite) { + undecodedChunk.discardReadBytes(); + } undecodedChunk.writeBytes(buf); } parseBody(); - if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) { - undecodedChunk.discardReadBytes(); - } return this; } @@ -980,47 +988,6 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest currentFieldAttributes.remove(HttpHeaderValues.FILENAME); } - /** - * Read one line up to the CRLF or LF - * - * @return the String from one line - * @throws NotEnoughDataDecoderException - * Need more chunks and reset the {@code readerIndex} to the previous - * value - */ - private static String readLineStandard(ByteBuf undecodedChunk, Charset charset) { - int readerIndex = undecodedChunk.readerIndex(); - ByteBuf line = undecodedChunk.alloc().heapBuffer(64); - try { - while (undecodedChunk.isReadable()) { - byte nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.CR) { - // check but do not changed readerIndex - 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); - } - } - } catch (IndexOutOfBoundsException e) { - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(e); - } finally { - line.release(); - } - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(); - } - /** * Read one line up to the CRLF or LF * @@ -1030,44 +997,23 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest * value */ private static String readLine(ByteBuf undecodedChunk, Charset charset) { - if (!undecodedChunk.hasArray()) { - return readLineStandard(undecodedChunk, charset); + final int readerIndex = undecodedChunk.readerIndex(); + int posLf = undecodedChunk.bytesBefore(HttpConstants.LF); + if (posLf == -1) { + throw new NotEnoughDataDecoderException(); } - 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 { - line.writeByte(nextByte); - } - } - } catch (IndexOutOfBoundsException e) { - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(e); - } finally { - line.release(); + boolean crFound = + undecodedChunk.getByte(readerIndex + posLf - 1) == HttpConstants.CR; + if (crFound) { + posLf--; } - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(); + CharSequence line = undecodedChunk.readCharSequence(posLf, charset); + if (crFound) { + undecodedChunk.skipBytes(2); + } else { + undecodedChunk.skipBytes(1); + } + return line.toString(); } /** @@ -1085,201 +1031,73 @@ 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) { - int readerIndex = undecodedChunk.readerIndex(); + private String readDelimiter(ByteBuf undecodedChunk, String delimiter) { + final int readerIndex = undecodedChunk.readerIndex(); try { - StringBuilder sb = new StringBuilder(64); - int delimiterPos = 0; - int len = delimiter.length(); - while (undecodedChunk.isReadable() && delimiterPos < len) { - byte nextByte = undecodedChunk.readByte(); - if (nextByte == delimiter.charAt(delimiterPos)) { - delimiterPos++; - sb.append((char) nextByte); + final int len = delimiter.length(); + if (len + 2 > undecodedChunk.readableBytes()) { + // Not able to check if "--" is present + throw new NotEnoughDataDecoderException(); + } + int newPositionDelimiter = findDelimiter(undecodedChunk, delimiter, 0); + if (newPositionDelimiter != 0) { + // Delimiter not fully found + throw new NotEnoughDataDecoderException(); + } + byte nextByte = undecodedChunk.getByte(readerIndex + len); + // first check for opening delimiter + if (nextByte == HttpConstants.CR) { + nextByte = undecodedChunk.getByte(readerIndex + len + 1); + if (nextByte == HttpConstants.LF) { + CharSequence line = undecodedChunk.readCharSequence(len, charset); + undecodedChunk.skipBytes(2); + return line.toString(); } else { + // error since CR must be followed by LF // delimiter not found so break here ! undecodedChunk.readerIndex(readerIndex); throw new NotEnoughDataDecoderException(); } - } - // Now check if either opening delimiter or closing delimiter - if (undecodedChunk.isReadable()) { - byte nextByte = undecodedChunk.readByte(); - // first check for opening delimiter - if (nextByte == HttpConstants.CR) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - return sb.toString(); - } 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) { - return sb.toString(); - } else if (nextByte == '-') { - sb.append('-'); - // second check for closing delimiter - nextByte = undecodedChunk.readByte(); - if (nextByte == '-') { - sb.append('-'); - // now try to find if CRLF or LF there - if (undecodedChunk.isReadable()) { + } else if (nextByte == HttpConstants.LF) { + CharSequence line = undecodedChunk.readCharSequence(len, charset); + undecodedChunk.skipBytes(1); + return line.toString(); + } else if (nextByte == '-') { + // second check for closing delimiter + nextByte = undecodedChunk.getByte(readerIndex + len + 1); + if (nextByte == '-') { + CharSequence line = undecodedChunk.readCharSequence(len + 2, charset); + // now try to find if CRLF or LF there + if (undecodedChunk.isReadable()) { + nextByte = undecodedChunk.readByte(); + if (nextByte == HttpConstants.CR) { nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.CR) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - return sb.toString(); - } else { - // error CR without LF - // delimiter not found so break here ! - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(); - } - } else if (nextByte == HttpConstants.LF) { - return sb.toString(); + if (nextByte == HttpConstants.LF) { + return line.toString(); } else { - // No CRLF but ok however (Adobe Flash uploader) - // minus 1 since we read one char ahead but - // should not - undecodedChunk.readerIndex(undecodedChunk.readerIndex() - 1); - return sb.toString(); + // error CR without LF + // delimiter not found so break here ! + undecodedChunk.readerIndex(readerIndex); + throw new NotEnoughDataDecoderException(); } - } - // FIXME what do we do here? - // either considering it is fine, either waiting for - // more data to come? - // lets try considering it is fine... - return sb.toString(); - } - // only one '-' => not enough - // whatever now => error since incomplete - } - } - } catch (IndexOutOfBoundsException e) { - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(e); - } - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(); - } - - /** - * Read one line up to --delimiter or --delimiter-- and if existing the CRLF - * or LF. Note that CRLF or LF are mandatory for opening delimiter - * (--delimiter) but not for closing delimiter (--delimiter--) since some - * clients does not include CRLF in this case. - * - * @param delimiter - * of the form --string, such that '--' is already included - * @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) { - if (!undecodedChunk.hasArray()) { - return readDelimiterStandard(undecodedChunk, delimiter); - } - SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk); - int readerIndex = undecodedChunk.readerIndex(); - int delimiterPos = 0; - int len = delimiter.length(); - try { - StringBuilder sb = new StringBuilder(64); - // 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 if (nextByte == HttpConstants.LF) { + return line.toString(); } else { - // error CR without LF - // delimiter not found so break here ! - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(); + // No CRLF but ok however (Adobe Flash uploader) + // minus 1 since we read one char ahead but + // should not + undecodedChunk.readerIndex(undecodedChunk.readerIndex() - 1); + return line.toString(); } - } 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 } + // FIXME what do we do here? + // either considering it is fine, either waiting for + // more data to come? + // lets try considering it is fine... + return line.toString(); } + // only one '-' => not enough + // whatever now => error since incomplete } } catch (IndexOutOfBoundsException e) { undecodedChunk.readerIndex(readerIndex); @@ -1290,96 +1108,93 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } /** - * Load the field value or file data from a Multipart request + * @param undecodedChunk the source where the delimiter is to be found + * @param delimiter the string to find out + * @param offset the offset from readerIndex within the undecodedChunk to + * start from to find out the delimiter * - * @return {@code true} if the last chunk is loaded (boundary delimiter found), {@code false} if need more chunks - * @throws ErrorDataDecoderException + * @return a number >= 0 if found, else new offset with negative value + * (to inverse), both from readerIndex + * @throws NotEnoughDataDecoderException + * Need more chunks while relative position with readerIndex is 0 */ - private static boolean loadDataMultipartStandard(ByteBuf undecodedChunk, String delimiter, HttpData httpData) { + private static int findDelimiter(ByteBuf undecodedChunk, String delimiter, int offset) { final int startReaderIndex = undecodedChunk.readerIndex(); final int delimeterLength = delimiter.length(); - int index = 0; - int lastPosition = startReaderIndex; - byte prevByte = HttpConstants.LF; - boolean delimiterFound = false; - while (undecodedChunk.isReadable()) { - final byte nextByte = undecodedChunk.readByte(); - // Check the delimiter - if (prevByte == HttpConstants.LF && nextByte == delimiter.codePointAt(index)) { - index++; - if (delimeterLength == index) { - delimiterFound = true; + final int toRead = undecodedChunk.readableBytes(); + int newOffset = offset; + boolean delimiterNotFound = true; + while (delimiterNotFound && newOffset + delimeterLength <= toRead) { + int posFirstChar = undecodedChunk + .bytesBefore(startReaderIndex + newOffset, toRead - newOffset, + (byte) delimiter.codePointAt(0)); + if (posFirstChar == -1) { + newOffset = toRead; + return -newOffset; + } + newOffset = posFirstChar + offset; + if (newOffset + delimeterLength > toRead) { + return -newOffset; + } + // assume will found it + delimiterNotFound = false; + for (int index = 1; index < delimeterLength; index++) { + if (undecodedChunk.getByte(startReaderIndex + newOffset + index) != delimiter.codePointAt(index)) { + // ignore first found offset and redo search from next char + newOffset++; + delimiterNotFound = true; break; } - continue; } - lastPosition = undecodedChunk.readerIndex(); - if (nextByte == HttpConstants.LF) { - index = 0; - lastPosition -= (prevByte == HttpConstants.CR)? 2 : 1; + } + if (delimiterNotFound || newOffset + delimeterLength > toRead) { + if (newOffset == 0) { + throw new NotEnoughDataDecoderException(); } - prevByte = nextByte; + return -newOffset; } - if (prevByte == HttpConstants.CR) { - lastPosition--; - } - ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, lastPosition - startReaderIndex); - try { - httpData.addContent(content, delimiterFound); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - undecodedChunk.readerIndex(lastPosition); - return delimiterFound; + return newOffset; } /** * 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); + private boolean loadDataMultipart(ByteBuf undecodedChunk, String delimiter, + HttpData httpData) { 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); + int newOffset; try { - httpData.addContent(content, delimiterFound); + newOffset = findDelimiter(undecodedChunk, delimiter, lastDataPosition); + if (newOffset < 0) { + // delimiter not found + lastDataPosition = -newOffset; + return false; + } + } catch (NotEnoughDataDecoderException e) { + // Not enough data and no change to lastDataPosition + return false; + } + // found delimiter but still need to check if CRLF before + int startDelimiter = newOffset; + if (undecodedChunk.getByte(startReaderIndex + startDelimiter - 1) == HttpConstants.LF) { + startDelimiter--; + if (undecodedChunk.getByte(startReaderIndex + startDelimiter - 1) == HttpConstants.CR) { + startDelimiter--; + } + } + ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, startDelimiter); + try { + httpData.addContent(content, true); } catch (IOException e) { throw new ErrorDataDecoderException(e); } - undecodedChunk.readerIndex(lastPosition); - return delimiterFound; + lastDataPosition = 0; + undecodedChunk.readerIndex(startReaderIndex + startDelimiter); + return true; } /** diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java index 37e41fa917..89733794a9 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java @@ -299,12 +299,15 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD // which is not really usable for us as we may exceed it once we add more bytes. buf.alloc().buffer(buf.readableBytes()).writeBytes(buf); } else { + int readPos = undecodedChunk.readerIndex(); + int writable = undecodedChunk.writableBytes(); + int toWrite = buf.readableBytes(); + if (undecodedChunk.refCnt() == 1 && writable < toWrite && readPos + writable >= toWrite) { + undecodedChunk.discardReadBytes(); + } undecodedChunk.writeBytes(buf); } parseBody(); - if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) { - undecodedChunk.discardReadBytes(); - } return this; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java index ba01e4d240..45b8461ce7 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java @@ -132,6 +132,7 @@ public class HttpPostRequestDecoderTest { // Create decoder instance to test. final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); assertFalse(decoder.getBodyHttpDatas().isEmpty()); + req.release(); decoder.destroy(); } @@ -178,6 +179,7 @@ public class HttpPostRequestDecoderTest { assertNotNull(datar); assertEquals(datas[i].getBytes(CharsetUtil.UTF_8).length, datar.length); + req.release(); decoder.destroy(); } } @@ -211,6 +213,7 @@ public class HttpPostRequestDecoderTest { // Create decoder instance to test. final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); assertFalse(decoder.getBodyHttpDatas().isEmpty()); + req.release(); decoder.destroy(); } @@ -368,6 +371,7 @@ public class HttpPostRequestDecoderTest { // Create decoder instance to test. final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); assertFalse(decoder.getBodyHttpDatas().isEmpty()); + req.release(); decoder.destroy(); } @@ -397,6 +401,7 @@ public class HttpPostRequestDecoderTest { assertTrue(part1 instanceof FileUpload); FileUpload fileUpload = (FileUpload) part1; assertEquals("tmp 0.txt", fileUpload.getFilename()); + req.release(); decoder.destroy(); } @@ -427,6 +432,7 @@ public class HttpPostRequestDecoderTest { // Create decoder instance to test without any exception. final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); assertFalse(decoder.getBodyHttpDatas().isEmpty()); + req.release(); decoder.destroy(); } @@ -459,8 +465,8 @@ public class HttpPostRequestDecoderTest { FileUpload fileUpload = (FileUpload) part1; byte[] fileBytes = fileUpload.get(); assertTrue("the filecontent should not be decoded", filecontent.equals(new String(fileBytes))); - decoder.destroy(); req.release(); + decoder.destroy(); } @Test @@ -538,8 +544,8 @@ public class HttpPostRequestDecoderTest { } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { assertTrue(e.getCause() instanceof IllegalArgumentException); } finally { - decoder.destroy(); content.release(); + decoder.destroy(); } } @@ -573,8 +579,8 @@ public class HttpPostRequestDecoderTest { assertTrue("the item should be a FileUpload", part1 instanceof FileUpload); FileUpload fileUpload = (FileUpload) part1; assertEquals("the filename should be decoded", filename, fileUpload.getFilename()); - decoder.destroy(); req.release(); + decoder.destroy(); } // https://github.com/netty/netty/pull/7265 @@ -609,8 +615,8 @@ public class HttpPostRequestDecoderTest { assertTrue("the item should be a FileUpload", part1 instanceof FileUpload); FileUpload fileUpload = (FileUpload) part1; assertEquals("the filename should be decoded", filename, fileUpload.getFilename()); - decoder.destroy(); req.release(); + decoder.destroy(); } // https://github.com/netty/netty/pull/7265 @@ -704,6 +710,7 @@ public class HttpPostRequestDecoderTest { assertTrue(part1 instanceof FileUpload); FileUpload fileUpload = (FileUpload) part1; assertEquals("tmp-0.txt", fileUpload.getFilename()); + req.release(); decoder.destroy(); } @@ -752,7 +759,7 @@ public class HttpPostRequestDecoderTest { FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", Unpooled.copiedBuffer("a=1&&b=2", CharsetUtil.US_ASCII)); try { - new HttpPostStandardRequestDecoder(request); + new HttpPostStandardRequestDecoder(request).destroy(); } finally { assertTrue(request.release()); } @@ -772,7 +779,7 @@ public class HttpPostRequestDecoderTest { buf.writeCharSequence("a=b&foo=%22bar%22&==", CharsetUtil.US_ASCII); FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", buf); try { - new HttpPostStandardRequestDecoder(request); + new HttpPostStandardRequestDecoder(request).destroy(); } finally { assertTrue(request.release()); } @@ -823,8 +830,8 @@ public class HttpPostRequestDecoderTest { FileUpload fileUpload = (FileUpload) part1; assertEquals("the filename should be decoded", filename, fileUpload.getFilename()); - decoder.destroy(); req.release(); + decoder.destroy(); } @Test @@ -860,8 +867,8 @@ public class HttpPostRequestDecoderTest { assertTrue(attr.getByteBuf().isDirect()); assertEquals("los angeles", attr.getValue()); - decoder.destroy(); req.release(); + decoder.destroy(); } @Test @@ -877,7 +884,7 @@ public class HttpPostRequestDecoderTest { } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { assertEquals("Invalid hex byte at index '0' in string: '%'", e.getMessage()); } finally { - req.release(); + assertTrue(req.release()); } } @@ -894,7 +901,7 @@ public class HttpPostRequestDecoderTest { } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { assertEquals("Invalid hex byte at index '0' in string: '%2'", e.getMessage()); } finally { - req.release(); + assertTrue(req.release()); } } @@ -911,7 +918,7 @@ public class HttpPostRequestDecoderTest { } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { assertEquals("Invalid hex byte at index '0' in string: '%Zc'", e.getMessage()); } finally { - req.release(); + assertTrue(req.release()); } } @@ -928,7 +935,7 @@ public class HttpPostRequestDecoderTest { } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { assertEquals("Invalid hex byte at index '0' in string: '%2g'", e.getMessage()); } finally { - req.release(); + assertTrue(req.release()); } } } diff --git a/microbench/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoderBenchmark.java b/microbench/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoderBenchmark.java new file mode 100644 index 0000000000..c570cd6745 --- /dev/null +++ b/microbench/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoderBenchmark.java @@ -0,0 +1,202 @@ +/* + * Copyright 2020 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http.multipart; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.handler.codec.http.DefaultHttpContent; +import io.netty.handler.codec.http.DefaultHttpRequest; +import io.netty.handler.codec.http.DefaultLastHttpContent; +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.ResourceLeakDetector; +import io.netty.util.ResourceLeakDetector.Level; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +import java.util.concurrent.TimeUnit; + + +@Threads(1) +@Warmup(iterations = 2) +@Measurement(iterations = 3) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +public class HttpPostMultipartRequestDecoderBenchmark + extends AbstractMicrobenchmark { + + public double testHighNumberChunks(boolean big, boolean noDisk) { + String BOUNDARY = "01f136d9282f"; + int size = 8 * 1024; + int chunkNumber = 64; + StringBuilder stringBuilder = new StringBuilder(size); + stringBuilder.setLength(size); + String data = stringBuilder.toString(); + + byte[] bodyStartBytes = ("--" + BOUNDARY + "\n" + + "Content-Disposition: form-data; name=\"msg_id\"\n\n15200\n--" + + BOUNDARY + + "\nContent-Disposition: form-data; name=\"msg1\"; filename=\"file1.txt\"\n\n" + + data).getBytes(); + byte[] bodyPartBigBytes = data.getBytes(); + byte[] intermediaryBytes = ("\n--" + BOUNDARY + + "\nContent-Disposition: form-data; name=\"msg2\"; filename=\"file2.txt\"\n\n" + + data).getBytes(); + byte[] finalBigBytes = ("\n" + "--" + BOUNDARY + "--\n").getBytes(); + ByteBuf firstBuf = Unpooled.wrappedBuffer(bodyStartBytes); + ByteBuf finalBuf = Unpooled.wrappedBuffer(finalBigBytes); + ByteBuf nextBuf; + if (big) { + nextBuf = Unpooled.wrappedBuffer(bodyPartBigBytes); + } else { + nextBuf = Unpooled.wrappedBuffer(intermediaryBytes); + } + DefaultHttpRequest req = + new DefaultHttpRequest(HttpVersion.HTTP_1_0, HttpMethod.POST, "/up"); + req.headers().add(HttpHeaderNames.CONTENT_TYPE, + "multipart/form-data; boundary=" + BOUNDARY); + + long start = System.nanoTime(); + + DefaultHttpDataFactory defaultHttpDataFactory = + new DefaultHttpDataFactory(noDisk? 1024 * 1024 : 16 * 1024); + HttpPostRequestDecoder decoder = + new HttpPostRequestDecoder(defaultHttpDataFactory, req); + firstBuf.retain(); + decoder.offer(new DefaultHttpContent(firstBuf)); + firstBuf.release(); + for (int i = 1; i < chunkNumber; i++) { + nextBuf.retain(); + decoder.offer(new DefaultHttpContent(nextBuf)); + nextBuf.release(); + nextBuf.readerIndex(0); + } + finalBuf.retain(); + decoder.offer(new DefaultLastHttpContent(finalBuf)); + finalBuf.release(); + while (decoder.hasNext()) { + InterfaceHttpData httpData = decoder.next(); + } + while (finalBuf.refCnt() > 0) { + finalBuf.release(); + } + while (nextBuf.refCnt() > 0) { + nextBuf.release(); + } + while (finalBuf.refCnt() > 0) { + finalBuf.release(); + } + long stop = System.nanoTime(); + double time = (stop - start) / 1000000.0; + defaultHttpDataFactory.cleanAllHttpData(); + defaultHttpDataFactory.cleanRequestHttpData(req); + decoder.destroy(); + return time; + } + + @Benchmark + public double multipartRequestDecoderHighDisabledLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.DISABLED); + return testHighNumberChunks(false, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderBigDisabledLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.DISABLED); + return testHighNumberChunks(true, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderHighSimpleLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.SIMPLE); + return testHighNumberChunks(false, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderBigSimpleLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.SIMPLE); + return testHighNumberChunks(true, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderHighAdvancedLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.ADVANCED); + return testHighNumberChunks(false, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderBigAdvancedLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.ADVANCED); + return testHighNumberChunks(true, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderHighParanoidLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.PARANOID); + return testHighNumberChunks(false, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + + @Benchmark + public double multipartRequestDecoderBigParanoidLevel() { + final Level level = ResourceLeakDetector.getLevel(); + try { + ResourceLeakDetector.setLevel(Level.PARANOID); + return testHighNumberChunks(true, true); + } finally { + ResourceLeakDetector.setLevel(level); + } + } + +} diff --git a/microbench/src/main/java/io/netty/handler/codec/http/multipart/package-info.java b/microbench/src/main/java/io/netty/handler/codec/http/multipart/package-info.java new file mode 100644 index 0000000000..1f4cc119dc --- /dev/null +++ b/microbench/src/main/java/io/netty/handler/codec/http/multipart/package-info.java @@ -0,0 +1,19 @@ +/* + * Copyright 2020 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +/** + * Benchmarks for {@link io.netty.handler.codec.http.multipart}. + */ +package io.netty.handler.codec.http.multipart;