Permalink
Browse files

Extends HttpPostRequestEncoder to support all methods except TRACE

Motivation:

In Netty, currently, the HttpPostRequestEncoder only supports POST, PUT, PATCH and OPTIONS, while the RFC 7231 allows with a warning that GET, HEAD, DELETE and CONNECT use a body too (but not TRACE where it is explicitely not allowed).
The RFC in chapter 4.3 says:
"A payload within a XXX request message has no defined semantics;
sending a payload body on a XXX request might cause some existing
implementations to reject the request."
where XXX can be replaced by one of GET, HEAD, DELETE or CONNECT.

Current usages, on particular in REST mode, tend to use those extra HttpMethods for such queries.

So this PR proposes to remove the current restrictions, leaving only TRACE as explicitely not supported.

Modification:

In the constructor, where the test is done, replacing all by checking only against TRACE, and adding one test to check that all methods are supported or not.

Result:

Fixes #6138.
  • Loading branch information...
1 parent 0eeeb76 commit 56ddc47f23611f6e1c8009caf0b67f535a41802c Frederic BREGIER committed with Scottmitch Dec 22, 2016
@@ -53,6 +53,15 @@
/**
* This encoder will help to encode Request for a FORM as POST.
+ *
+ * <P>According to RFC 7231, POST, PUT and OPTIONS allow to have a body.
+ * This encoder will support widely all methods except TRACE since the RFC notes
+ * for GET, DELETE, HEAD and CONNECT: (replaces XXX by one of these methods)</P>
+ * <P>"A payload within a XXX request message has no defined semantics;
+ * sending a payload body on a XXX request might cause some existing
+ * implementations to reject the request."</P>
+ * <P>On the contrary, for TRACE method, RFC says:</P>
+ * <P>"A client MUST NOT send a message body in a TRACE request."</P>
*/
public class HttpPostRequestEncoder implements ChunkedInput<HttpContent> {
@@ -150,7 +159,7 @@
* @throws NullPointerException
* for request
* @throws ErrorDataEncoderException
- * if the request is not a POST
+ * if the request is a TRACE
*/
public HttpPostRequestEncoder(HttpRequest request, boolean multipart) throws ErrorDataEncoderException {
this(new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE), request, multipart,
@@ -168,7 +177,7 @@ public HttpPostRequestEncoder(HttpRequest request, boolean multipart) throws Err
* @throws NullPointerException
* for request and factory
* @throws ErrorDataEncoderException
- * if the request is not a POST
+ * if the request is a TRACE
*/
public HttpPostRequestEncoder(HttpDataFactory factory, HttpRequest request, boolean multipart)
throws ErrorDataEncoderException {
@@ -190,7 +199,7 @@ public HttpPostRequestEncoder(HttpDataFactory factory, HttpRequest request, bool
* @throws NullPointerException
* for request or charset or factory
* @throws ErrorDataEncoderException
- * if the request is not a POST
+ * if the request is a TRACE
*/
public HttpPostRequestEncoder(
HttpDataFactory factory, HttpRequest request, boolean multipart, Charset charset,
@@ -206,9 +215,8 @@ public HttpPostRequestEncoder(
throw new NullPointerException("charset");
}
HttpMethod method = request.method();
- if (!(method.equals(HttpMethod.POST) || method.equals(HttpMethod.PUT)
- || method.equals(HttpMethod.PATCH) || method.equals(HttpMethod.OPTIONS))) {
- throw new ErrorDataEncoderException("Cannot create a Encoder if not a POST");
+ if (method.equals(HttpMethod.TRACE)) {
+ throw new ErrorDataEncoderException("Cannot create a Encoder if request is a TRACE");
}
this.request = request;
this.charset = charset;
@@ -20,10 +20,9 @@
import java.util.List;
/**
- * This decoder will decode Body and can handle POST BODY (or for PUT, PATCH or OPTIONS).
+ * This decoder will decode Body and can handle POST BODY.
*
* You <strong>MUST</strong> call {@link #destroy()} after completion to release all resources.
- *
*/
public interface InterfaceHttpPostRequestDecoder {
/**
@@ -23,6 +23,7 @@
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.multipart.HttpPostRequestEncoder.EncoderMode;
+import io.netty.handler.codec.http.multipart.HttpPostRequestEncoder.ErrorDataEncoderException;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.StringUtil;
import org.junit.Test;
@@ -37,14 +38,32 @@
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/** {@link HttpPostRequestEncoder} test case. */
public class HttpPostRequestEncoderTest {
@Test
- public void testSingleFileUpload() throws Exception {
+ public void testAllowedMethods() throws Exception {
+ shouldThrowExceptionIfNotAllowed(HttpMethod.CONNECT);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.PUT);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.POST);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.PATCH);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.DELETE);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.GET);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.HEAD);
+ shouldThrowExceptionIfNotAllowed(HttpMethod.OPTIONS);
+ try {
+ shouldThrowExceptionIfNotAllowed(HttpMethod.TRACE);
+ fail("Should raised an exception with TRACE method");
+ } catch (ErrorDataEncoderException e) {
+ // Exception is willing
+ }
+ }
+
+ private void shouldThrowExceptionIfNotAllowed(HttpMethod method) throws Exception {
DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
- HttpMethod.POST, "http://localhost");
+ method, "http://localhost");
HttpPostRequestEncoder encoder = new HttpPostRequestEncoder(request, true);
File file1 = new File(getClass().getResource("/file-01.txt").toURI());

0 comments on commit 56ddc47

Please sign in to comment.