From 3d7cec63769b32ad323e86d168a2e1237667ff32 Mon Sep 17 00:00:00 2001 From: Julien Viet Date: Wed, 9 Mar 2016 21:58:40 +0100 Subject: [PATCH] Bug fix for HttpPostMultipartRequestDecoder part decoding with an invalid charset not reported as an ErrorDataDecoderException Motivation: The current HttpPostMultipartRequestDecoder can decode multipart/form-data parts with a Content-Type that specifies a charset. When this charset is invalid the Charset.forName() throws an unchecked UnsupportedCharsetException. This exception is not catched by the decoder. It should actually be rethrown as an ErrorDataDecoderException, because the developer using the API would expect this validation failure to be reported as such. Modifications: Add a catch block for UnsupportedCharsetException and rethrow it as an ErrorDataDecoderException. Result: UnsupportedCharsetException are now rethrown as ErrorDataDecoderException. --- .../HttpPostMultipartRequestDecoder.java | 5 ++ .../multipart/HttpPostRequestDecoderTest.java | 65 +++++++++++++++++++ 2 files changed, 70 insertions(+) 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 88030582c2..bb1b746b25 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 @@ -35,6 +35,7 @@ import io.netty.util.internal.StringUtil; import java.io.IOException; import java.nio.charset.Charset; +import java.nio.charset.UnsupportedCharsetException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -508,6 +509,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest localCharset = Charset.forName(charsetAttribute.getValue()); } catch (IOException e) { throw new ErrorDataDecoderException(e); + } catch (UnsupportedCharsetException e) { + throw new ErrorDataDecoderException(e); } } Attribute nameAttribute = currentFieldAttributes.get(HttpHeaderValues.NAME); @@ -858,6 +861,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest localCharset = Charset.forName(charsetAttribute.getValue()); } catch (IOException e) { throw new ErrorDataDecoderException(e); + } catch (UnsupportedCharsetException e) { + throw new ErrorDataDecoderException(e); } } if (currentFileUpload == null) { 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 b5ccef3994..687745584e 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 @@ -32,6 +32,7 @@ import io.netty.handler.codec.http.LastHttpContent; import io.netty.util.CharsetUtil; import org.junit.Test; +import java.nio.charset.UnsupportedCharsetException; import java.util.Arrays; import static io.netty.util.ReferenceCountUtil.releaseLater; @@ -39,6 +40,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** {@link HttpPostRequestDecoder} test case. */ public class HttpPostRequestDecoderTest { @@ -409,4 +411,67 @@ public class HttpPostRequestDecoderTest { assertFalse(decoder.getBodyHttpDatas().isEmpty()); decoder.destroy(); } + + @Test + public void testMultipartRequestWithFileInvalidCharset() 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: image/gif; charset=ABCD\r\n" + + "\r\n" + + data + "\r\n" + + "--" + boundary + "--\r\n"; + + req.content().writeBytes(body.getBytes(CharsetUtil.UTF_8)); + // Create decoder instance to test. + try { + new HttpPostRequestDecoder(inMemoryFactory, req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { + assertTrue(e.getCause() instanceof UnsupportedCharsetException); + } finally { + req.release(); + } + } + + @Test + public void testMultipartRequestWithFieldInvalidCharset() 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 aData = "some data would be here. the data should be long enough that it " + + "will be longer than the original buffer length of 256 bytes in " + + "the HttpPostRequestDecoder in order to trigger the issue. Some more " + + "data just to be on the safe side."; + final String body = + "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"root\"\r\n" + + "Content-Type: text/plain; charset=ABCD\r\n" + + "\r\n" + + aData + + "\r\n" + + "--" + boundary + "--\r\n"; + + req.content().writeBytes(body.getBytes(CharsetUtil.UTF_8)); + // Create decoder instance to test. + try { + new HttpPostRequestDecoder(inMemoryFactory, req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) { + assertTrue(e.getCause() instanceof UnsupportedCharsetException); + } finally { + req.release(); + } + } }