From 8aeba78ecc9c9c3806ca729120c57e521f3d0501 Mon Sep 17 00:00:00 2001 From: Dmitry Minkovsky Date: Tue, 24 Oct 2017 19:28:07 +0200 Subject: [PATCH] HttpPostMultipartRequestDecoder should decode header field parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: I am receiving a multipart/form_data upload from a Mailgun webhook. This webhook used to send parts like this: --74e78d11b0214bdcbc2f86491eeb4902 Content-Disposition: form-data; name="attachment-2"; filename="attached_Ñ�айл.txt" Content-Type: text/plain Content-Length: 32 This is the content of the file --74e78d11b0214bdcbc2f86491eeb4902-- but now it posts parts like this: --74e78d11b0214bdcbc2f86491eeb4902 Content-Disposition: form-data; name="attachment-2"; filename*=utf-8''attached_%D1%84%D0%B0%D0%B9%D0%BB.txt This is the content of the file --74e78d11b0214bdcbc2f86491eeb4902-- This new format uses field parameter encoding described in RFC 5987. More about this encoding can be found here. Netty does not parse this format. The result is the filename is not decoded and the part is not parsed into a FileUpload. Modification: Added failing test in HttpPostRequestDecoderTest.java and updated HttpPostMultipartRequestDecoder.java Refactored to please Netkins Result: Fixes: HttpPostMultipartRequestDecoder identifies the RFC 5987 format and parses it. Previous functionality is retained. --- .../HttpPostMultipartRequestDecoder.java | 46 ++++-- .../multipart/HttpPostRequestDecoderTest.java | 135 ++++++++++++++++++ 2 files changed, 169 insertions(+), 12 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 e60b93b7a3..44b2641c6a 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 @@ -22,6 +22,7 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaderValues; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.LastHttpContent; +import io.netty.handler.codec.http.QueryStringDecoder; import io.netty.handler.codec.http.multipart.HttpPostBodyUtil.SeekAheadOptimize; import io.netty.handler.codec.http.multipart.HttpPostBodyUtil.TransferEncodingMechanism; import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.EndOfDataDecoderException; @@ -691,18 +692,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest String[] values = contents[i].split("=", 2); Attribute attribute; try { - String name = cleanString(values[0]); - String value = values[1]; - - // See http://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html - if (HttpHeaderValues.FILENAME.contentEquals(name)) { - // filename value is quoted string so strip them - value = value.substring(1, value.length() - 1); - } else { - // otherwise we need to clean the value - value = cleanString(value); - } - attribute = factory.createAttribute(request, name, value); + attribute = getContentDispositionAttribute(values); } catch (NullPointerException e) { throw new ErrorDataDecoderException(e); } catch (IllegalArgumentException e) { @@ -805,6 +795,38 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } } + private static final String FILENAME_ENCODED = HttpHeaderValues.FILENAME.toString() + '*'; + + private Attribute getContentDispositionAttribute(String... values) { + String name = cleanString(values[0]); + String value = values[1]; + + // Filename can be token, quoted or encoded. See https://tools.ietf.org/html/rfc5987 + if (HttpHeaderValues.FILENAME.contentEquals(name)) { + // Value is quoted or token. Strip if quoted: + int last = value.length() - 1; + if (last > 0 && + value.charAt(0) == HttpConstants.DOUBLE_QUOTE && + value.charAt(last) == HttpConstants.DOUBLE_QUOTE) { + value = value.substring(1, last); + } + } else if (FILENAME_ENCODED.equals(name)) { + try { + name = HttpHeaderValues.FILENAME.toString(); + String[] split = value.split("'", 3); + value = QueryStringDecoder.decodeComponent(split[2], Charset.forName(split[0])); + } catch (ArrayIndexOutOfBoundsException e) { + throw new ErrorDataDecoderException(e); + } catch (UnsupportedCharsetException e) { + throw new ErrorDataDecoderException(e); + } + } else { + // otherwise we need to clean the value + value = cleanString(value); + } + return factory.createAttribute(request, name, value); + } + /** * Get the FileUpload (new one or current one) * 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 13341074ca..31e786a38c 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 @@ -33,6 +33,7 @@ import io.netty.handler.codec.http.LastHttpContent; import io.netty.util.CharsetUtil; import org.junit.Test; +import java.net.URLEncoder; import java.nio.charset.UnsupportedCharsetException; import java.util.Arrays; @@ -491,4 +492,138 @@ public class HttpPostRequestDecoderTest { content.release(); } } + + // https://github.com/netty/netty/pull/7265 + @Test + public void testDecodeContentDispositionFieldParameters() throws Exception { + + final String boundary = "74e78d11b0214bdcbc2f86491eeb4902"; + + String encoding = "utf-8"; + String filename = "attached_файл.txt"; + String filenameEncoded = URLEncoder.encode(filename, encoding); + + final String body = "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename*=" + encoding + "''" + filenameEncoded + "\r\n" + + "\r\n" + + "foo\r\n" + + "\r\n" + + "--" + boundary + "--"; + + final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.POST, + "http://localhost", + Unpooled.wrappedBuffer(body.getBytes())); + + req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary); + final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false); + final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); + assertFalse(decoder.getBodyHttpDatas().isEmpty()); + InterfaceHttpData part1 = decoder.getBodyHttpDatas().get(0); + 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(); + } + + // https://github.com/netty/netty/pull/7265 + @Test + public void testDecodeWithLanguageContentDispositionFieldParameters() throws Exception { + + final String boundary = "74e78d11b0214bdcbc2f86491eeb4902"; + + String encoding = "utf-8"; + String filename = "attached_файл.txt"; + String language = "anything"; + String filenameEncoded = URLEncoder.encode(filename, encoding); + + final String body = "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename*=" + + encoding + "'" + language + "'" + filenameEncoded + "\r\n" + + "\r\n" + + "foo\r\n" + + "\r\n" + + "--" + boundary + "--"; + + final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.POST, + "http://localhost", + Unpooled.wrappedBuffer(body.getBytes())); + + req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary); + final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false); + final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); + assertFalse(decoder.getBodyHttpDatas().isEmpty()); + InterfaceHttpData part1 = decoder.getBodyHttpDatas().get(0); + 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(); + } + + // https://github.com/netty/netty/pull/7265 + @Test + public void testDecodeMalformedNotEncodedContentDispositionFieldParameters() throws Exception { + + final String boundary = "74e78d11b0214bdcbc2f86491eeb4902"; + + final String body = "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename*=not-encoded\r\n" + + "\r\n" + + "foo\r\n" + + "\r\n" + + "--" + boundary + "--"; + + final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.POST, + "http://localhost", + Unpooled.wrappedBuffer(body.getBytes())); + + req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary); + + final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false); + + try { + new HttpPostRequestDecoder(inMemoryFactory, req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { + assertTrue(e.getCause() instanceof ArrayIndexOutOfBoundsException); + } finally { + req.release(); + } + } + + // https://github.com/netty/netty/pull/7265 + @Test + public void testDecodeMalformedBadCharsetContentDispositionFieldParameters() throws Exception { + + final String boundary = "74e78d11b0214bdcbc2f86491eeb4902"; + + final String body = "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename*=not-a-charset''filename\r\n" + + "\r\n" + + "foo\r\n" + + "\r\n" + + "--" + boundary + "--"; + + final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.POST, + "http://localhost", + Unpooled.wrappedBuffer(body.getBytes())); + + req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary); + + final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false); + + try { + new HttpPostRequestDecoder(inMemoryFactory, req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { + assertTrue(e.getCause() instanceof UnsupportedCharsetException); + } finally { + req.release(); + } + } }