Repository: netty/netty
Issue: HttpResponseEncoder does not treat status 205 as a content-is-always-empty response

### Expected behavior
RFC 2616 (https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.6) states that responses with HTTP status code 205 should never return a payload: `The response MUST NOT include an entity`.

Additionally, the RFC 7230 section on content-length (https://tools.ietf.org/html/rfc7230#section-3.3.2) does not call out 205 responses as being in the allowed-to-lie-about-content-length category (only HEAD and 204 responses are in that category). Also from that RFC 7230 section, 205 responses are not in the must-never-send-content-length category (only 1xx, 204, and CONNECT with a 2xx are in that category).

So based on that I would expect that when Netty encodes a 205 response it should:
* Strip out any payload (other than the no-data chunk when transfer-encoding is chunked)
* Set content-length to 0 for non-chunked responses

NOTE: The HTTP spec is obviously difficult to follow on these corner cases. Please double check my logic on this issue - I believe my analysis to be correct based on my reading of those RFCs, but I could have interpreted it wrong, or I might be missing something in these RFCs or another spec doc entirely that explains how to handle 205 more directly than what I've cobbled together here.

### Actual behavior
Netty does not currently do any smart filtering for 205 responses like it does for other always-empty responses (204, 304, HEAD, etc). I can send a 205 response with a payload and that payload will be sent over the wire as if it was a 200.

Netty *does* appear to sort of pay attention to this on the decoding side - i.e. HttpRequestDecoder drops the payload on the floor if it sees an incoming 205 with a payload. So this issue is mainly about the HttpResponseEncoder side (unless you find something I missed).

### Steps to reproduce
Send a 205 response with a payload from a Netty server and monitor the bytes on the wire. You'll see the payload is sent, and content-length is not set to 0 for non-chunked responses.

### Minimal yet complete reproducer code (or URL to code)
``` java
import org.junit.*;

import java.io.IOException;
import java.net.ServerSocket;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

import io.netty.bootstrap.*;
import io.netty.buffer.*;
import io.netty.channel.*;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.SocketChannel;
import io.netty.channel.socket.nio.*;
import io.netty.handler.codec.http.*;
import io.netty.util.CharsetUtil;
import io.netty.util.concurrent.EventExecutorGroup;

import static io.netty.handler.codec.http.HttpHeaders.Names.*;
import static io.netty.handler.codec.http.HttpHeaders.Values.CHUNKED;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import static io.netty.util.CharsetUtil.UTF_8;
import static org.junit.Assert.assertTrue;

public class NettyResponseEncodingSanitizingForHttpStatus205Test {

    private static int serverPort;
    private static final List<EventLoopGroup> eventLoopGroups = new ArrayList<>();
    private static final List<Channel> channels = new ArrayList<>();

    private static final String RESPONSE_PAYLOAD = UUID.randomUUID().toString();

    private static StringBuilder nettyServerRawResponse;
    private static CompletableFuture<FullHttpResponse> responseFuture;

    private static boolean responseShouldBeChunked = false;

    public static int findFreePort() throws IOException {
        try (ServerSocket serverSocket = new ServerSocket(0)) {
            return serverSocket.getLocalPort();
        }
    }

    @Test
    public void verify_netty_encodes_http_status_205_responses_as_always_empty_non_chunked() throws Exception {
        executeRequestFor205Response(false);

        String rawResponse = nettyServerRawResponse.toString();
        String rawResponseBody = extractBodyFromResponse(rawResponse);

        assertTrue(
            "The response payload should have been stripped out by Netty when encoding the 205 response, "
            + "but the response payload was still sent. Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            !rawResponse.contains(RESPONSE_PAYLOAD)
        );
        assertTrue(
            "The Netty response encoder should have set the content-length header to a value of 0 for a "
            + "non-chunked 205 response, but didn't. Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            rawResponse.toLowerCase().contains("content-length: 0\r\n")
        );
        assertTrue(
            "The response payload should have been completely empty, but found a non-empty payload instead. "
            + "Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            rawResponseBody.isEmpty()
        );
    }

    @Test
    public void verify_netty_encodes_http_status_205_responses_as_always_empty_chunked() throws Exception {
        executeRequestFor205Response(true);

        String rawResponse = nettyServerRawResponse.toString();
        String rawResponseBody = extractBodyFromResponse(rawResponse);

        assertTrue(
            "The response payload should have been stripped out by Netty when encoding the 205 response, "
            + "but the response payload was still sent. Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            !rawResponse.contains(RESPONSE_PAYLOAD)
        );
        assertTrue(
            "Expecting to find 'transfer-encoding: chunked' header for chunked response. "
            + "Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            rawResponse.toLowerCase().contains("transfer-encoding: chunked\r\n")
        );
        assertTrue(
            "The response payload should have been completely empty other than the zero-data chunk "
            + "'0\\r\\n\\r\\n', but found a non-empty payload instead. "
            + "Raw bytes-on-the-wire response: " + escapedRawResponse(rawResponse),
            rawResponseBody.equals("0\r\n\r\n")
        );
    }

    private String extractBodyFromResponse(String response) {
        String bodyStartCrLfs = "\r\n\r\n";
        int indexOfBodyStartCrLfs = response.indexOf(bodyStartCrLfs);
        return response.substring(indexOfBodyStartCrLfs + bodyStartCrLfs.length());
    }

    private void executeRequestFor205Response(boolean chunkedResponse) throws Exception {
        responseShouldBeChunked = chunkedResponse;

        Bootstrap clientBootstrap = createNettyHttpClientBootstrap();
        Channel clientChannel = connectNettyHttpClientToLocalServer(clientBootstrap, serverPort);

        clientChannel.writeAndFlush(new DefaultFullHttpRequest(HTTP_1_1, HttpMethod.GET, "/does/not/matter"));

        // We don't want the actual response, we just need to know when it's done so we can inspect the
        //      aggregated bytes-on-the-wire via nettyServerRawResponse.
        responseFuture.get(5, TimeUnit.SECONDS);
    }

    private String escapedRawResponse(String escapeMe) {
        return escapeMe.replace("\r", "\\r").replace("\n", "\\n");
    }

    private static ChannelHandler serverRequestHandler() {
        return new SimpleChannelInboundHandler<HttpObject>() {
            @Override
            protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) {
                if (msg instanceof LastHttpContent) {
                    List<HttpObject> responseChunks = (responseShouldBeChunked)
                                                      ? handleChunkedResponse()
                                                      : handleFullResponse();

                    responseChunks.forEach(ctx::write);
                    ctx.flush();
                }
            }

            private List<HttpObject> handleFullResponse() {
                ByteBuf responsePayload = Unpooled.wrappedBuffer(RESPONSE_PAYLOAD.getBytes(CharsetUtil.UTF_8));

                FullHttpResponse fullResponse = new DefaultFullHttpResponse(
                    HTTP_1_1,
                    HttpResponseStatus.valueOf(205),
                    responsePayload
                );

                fullResponse.headers().set(CONTENT_LENGTH, responsePayload.readableBytes());

                return Collections.singletonList(fullResponse);
            }

            private List<HttpObject> handleChunkedResponse() {
                ByteBuf responsePayload = Unpooled.wrappedBuffer(RESPONSE_PAYLOAD.getBytes(CharsetUtil.UTF_8));

                HttpResponse firstChunk = new DefaultHttpResponse(
                    HTTP_1_1,
                    HttpResponseStatus.valueOf(205)
                );
                firstChunk.headers().set(TRANSFER_ENCODING, CHUNKED);

                HttpContent contentChunk = new DefaultHttpContent(responsePayload);

                return Arrays.asList(firstChunk, contentChunk, LastHttpContent.EMPTY_LAST_CONTENT);
            }
        };
    }

    public static Bootstrap createNettyHttpClientBootstrap() {
        Bootstrap bootstrap = new Bootstrap();
        bootstrap.group(new NioEventLoopGroup())
                 .channel(NioSocketChannel.class)
                 .handler(new ChannelInitializer<SocketChannel>() {
                     @Override
                     protected void initChannel(SocketChannel ch) {
                         ChannelPipeline p = ch.pipeline();
                         p.addLast(new HttpClientCodec());
                         p.addLast(new HttpObjectAggregator(Integer.MAX_VALUE));
                         p.addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                             @Override
                             protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                 // Complete the future so the tests know the response is done.
                                 responseFuture.complete(msg);
                             }
                         });
                     }
                 });

        return bootstrap;
    }

    public static Channel connectNettyHttpClientToLocalServer(Bootstrap bootstrap, int port) throws InterruptedException {
        return bootstrap.connect("localhost", port).sync().channel();
    }

    public static class RecordNettyServerOutboundResponseWireBytes extends ChannelOutboundHandlerAdapter {

        @Override
        public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception {
            ByteBuf byteBuf = (ByteBuf) msg;
            nettyServerRawResponse.append(byteBuf.toString(UTF_8));
            super.write(ctx, msg, promise);
        }
    }

    @Before
    public void beforeMethod() {
        nettyServerRawResponse = new StringBuilder();
        responseFuture = new CompletableFuture<>();
    }

    @BeforeClass
    public static void beforeClass() throws IOException, InterruptedException {
        serverPort = findFreePort();

        EventLoopGroup bossGroup = new NioEventLoopGroup();
        EventLoopGroup workerGroup = new NioEventLoopGroup();
        Class<? extends ServerChannel> channelClass = NioServerSocketChannel.class;

        eventLoopGroups.add(bossGroup);
        eventLoopGroups.add(workerGroup);

        ChannelInitializer<SocketChannel> channelInitializer = new ChannelInitializer<SocketChannel>() {
            @Override
            protected void initChannel(SocketChannel ch) {
                ChannelPipeline pipeline = ch.pipeline();
                pipeline.addLast(new RecordNettyServerOutboundResponseWireBytes());
                pipeline.addLast(new HttpServerCodec());
                pipeline.addLast(serverRequestHandler());
            }
        };

        // Create the server bootstrap
        ServerBootstrap b = new ServerBootstrap();
        b.group(bossGroup, workerGroup)
         .channel(channelClass)
         .childHandler(channelInitializer);

        // Bind the server to the desired port and start it up so it is ready to receive requests
        Channel ch = b.bind(serverPort)
                      .sync()
                      .channel();

        channels.add(ch);

    }

    @AfterClass
    public static void afterClass() throws InterruptedException {
        try {
            List<ChannelFuture> channelCloseFutures = new ArrayList<>();
            for (Channel ch : channels) {
                channelCloseFutures.add(ch.close());
            }
            for (ChannelFuture chf : channelCloseFutures) {
                chf.sync();
            }
        }
        finally {
            eventLoopGroups.forEach(EventExecutorGroup::shutdownGracefully);
        }
    }
}
```

### Netty version
4.0.56.Final

### JVM version (e.g. `java -version`)
1.8.0_151

### OS version (e.g. `uname -a`)
Mac OSX 10.12.6 (Darwin AP10FD59C71492 16.7.0 Darwin Kernel Version 16.7.0: Thu Jan 11 22:59:40 PST 2018; root:xnu-3789.73.8~1/RELEASE_X86_64 x86_64)

Issue comments:
Also verified the same behavior on Netty 4.1.24.Final.
@nicmunroe PTAL https://github.com/netty/netty/pull/7891

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseEncoder.java
index 60086cfd8a2..5ad7a7938e7 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseEncoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseEncoder.java
@@ -53,6 +53,14 @@ protected void sanitizeHeadersBeforeEncode(HttpResponse msg, boolean isAlwaysEmp
                 // Stripping Transfer-Encoding:
                 // See https://tools.ietf.org/html/rfc7230#section-3.3.1
                 msg.headers().remove(HttpHeaderNames.TRANSFER_ENCODING);
+            } else if (status.code() == HttpResponseStatus.RESET_CONTENT.code()) {
+
+                // Stripping Transfer-Encoding:
+                msg.headers().remove(HttpHeaderNames.TRANSFER_ENCODING);
+
+                // Set Content-Length: 0
+                // https://httpstatuses.com/205
+                msg.headers().setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
             }
         }
     }
