From eb415fded66b5c825aa2818f6223eb8a6480c024 Mon Sep 17 00:00:00 2001 From: Frederic Bregier Date: Wed, 15 Oct 2014 23:47:27 +0200 Subject: [PATCH] V4.1 Fix "=" character in HttpPostRequestDecoder Motivation Issue #3004 shows that "=" character was not supported as it should in the HttpPostRequestDecoder in form-data boundary. Modifications: Add 2 methods in StringUtil - split with maxPart argument: String split with max parts only (to prevent multiple '=' to be source of extra split while not needed) - substringAfter: String part after delimiter (since first part is not needed) Use those methods in HttpPostRequestDecoder. Change and the HttpPostRequestDecoderTest to check using a boundary beginning with "=". Results: The fix implies more stability and fix the issue. --- .../HttpPostMultipartRequestDecoder.java | 10 ++-- .../multipart/HttpPostRequestDecoder.java | 18 +++--- .../io/netty/util/internal/StringUtil.java | 57 +++++++++++++++++++ .../netty/util/internal/StringUtilTest.java | 11 ++++ 4 files changed, 82 insertions(+), 14 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 10481919bc..2b15bd1bcc 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 @@ -672,7 +672,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest if (checkSecondArg) { // read next values and store them in the map as Attribute for (int i = 2; i < contents.length; i++) { - String[] values = StringUtil.split(contents[i], '='); + String[] values = StringUtil.split(contents[i], '=', 2); Attribute attribute; try { String name = cleanString(values[0]); @@ -721,8 +721,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest // Take care of possible "multipart/mixed" if (contents[1].equalsIgnoreCase(HttpPostBodyUtil.MULTIPART_MIXED)) { if (currentStatus == MultiPartStatus.DISPOSITION) { - String[] values = StringUtil.split(contents[2], '='); - multipartMixedBoundary = "--" + values[1]; + String values = StringUtil.substringAfter(contents[2], '='); + multipartMixedBoundary = "--" + values; currentStatus = MultiPartStatus.MIXEDDELIMITER; return decodeMultipart(MultiPartStatus.MIXEDDELIMITER); } else { @@ -731,11 +731,11 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } else { for (int i = 1; i < contents.length; i++) { if (contents[i].toLowerCase().startsWith(HttpHeaders.Values.CHARSET)) { - String[] values = StringUtil.split(contents[i], '='); + String values = StringUtil.substringAfter(contents[i], '='); Attribute attribute; try { attribute = factory.createAttribute(request, HttpHeaders.Values.CHARSET, - cleanString(values[1])); + cleanString(values)); } catch (NullPointerException e) { throw new ErrorDataDecoderException(e); } catch (IllegalArgumentException e) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java index d0e66de839..fbaeecf155 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java @@ -169,25 +169,25 @@ public class HttpPostRequestDecoder implements InterfaceHttpPostRequestDecoder { } else { return null; } - String[] boundary = StringUtil.split(headerContentType[mrank], '='); - if (boundary.length != 2) { + String boundary = StringUtil.substringAfter(headerContentType[mrank], '='); + if (boundary == null) { throw new ErrorDataDecoderException("Needs a boundary value"); } - if (boundary[1].charAt(0) == '"') { - String bound = boundary[1].trim(); + if (boundary.charAt(0) == '"') { + String bound = boundary.trim(); int index = bound.length() - 1; if (bound.charAt(index) == '"') { - boundary[1] = bound.substring(1, index); + boundary = bound.substring(1, index); } } if (headerContentType[crank].toLowerCase().startsWith( HttpHeaders.Values.CHARSET)) { - String[] charset = StringUtil.split(headerContentType[crank], '='); - if (charset.length > 1) { - return new String[] {"--" + boundary[1], charset[1]}; + String charset = StringUtil.substringAfter(headerContentType[crank], '='); + if (charset != null) { + return new String[] {"--" + boundary, charset}; } } - return new String[] {"--" + boundary[1]}; + return new String[] {"--" + boundary}; } return null; } diff --git a/common/src/main/java/io/netty/util/internal/StringUtil.java b/common/src/main/java/io/netty/util/internal/StringUtil.java index 69d3bfa7db..09b827ef38 100644 --- a/common/src/main/java/io/netty/util/internal/StringUtil.java +++ b/common/src/main/java/io/netty/util/internal/StringUtil.java @@ -111,6 +111,63 @@ public final class StringUtil { return res.toArray(new String[res.size()]); } + /** + * Splits the specified {@link String} with the specified delimiter in maxParts maximum parts. + * This operation is a simplified and optimized + * version of {@link String#split(String, int)}. + */ + public static String[] split(String value, char delim, int maxParts) { + final int end = value.length(); + final List res = new ArrayList(); + + int start = 0; + int cpt = 1; + for (int i = 0; i < end && cpt < maxParts; i ++) { + if (value.charAt(i) == delim) { + if (start == i) { + res.add(EMPTY_STRING); + } else { + res.add(value.substring(start, i)); + } + start = i + 1; + cpt++; + } + } + + if (start == 0) { // If no delimiter was found in the value + res.add(value); + } else { + if (start != end) { + // Add the last element if it's not empty. + res.add(value.substring(start, end)); + } else { + // Truncate trailing empty elements. + for (int i = res.size() - 1; i >= 0; i --) { + if (res.get(i).isEmpty()) { + res.remove(i); + } else { + break; + } + } + } + } + + return res.toArray(new String[res.size()]); + } + + /** + * Get the item after one char delim if the delim is found (else null). + * This operation is a simplified and optimized + * version of {@link String#split(String, int)}. + */ + public static String substringAfter(String value, char delim) { + int pos = value.indexOf(delim); + if (pos >= 0) { + return value.substring(pos + 1); + } + return null; + } + /** * Converts the specified byte value into a 2-digit hexadecimal integer. */ diff --git a/common/src/test/java/io/netty/util/internal/StringUtilTest.java b/common/src/test/java/io/netty/util/internal/StringUtilTest.java index 5f59849a9e..036f5b1c29 100644 --- a/common/src/test/java/io/netty/util/internal/StringUtilTest.java +++ b/common/src/test/java/io/netty/util/internal/StringUtilTest.java @@ -70,4 +70,15 @@ public class StringUtilTest { public void splitWithDelimiterAtBeginning() { assertArrayEquals(new String[] { "", "foo", "bar" }, split("#foo#bar", '#')); } + + @Test + public void splitMaxPart() { + assertArrayEquals(new String[] { "foo", "bar:bar2" }, split("foo:bar:bar2", ':', 2)); + assertArrayEquals(new String[] { "foo", "bar", "bar2" }, split("foo:bar:bar2", ':', 3)); + } + + @Test + public void substringAfterTest() { + assertEquals("bar:bar2", substringAfter("foo:bar:bar2", ':')); + } }