From 4c11ce7241f3f768955a20d2c1c8fbc41d8fd651 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 27 Apr 2021 16:37:37 +0200 Subject: [PATCH] Correctly throw ErrorDataDecoderException for errors while decoding (#11198) Motivation: We didn't correctly handle the case when no content-type header was found or if the charset was illegal and just did throw a NPE or ICE. We should in both cases throw an ErrorDataDecoderException to reflect what is documented in the javadocs. Modifications: - Throw correct exception - Merge private method into the constructor as it is only used there - Add unit tests Result: Throw expected exceptions on decoding errors --- .../HttpPostMultipartRequestDecoder.java | 38 ++++++------ .../HttpPostMultiPartRequestDecoderTest.java | 58 +++++++++++++++++++ 2 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java 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 43ba3444a5..a6b98552e3 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.IllegalCharsetNameException; import java.nio.charset.UnsupportedCharsetException; import java.util.ArrayList; import java.util.List; @@ -183,7 +184,26 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest this.factory = requireNonNull(factory, "factory"); // Fill default values - setMultipart(this.request.headers().get(HttpHeaderNames.CONTENT_TYPE)); + String contentTypeValue = this.request.headers().get(HttpHeaderNames.CONTENT_TYPE); + if (contentTypeValue == null) { + throw new ErrorDataDecoderException("No '" + HttpHeaderNames.CONTENT_TYPE + "' header present."); + } + + String[] dataBoundary = HttpPostRequestDecoder.getMultipartDataBoundary(contentTypeValue); + if (dataBoundary != null) { + multipartDataBoundary = dataBoundary[0]; + if (dataBoundary.length > 1 && dataBoundary[1] != null) { + try { + this.charset = Charset.forName(dataBoundary[1]); + } catch (IllegalCharsetNameException e) { + throw new ErrorDataDecoderException(e); + } + } + } else { + multipartDataBoundary = null; + } + currentStatus = MultiPartStatus.HEADERDELIMITER; + if (request instanceof HttpContent) { // Offer automatically if the given request is als type of HttpContent // See #1089 @@ -193,22 +213,6 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest } } - /** - * Set from the request ContentType the multipartDataBoundary and the possible charset. - */ - private void setMultipart(String contentType) { - String[] dataBoundary = HttpPostRequestDecoder.getMultipartDataBoundary(contentType); - if (dataBoundary != null) { - multipartDataBoundary = dataBoundary[0]; - if (dataBoundary.length > 1 && dataBoundary[1] != null) { - charset = Charset.forName(dataBoundary[1]); - } - } else { - multipartDataBoundary = null; - } - currentStatus = MultiPartStatus.HEADERDELIMITER; - } - private void checkDestroyed() { if (destroyed) { throw new IllegalStateException(HttpPostMultipartRequestDecoder.class.getSimpleName() diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java new file mode 100644 index 0000000000..696c2cb20a --- /dev/null +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2021 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http.multipart; + +import io.netty.handler.codec.http.DefaultFullHttpRequest; +import io.netty.handler.codec.http.FullHttpRequest; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpVersion; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class HttpPostMultiPartRequestDecoderTest { + + @Test + public void testDecodeFullHttpRequestWithNoContentTypeHeader() { + FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/"); + try { + new HttpPostMultipartRequestDecoder(req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException expected) { + // expected + } finally { + assertTrue(req.release()); + } + } + + @Test + public void testDecodeFullHttpRequestWithInvalidCharset() { + FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/"); + req.headers().set(HttpHeaderNames.CONTENT_TYPE, + "multipart/form-data; boundary=--89421926422648 [; charset=UTF-8]"); + + try { + new HttpPostMultipartRequestDecoder(req); + fail("Was expecting an ErrorDataDecoderException"); + } catch (HttpPostRequestDecoder.ErrorDataDecoderException expected) { + // expected + } finally { + assertTrue(req.release()); + } + } +}