@@ -74,6 +82,7 @@ protected boolean isContentAlwaysEmpty(HttpResponse msg) {
             return true;
         }
         return status.code() == HttpResponseStatus.NO_CONTENT.code() ||
-                status.code() == HttpResponseStatus.NOT_MODIFIED.code();
+                status.code() == HttpResponseStatus.NOT_MODIFIED.code() ||
+                status.code() == HttpResponseStatus.RESET_CONTENT.code();
     }
 }


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
index 218f6bdf748..2cd07e1018b 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
@@ -353,4 +353,48 @@ private void testEmptyContents(boolean chunked, boolean trailers) throws Excepti
         lastContent.release();
         assertFalse(channel.finish());
     }
+
+    @Test
+    public void testStatusResetContentTransferContentLength() {
+        testStatusResetContentTransferContentLength0(HttpHeaderNames.CONTENT_LENGTH, Unpooled.buffer().writeLong(8));
+    }
+
+    @Test
+    public void testStatusResetContentTransferEncoding() {
+        testStatusResetContentTransferContentLength0(HttpHeaderNames.TRANSFER_ENCODING, Unpooled.buffer().writeLong(8));
+    }
+
+    private static void testStatusResetContentTransferContentLength0(CharSequence headerName, ByteBuf content) {
+        EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseEncoder());
+
+        HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.RESET_CONTENT);
+        if (HttpHeaderNames.CONTENT_LENGTH.contentEqualsIgnoreCase(headerName)) {
+            response.headers().set(HttpHeaderNames.CONTENT_LENGTH, content.readableBytes());
+        } else {
+            response.headers().set(HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderValues.CHUNKED);
+        }
+
+        assertTrue(channel.writeOutbound(response));
+        assertTrue(channel.writeOutbound(new DefaultHttpContent(content)));
+        assertTrue(channel.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT));
+
+        StringBuilder responseText = new StringBuilder();
+        responseText.append(HttpVersion.HTTP_1_1.toString()).append(' ')
+                .append(HttpResponseStatus.RESET_CONTENT.toString()).append("\r\n");
+        responseText.append(HttpHeaderNames.CONTENT_LENGTH).append(": 0\r\n");
+        responseText.append("\r\n");
+
+        StringBuilder written = new StringBuilder();
+        for (;;) {
+            ByteBuf buffer = channel.readOutbound();
+            if (buffer == null) {
+                break;
+            }
+            written.append(buffer.toString(CharsetUtil.US_ASCII));
+            buffer.release();
+        }
+
+        assertEquals(responseText.toString(), written.toString());
+        assertFalse(channel.finish());
+    }
 }