From bbbcb9a7067785b25f75fd6719b408b833dec9fb Mon Sep 17 00:00:00 2001 From: Frederic Bregier Date: Thu, 16 Oct 2014 00:15:20 +0200 Subject: [PATCH] V3.9 - 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 maxParm 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. --- pom.xml | 3 +- .../HttpPostMultipartRequestDecoder.java | 10 ++-- .../multipart/HttpPostRequestDecoder.java | 18 +++--- .../jboss/netty/util/internal/StringUtil.java | 57 +++++++++++++++++++ .../netty/util/internal/StringUtilTest.java | 11 ++++ 5 files changed, 83 insertions(+), 16 deletions(-) diff --git a/pom.xml b/pom.xml index 331c11f5cb..e6e5bda81e 100644 --- a/pom.xml +++ b/pom.xml @@ -75,8 +75,7 @@ ${project.groupId} netty-tcnative 1.1.30.Fork2 - ${os.detected.classifier} - compile + windows-x86_64 true diff --git a/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java b/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java index 603e7cbad4..a3a37a99e7 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java @@ -540,7 +540,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]); @@ -596,8 +596,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 { @@ -608,12 +608,12 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest 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/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java b/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java index 4924ecc8b5..af862cf6c0 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java @@ -179,25 +179,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.toString())) { - 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/src/main/java/org/jboss/netty/util/internal/StringUtil.java b/src/main/java/org/jboss/netty/util/internal/StringUtil.java index e20e608043..b778a60cff 100644 --- a/src/main/java/org/jboss/netty/util/internal/StringUtil.java +++ b/src/main/java/org/jboss/netty/util/internal/StringUtil.java @@ -159,4 +159,61 @@ 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).length() == 0) { + 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; + } } diff --git a/src/test/java/org/jboss/netty/util/internal/StringUtilTest.java b/src/test/java/org/jboss/netty/util/internal/StringUtilTest.java index 9530a7bf1d..7fea3e56d3 100644 --- a/src/test/java/org/jboss/netty/util/internal/StringUtilTest.java +++ b/src/test/java/org/jboss/netty/util/internal/StringUtilTest.java @@ -104,4 +104,15 @@ public class StringUtilTest { public void splitWithDelimiterAtBeginning() { assertArrayEquals(new String[] { "", "foo", "bar" }, StringUtil.split("#foo#bar", '#')); } + + @Test + public void splitMaxPart() { + assertArrayEquals(new String[] { "foo", "bar:bar2" }, StringUtil.split("foo:bar:bar2", ':', 2)); + assertArrayEquals(new String[] { "foo", "bar", "bar2" }, StringUtil.split("foo:bar:bar2", ':', 3)); + } + + @Test + public void substringAfterTest() { + assertEquals("bar:bar2", StringUtil.substringAfter("foo:bar:bar2", ':')); + } }