HttpMessages implementing HttpContent types too aren't handled correc…

…tly (Fixes netty#12750) (netty#12751) Motivation: Changes due to netty#12709 have added a code path. Modifications: Restore the "just http message" case as separated by the http content ones Result: Same encoding as original code Co-authored-by: Chris Vest <[email protected]>
ible · Sep 1, 2022 · 9f10a28 · 9f10a28
1 parent 05943f5
commit 9f10a28
Show file tree

Hide file tree

Showing 2 changed files with 132 additions and 1 deletion.
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java
@@ -174,8 +174,9 @@ protected void encode(ChannelHandlerContext ctx, Object msg, List<Object> out) t
                 encodeHttpMessageLastContent(ctx, m, out);
             } else if (m instanceof HttpContent) {
                 encodeHttpMessageNotLastContent(ctx, m, out);
+            } else {
+                encodeJustHttpMessage(ctx, m, out);
             }
-            encodeJustHttpMessage(ctx, m, out);
         } else {
             encodeNotHttpMessageContentTypes(ctx, msg, out);
         }

diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java
@@ -29,11 +29,13 @@
 import java.nio.charset.Charset;
 import java.util.concurrent.ExecutionException;
 
+import static io.netty.util.internal.ObjectUtil.checkNotNull;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -182,6 +184,134 @@ public void testEmptyContentNotsChunkedWithTrailers() throws Exception {
         testEmptyContents(false, true);
     }
 
+    // this is not using Full types on purpose!!!
+    private static class CustomFullHttpRequest extends DefaultHttpRequest implements LastHttpContent {
+        private final ByteBuf content;
+        private final HttpHeaders trailingHeader;
+
+        CustomFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri, ByteBuf content) {
+            this(httpVersion, method, uri, content, true);
+        }
+
+        CustomFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri,
+                                     ByteBuf content, boolean validateHeaders) {
+            super(httpVersion, method, uri, validateHeaders);
+            this.content = checkNotNull(content, "content");
+            trailingHeader = new DefaultHttpHeaders(validateHeaders);
+        }
+
+        private CustomFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri,
+                                     ByteBuf content, HttpHeaders headers, HttpHeaders trailingHeader) {
+            super(httpVersion, method, uri, headers);
+            this.content = checkNotNull(content, "content");
+            this.trailingHeader = checkNotNull(trailingHeader, "trailingHeader");
+        }
+
+        @Override
+        public HttpHeaders trailingHeaders() {
+            return trailingHeader;
+        }
+
+        @Override
+        public ByteBuf content() {
+            return content;
+        }
+
+        @Override
+        public int refCnt() {
+            return content.refCnt();
+        }
+
+        @Override
+        public CustomFullHttpRequest retain() {
+            content.retain();
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest retain(int increment) {
+            content.retain(increment);
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest touch() {
+            content.touch();
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest touch(Object hint) {
+            content.touch(hint);
+            return this;
+        }
+
+        @Override
+        public boolean release() {
+            return content.release();
+        }
+
+        @Override
+        public boolean release(int decrement) {
+            return content.release(decrement);
+        }
+
+        @Override
+        public CustomFullHttpRequest setProtocolVersion(HttpVersion version) {
+            super.setProtocolVersion(version);
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest setMethod(HttpMethod method) {
+            super.setMethod(method);
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest setUri(String uri) {
+            super.setUri(uri);
+            return this;
+        }
+
+        @Override
+        public CustomFullHttpRequest copy() {
+            return replace(content().copy());
+        }
+
+        @Override
+        public CustomFullHttpRequest duplicate() {
+            return replace(content().duplicate());
+        }
+
+        @Override
+        public CustomFullHttpRequest retainedDuplicate() {
+            return replace(content().retainedDuplicate());
+        }
+
+        @Override
+        public CustomFullHttpRequest replace(ByteBuf content) {
+            CustomFullHttpRequest request = new CustomFullHttpRequest(protocolVersion(), method(), uri(), content,
+                    headers().copy(), trailingHeaders().copy());
+            request.setDecoderResult(decoderResult());
+            return request;
+        }
+    }
+
+    @Test
+    public void testCustomMessageEmptyLastContent() {
+        HttpRequestEncoder encoder = new HttpRequestEncoder();
+        EmbeddedChannel channel = new EmbeddedChannel(encoder);
+        HttpRequest customMsg = new CustomFullHttpRequest(HttpVersion.HTTP_1_1,
+                HttpMethod.POST, "/", Unpooled.EMPTY_BUFFER);
+        assertTrue(channel.writeOutbound(customMsg));
+        // Ensure we only produce ByteBuf instances.
+        ByteBuf head = channel.readOutbound();
+        assertTrue(head.release());
+        assertNull(channel.readOutbound());
+        assertFalse(channel.finish());
+    }
+
     private void testEmptyContents(boolean chunked, boolean trailers) throws Exception {
         HttpRequestEncoder encoder = new HttpRequestEncoder();
         EmbeddedChannel channel = new EmbeddedChannel(encoder);