Repository: netty/netty
Issue: HttpPostMultipartRequestDecoder.splitMultipartHeader() String index out of range: -1 with empty Content-Type

Parsing a multi-part request with an empty content type header: e.g. "Content-Type: "
will produce a java.lang.StringIndexOutOfBoundsException: String index out of range: -1

```
java.lang.StringIndexOutOfBoundsException: String index out of range: -1
 at java.lang.String.substring(String.java:1967)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.splitMultipartHeader(HttpPostMultipartRequestDecoder.java:1470)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.findMultipartDisposition(HttpPostMultipartRequestDecoder.java:680)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.decodeMultipart(HttpPostMultipartRequestDecoder.java:490)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.findMultipartDelimiter(HttpPostMultipartRequestDecoder.java:642)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.decodeMultipart(HttpPostMultipartRequestDecoder.java:477)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.parseBodyMultipart(HttpPostMultipartRequestDecoder.java:448)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.parseBody(HttpPostMultipartRequestDecoder.java:411)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.offer(HttpPostMultipartRequestDecoder.java:336)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.<init>(HttpPostMultipartRequestDecoder.java:185)
 at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.<init>(HttpPostMultipartRequestDecoder.java:142)
```

Issue comments:
```
private static String[] splitMultipartHeader(String sb) {
        ...
        valueStart = HttpPostBodyUtil.findNonWhitespace(sb, colonEnd);
        valueEnd = HttpPostBodyUtil.findEndOfString(sb);
        headers.add(sb.substring(nameStart, nameEnd));

        String svalue = sb.substring(valueStart, valueEnd); // <--- StringIndexOutOfBoundsException
        ...
```

```
splitMultipartHeader("Content-Type:") -> valueStart=13 valueEnd=13 (OK returns [Content-Type, ])
splitMultipartHeader("Content-Type: ") -> valueStart=14 valueEnd=13
splitMultipartHeader("Content-Type: \r\n") -> valueStart=16 valueEnd=13
```
Maybe you can provide a unit test and fix ?

2018-01-24 18:54 GMT+01:00 Matteo Bertozzi <notifications@github.com>:

> private static String[] splitMultipartHeader(String sb) {
>         ...
>         valueStart = HttpPostBodyUtil.findNonWhitespace(sb, colonEnd);
>         valueEnd = HttpPostBodyUtil.findEndOfString(sb);
>         headers.add(sb.substring(nameStart, nameEnd));
>
>         String svalue = sb.substring(valueStart, valueEnd); // <--- StringIndexOutOfBoundsException
>         ...
>
> splitMultipartHeader("Content-Type:") -> valueStart=13 valueEnd=13
> splitMultipartHeader("Content-Type: ") -> valueStart=14 valueEnd=13
> splitMultipartHeader("Content-Type: \r\n") -> valueStart=16 valueEnd=13
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/netty/netty/issues/7620#issuecomment-360218651>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAa0Qn1wYrWco54QVlb73vmnkCjaPl2yks5tN25igaJpZM4Rri_n>
> .
>

https://gist.github.com/matteobertozzi/86e1b7d5f4205beee243897050a47069
@matteobertozzi can you open a PR against 4.1 ?
Fixed by https://github.com/netty/netty/pull/7622

Pull request review comments:
@matteobertozzi should we just make this `StringUtil.EMPTY_STRING` when `valueStart >= valueEnd` ?

So something like this:

```
String svalue == valueStart >= valueEnd ? StringUtil.EMPTY_STRING : sb.substring(valueStart, valueEnd))
```
WDYT ?
yeah, that's better
@matteobertozzi please adjust and push.

Patch that resolved the issue:
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 44b2641c6a4..8e3a90c009b 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
@@ -1467,7 +1467,7 @@ private static String[] splitMultipartHeader(String sb) {
         valueStart = HttpPostBodyUtil.findNonWhitespace(sb, colonEnd);
         valueEnd = HttpPostBodyUtil.findEndOfString(sb);
         headers.add(sb.substring(nameStart, nameEnd));
-        String svalue = sb.substring(valueStart, valueEnd);
+        String svalue = (valueStart >= valueEnd) ? StringUtil.EMPTY_STRING : sb.substring(valueStart, valueEnd);
         String[] values;
         if (svalue.indexOf(';') >= 0) {
             values = splitMultipartHeaderValues(svalue);


Test patch:
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 31e786a38c3..f8b756ca70a 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
@@ -626,4 +626,34 @@ public void testDecodeMalformedBadCharsetContentDispositionFieldParameters() thr
             req.release();
         }
     }
+
+    // https://github.com/netty/netty/issues/7620
+    @Test
+    public void testDecodeMalformedEmptyContentTypeFieldParameters() throws Exception {
+        final String boundary = "dLV9Wyq26L_-JQxk6ferf-RT153LhOO";
+        final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST,
+                                                                      "http://localhost");
+        req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
+        // Force to use memory-based data.
+        final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
+        final String data = "asdf";
+        final String filename = "tmp-0.txt";
+        final String body =
+                "--" + boundary + "\r\n" +
+                "Content-Disposition: form-data; name=\"file\"; filename=\"" + filename + "\"\r\n" +
+                "Content-Type: \r\n" +
+                "\r\n" +
+                data + "\r\n" +
+                "--" + boundary + "--\r\n";
+
+        req.content().writeBytes(body.getBytes(CharsetUtil.UTF_8.name()));
+        // Create decoder instance to test.
+        final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req);
+        assertFalse(decoder.getBodyHttpDatas().isEmpty());
+        InterfaceHttpData part1 = decoder.getBodyHttpDatas().get(0);
+        assertTrue(part1 instanceof FileUpload);
+        FileUpload fileUpload = (FileUpload) part1;
+        assertEquals("tmp-0.txt", fileUpload.getFilename());
+        decoder.destroy();
+    }
 }