From 3e9f617504adb6f3ac984a9320020dd2c202739e Mon Sep 17 00:00:00 2001 From: Nikolay Fedorovskikh Date: Tue, 20 Jun 2017 03:56:01 +0500 Subject: [PATCH] Deduplicate and simplify code in HttpPostMultipartRequestDecoder Motivation: - A `HttpPostMultipartRequestDecoder` contains two pairs of the same methods: `readFileUploadByteMultipartStandard`+`readFileUploadByteMultipart` and `loadFieldMultipartStandard`+`loadFieldMultipart`. - These methods use `NotEnoughDataDecoderException` to detecting not last data chunk (exception handling is very expensive). - These methods can be greatly simplified. - Methods `loadFieldMultipart` and `loadFieldMultipartStandard` has an unnecessary catching for the `IndexOutOfBoundsException`. Modifications: - Remove duplicate methods. - Replace handling `NotEnoughDataDecoderException` by the return of a boolean result. - Simplify code. Result: The code is cleaner and easier to support. Less exception handling logic. --- .../HttpPostMultipartRequestDecoder.java | 519 +++--------------- 1 file changed, 84 insertions(+), 435 deletions(-) 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 17c3e64467..e60b93b7a3 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 @@ -511,7 +511,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest .getValue()) : 0L; } catch (IOException e) { throw new ErrorDataDecoderException(e); - } catch (NumberFormatException e) { + } catch (NumberFormatException ignored) { size = 0; } try { @@ -534,9 +534,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } } // load data - try { - loadFieldMultipart(undecodedChunk, multipartDataBoundary, currentAttribute); - } catch (NotEnoughDataDecoderException ignored) { + if (!loadDataMultipart(undecodedChunk, multipartDataBoundary, currentAttribute)) { + // Delimiter is not found. Need more chunks. return null; } Attribute finalAttribute = currentAttribute; @@ -883,12 +882,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } } // load data as much as possible - try { - readFileUploadByteMultipart(undecodedChunk, delimiter, currentFileUpload); - } catch (NotEnoughDataDecoderException e) { - // do not change the buffer position - // since some can be already saved into FileUpload - // So do not change the currentStatus + if (!loadDataMultipart(undecodedChunk, delimiter, currentFileUpload)) { + // Delimiter is not found. Need more chunks. return null; } if (currentFileUpload.isCompleted()) { @@ -1271,442 +1266,96 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } /** - * Read a FileUpload data as Byte (Binary) and add the bytes directly to the - * FileUpload. If the delimiter is found, the FileUpload is completed. + * Load the field value or file data from a Multipart request * - * @throws NotEnoughDataDecoderException - * Need more chunks but do not reset the readerInder since some - * values will be already added to the FileOutput + * @return {@code true} if the last chunk is loaded (boundary delimiter found), {@code false} if need more chunks * @throws ErrorDataDecoderException - * write IO error occurs with the FileUpload */ - private static void readFileUploadByteMultipartStandard(ByteBuf undecodedChunk, String delimiter, - FileUpload currentFileUpload) { - int readerIndex = undecodedChunk.readerIndex(); - // found the decoder limit - boolean newLine = true; + private static boolean loadDataMultipartStandard(ByteBuf undecodedChunk, String delimiter, HttpData httpData) { + final int startReaderIndex = undecodedChunk.readerIndex(); + final int delimeterLength = delimiter.length(); int index = 0; - int lastPosition = undecodedChunk.readerIndex(); - boolean found = false; + int lastPosition = startReaderIndex; + byte prevByte = HttpConstants.LF; + boolean delimiterFound = false; while (undecodedChunk.isReadable()) { - byte nextByte = undecodedChunk.readByte(); - if (newLine) { - // Check the delimiter - if (nextByte == delimiter.codePointAt(index)) { - index++; - if (delimiter.length() == index) { - found = true; - break; - } - } else { - newLine = false; - index = 0; - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (undecodedChunk.isReadable()) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 2; - } else { - // save last valid position - lastPosition = undecodedChunk.readerIndex() - 1; - - // Unread next byte. - undecodedChunk.readerIndex(lastPosition); - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 1; - } else { - // save last valid position - lastPosition = undecodedChunk.readerIndex(); - } - } - } else { - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (undecodedChunk.isReadable()) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 2; - } else { - // save last valid position - lastPosition = undecodedChunk.readerIndex() - 1; - - // Unread next byte. - undecodedChunk.readerIndex(lastPosition); - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 1; - } else { - // save last valid position - lastPosition = undecodedChunk.readerIndex(); + final byte nextByte = undecodedChunk.readByte(); + // Check the delimiter + if (prevByte == HttpConstants.LF && nextByte == delimiter.codePointAt(index)) { + index++; + if (delimeterLength == index) { + delimiterFound = true; + break; } + continue; } + lastPosition = undecodedChunk.readerIndex(); + if (nextByte == HttpConstants.LF) { + index = 0; + lastPosition -= (prevByte == HttpConstants.CR)? 2 : 1; + } + prevByte = nextByte; } - ByteBuf buffer = undecodedChunk.copy(readerIndex, lastPosition - readerIndex); - if (found) { - // found so lastPosition is correct and final - try { - currentFileUpload.addContent(buffer, true); - // just before the CRLF and delimiter - undecodedChunk.readerIndex(lastPosition); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - } else { - // possibly the delimiter is partially found but still the last - // position is OK - try { - currentFileUpload.addContent(buffer, false); - // last valid char (not CR, not LF, not beginning of delimiter) - undecodedChunk.readerIndex(lastPosition); - throw new NotEnoughDataDecoderException(); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } + if (prevByte == HttpConstants.CR) { + lastPosition--; } + ByteBuf content = undecodedChunk.copy(startReaderIndex, lastPosition - startReaderIndex); + try { + httpData.addContent(content, delimiterFound); + } catch (IOException e) { + throw new ErrorDataDecoderException(e); + } + undecodedChunk.readerIndex(lastPosition); + return delimiterFound; } /** - * Read a FileUpload data as Byte (Binary) and add the bytes directly to the - * FileUpload. If the delimiter is found, the FileUpload is completed. + * Load the field value from a Multipart request * - * @throws NotEnoughDataDecoderException - * Need more chunks but do not reset the {@code readerIndex} since some - * values will be already added to the FileOutput + * @return {@code true} if the last chunk is loaded (boundary delimiter found), {@code false} if need more chunks * @throws ErrorDataDecoderException - * write IO error occurs with the FileUpload */ - private static void readFileUploadByteMultipart(ByteBuf undecodedChunk, String delimiter, - FileUpload currentFileUpload) { + private static boolean loadDataMultipart(ByteBuf undecodedChunk, String delimiter, HttpData httpData) { if (!undecodedChunk.hasArray()) { - readFileUploadByteMultipartStandard(undecodedChunk, delimiter, currentFileUpload); - return; + return loadDataMultipartStandard(undecodedChunk, delimiter, httpData); } - SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk); - int readerIndex = undecodedChunk.readerIndex(); - // found the decoder limit - boolean newLine = true; + final SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk); + final int startReaderIndex = undecodedChunk.readerIndex(); + final int delimeterLength = delimiter.length(); int index = 0; - int lastrealpos = sao.pos; - int lastPosition; - boolean found = false; - + int lastRealPos = sao.pos; + byte prevByte = HttpConstants.LF; + boolean delimiterFound = false; while (sao.pos < sao.limit) { - byte nextByte = sao.bytes[sao.pos++]; - if (newLine) { - // Check the delimiter - if (nextByte == delimiter.codePointAt(index)) { - index++; - if (delimiter.length() == index) { - found = true; - break; - } - } else { - newLine = false; - index = 0; - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (sao.pos < sao.limit) { - nextByte = sao.bytes[sao.pos++]; - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 2; - } else { - // unread next byte - sao.pos--; - - // save last valid position - lastrealpos = sao.pos; - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 1; - } else { - // save last valid position - lastrealpos = sao.pos; - } - } - } else { - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (sao.pos < sao.limit) { - nextByte = sao.bytes[sao.pos++]; - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 2; - } else { - // unread next byte - sao.pos--; - - // save last valid position - lastrealpos = sao.pos; - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 1; - } else { - // save last valid position - lastrealpos = sao.pos; + 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; } - lastPosition = sao.getReadPosition(lastrealpos); - ByteBuf buffer = undecodedChunk.copy(readerIndex, lastPosition - readerIndex); - if (found) { - // found so lastPosition is correct and final - try { - currentFileUpload.addContent(buffer, true); - // just before the CRLF and delimiter - undecodedChunk.readerIndex(lastPosition); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - } else { - // possibly the delimiter is partially found but still the last - // position is OK - try { - currentFileUpload.addContent(buffer, false); - // last valid char (not CR, not LF, not beginning of delimiter) - undecodedChunk.readerIndex(lastPosition); - throw new NotEnoughDataDecoderException(); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } + if (prevByte == HttpConstants.CR) { + lastRealPos--; } - } - - /** - * Load the field value from a Multipart request - * - * @throws NotEnoughDataDecoderException - * Need more chunks - * @throws ErrorDataDecoderException - */ - private static void loadFieldMultipartStandard(ByteBuf undecodedChunk, String delimiter, - Attribute currentAttribute) { - int readerIndex = undecodedChunk.readerIndex(); + final int lastPosition = sao.getReadPosition(lastRealPos); + final ByteBuf content = undecodedChunk.copy(startReaderIndex, lastPosition - startReaderIndex); try { - // found the decoder limit - boolean newLine = true; - int index = 0; - int lastPosition = undecodedChunk.readerIndex(); - boolean found = false; - while (undecodedChunk.isReadable()) { - byte nextByte = undecodedChunk.readByte(); - if (newLine) { - // Check the delimiter - if (nextByte == delimiter.codePointAt(index)) { - index++; - if (delimiter.length() == index) { - found = true; - break; - } - } else { - newLine = false; - index = 0; - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (undecodedChunk.isReadable()) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 2; - } else { - // Unread second nextByte - lastPosition = undecodedChunk.readerIndex() - 1; - undecodedChunk.readerIndex(lastPosition); - } - } else { - lastPosition = undecodedChunk.readerIndex() - 1; - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 1; - } else { - lastPosition = undecodedChunk.readerIndex(); - } - } - } else { - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (undecodedChunk.isReadable()) { - nextByte = undecodedChunk.readByte(); - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 2; - } else { - // Unread second nextByte - lastPosition = undecodedChunk.readerIndex() - 1; - undecodedChunk.readerIndex(lastPosition); - } - } else { - lastPosition = undecodedChunk.readerIndex() - 1; - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastPosition = undecodedChunk.readerIndex() - 1; - } else { - lastPosition = undecodedChunk.readerIndex(); - } - } - } - if (found) { - // found so lastPosition is correct - // but position is just after the delimiter (either close - // delimiter or simple one) - // so go back of delimiter size - try { - currentAttribute.addContent( - undecodedChunk.copy(readerIndex, lastPosition - readerIndex), true); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - undecodedChunk.readerIndex(lastPosition); - } else { - try { - currentAttribute.addContent( - undecodedChunk.copy(readerIndex, lastPosition - readerIndex), false); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - undecodedChunk.readerIndex(lastPosition); - throw new NotEnoughDataDecoderException(); - } - } catch (IndexOutOfBoundsException e) { - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(e); - } - } - - /** - * Load the field value from a Multipart request - * - * @throws NotEnoughDataDecoderException - * Need more chunks - * @throws ErrorDataDecoderException - */ - private static void loadFieldMultipart(ByteBuf undecodedChunk, String delimiter, Attribute currentAttribute) { - if (!undecodedChunk.hasArray()) { - loadFieldMultipartStandard(undecodedChunk, delimiter, currentAttribute); - return; - } - SeekAheadOptimize sao = new SeekAheadOptimize(undecodedChunk); - int readerIndex = undecodedChunk.readerIndex(); - try { - // found the decoder limit - boolean newLine = true; - int index = 0; - int lastPosition; - int lastrealpos = sao.pos; - boolean found = false; - - while (sao.pos < sao.limit) { - byte nextByte = sao.bytes[sao.pos++]; - if (newLine) { - // Check the delimiter - if (nextByte == delimiter.codePointAt(index)) { - index++; - if (delimiter.length() == index) { - found = true; - break; - } - } else { - newLine = false; - index = 0; - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (sao.pos < sao.limit) { - nextByte = sao.bytes[sao.pos++]; - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 2; - } else { - // Unread last nextByte - sao.pos--; - lastrealpos = sao.pos; - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 1; - } else { - lastrealpos = sao.pos; - } - } - } else { - // continue until end of line - if (nextByte == HttpConstants.CR) { - if (sao.pos < sao.limit) { - nextByte = sao.bytes[sao.pos++]; - if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 2; - } else { - // Unread last nextByte - sao.pos--; - lastrealpos = sao.pos; - } - } - } else if (nextByte == HttpConstants.LF) { - newLine = true; - index = 0; - lastrealpos = sao.pos - 1; - } else { - lastrealpos = sao.pos; - } - } - } - lastPosition = sao.getReadPosition(lastrealpos); - if (found) { - // found so lastPosition is correct - // but position is just after the delimiter (either close - // delimiter or simple one) - // so go back of delimiter size - try { - currentAttribute.addContent( - undecodedChunk.copy(readerIndex, lastPosition - readerIndex), true); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - undecodedChunk.readerIndex(lastPosition); - } else { - try { - currentAttribute.addContent( - undecodedChunk.copy(readerIndex, lastPosition - readerIndex), false); - } catch (IOException e) { - throw new ErrorDataDecoderException(e); - } - undecodedChunk.readerIndex(lastPosition); - throw new NotEnoughDataDecoderException(); - } - } catch (IndexOutOfBoundsException e) { - undecodedChunk.readerIndex(readerIndex); - throw new NotEnoughDataDecoderException(e); + httpData.addContent(content, delimiterFound); + } catch (IOException e) { + throw new ErrorDataDecoderException(e); } + undecodedChunk.readerIndex(lastPosition); + return delimiterFound; } /** @@ -1714,25 +1363,25 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest * * @return the cleaned String */ - @SuppressWarnings("StatementWithEmptyBody") private static String cleanString(String field) { - StringBuilder sb = new StringBuilder(field.length()); - for (int i = 0; i < field.length(); i++) { + int size = field.length(); + StringBuilder sb = new StringBuilder(size); + for (int i = 0; i < size; i++) { char nextChar = field.charAt(i); - if (nextChar == HttpConstants.COLON) { + switch (nextChar) { + case HttpConstants.COLON: + case HttpConstants.COMMA: + case HttpConstants.EQUALS: + case HttpConstants.SEMICOLON: + case HttpConstants.HT: sb.append(HttpConstants.SP_CHAR); - } else if (nextChar == HttpConstants.COMMA) { - sb.append(HttpConstants.SP_CHAR); - } else if (nextChar == HttpConstants.EQUALS) { - sb.append(HttpConstants.SP_CHAR); - } else if (nextChar == HttpConstants.SEMICOLON) { - sb.append(HttpConstants.SP_CHAR); - } else if (nextChar == HttpConstants.HT) { - sb.append(HttpConstants.SP_CHAR); - } else if (nextChar == HttpConstants.DOUBLE_QUOTE) { + break; + case HttpConstants.DOUBLE_QUOTE: // nothing added, just removes it - } else { + break; + default: sb.append(nextChar); + break; } } return sb.toString().trim();