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
This commit is contained in:
parent
268f5a9108
commit
3657805711
@ -35,6 +35,7 @@ import io.netty.util.internal.StringUtil;
|
|||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.nio.charset.Charset;
|
import java.nio.charset.Charset;
|
||||||
|
import java.nio.charset.IllegalCharsetNameException;
|
||||||
import java.nio.charset.UnsupportedCharsetException;
|
import java.nio.charset.UnsupportedCharsetException;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@ -178,7 +179,26 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
|||||||
this.factory = checkNotNull(factory, "factory");
|
this.factory = checkNotNull(factory, "factory");
|
||||||
// Fill default values
|
// 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) {
|
if (request instanceof HttpContent) {
|
||||||
// Offer automatically if the given request is als type of HttpContent
|
// Offer automatically if the given request is als type of HttpContent
|
||||||
// See #1089
|
// See #1089
|
||||||
@ -188,22 +208,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() {
|
private void checkDestroyed() {
|
||||||
if (destroyed) {
|
if (destroyed) {
|
||||||
throw new IllegalStateException(HttpPostMultipartRequestDecoder.class.getSimpleName()
|
throw new IllegalStateException(HttpPostMultipartRequestDecoder.class.getSimpleName()
|
||||||
|
@ -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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user