From a651804f9d9ca9b42ae5d4d9590228522f7851ba Mon Sep 17 00:00:00 2001
From: Oleksii Kachaiev <kachayev@gmail.com>
Date: Wed, 6 Mar 2019 16:51:58 +0200
Subject: [PATCH] Carefully manage Keep-Alive connections in
 HttpStaticFileServer (#8914)

Motivation:

Simple rules:

* close the connection when sending any error
* specify "Connection: close" header when closing the connection
* successful responses should keep the connection intact when otherwise is not requested by the client

Modifications:

* "send response and cleanup the connection" logic moved to a helper
* for all successful responses set "Content-Lenght" header
* do not specify "Connection: Keep-Alive" header as far it's a default for HTTP/1.1
* set "Connection: close" header when necessary

Result:

Keep-Alive connections management is inlined with RFCs.
---
 .../file/HttpStaticFileServerHandler.java     | 53 +++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java b/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
index 8095fe419c..d5e875102e 100644
--- a/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
+++ b/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
@@ -122,6 +122,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
             return;
         }
 
+        final boolean keepAlive = HttpUtil.isKeepAlive(request);
         final String uri = request.uri();
         final String path = sanitizeUri(uri);
         if (path == null) {
@@ -137,9 +138,9 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
 
         if (file.isDirectory()) {
             if (uri.endsWith("/")) {
-                sendListing(ctx, file, uri);
+                sendListing(ctx, file, uri, keepAlive);
             } else {
-                sendRedirect(ctx, uri + '/');
+                sendRedirect(ctx, uri + '/', keepAlive);
             }
             return;
         }
@@ -160,7 +161,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
             long ifModifiedSinceDateSeconds = ifModifiedSinceDate.getTime() / 1000;
             long fileLastModifiedSeconds = file.lastModified() / 1000;
             if (ifModifiedSinceDateSeconds == fileLastModifiedSeconds) {
-                sendNotModified(ctx);
+                sendNotModified(ctx, keepAlive);
                 return;
             }
         }
@@ -178,8 +179,9 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
         HttpUtil.setContentLength(response, fileLength);
         setContentTypeHeader(response, file);
         setDateAndCacheHeaders(response, file);
-        if (HttpUtil.isKeepAlive(request)) {
-            response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
+
+        if (!keepAlive) {
+            response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE);
         }
 
         // Write the initial line and the header.
@@ -218,7 +220,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
         });
 
         // Decide whether to close the connection or not.
-        if (!HttpUtil.isKeepAlive(request)) {
+        if (!keepAlive) {
             // Close the connection when the whole content is written out.
             lastContentFuture.addListener(ChannelFutureListener.CLOSE);
         }
@@ -264,7 +266,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
 
     private static final Pattern ALLOWED_FILE_NAME = Pattern.compile("[^-\\._]?[^<>&\\\"]*");
 
-    private static void sendListing(ChannelHandlerContext ctx, File dir, String dirPath) {
+    private static void sendListing(ChannelHandlerContext ctx, File dir, String dirPath, boolean keepAlive) {
         FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, OK);
         response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/html; charset=UTF-8");
 
@@ -304,16 +306,14 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
         response.content().writeBytes(buffer);
         buffer.release();
 
-        // Close the connection as soon as the error message is sent.
-        ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE);
+        sendAndCleanupConnection(ctx, response, keepAlive);
     }
 
-    private static void sendRedirect(ChannelHandlerContext ctx, String newUri) {
+    private static void sendRedirect(ChannelHandlerContext ctx, String newUri, boolean keepAlive) {
         FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, FOUND);
         response.headers().set(HttpHeaderNames.LOCATION, newUri);
 
-        // Close the connection as soon as the error message is sent.
-        ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE);
+        sendAndCleanupConnection(ctx, response, keepAlive);
     }
 
     private static void sendError(ChannelHandlerContext ctx, HttpResponseStatus status) {
@@ -321,8 +321,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
                 HTTP_1_1, status, Unpooled.copiedBuffer("Failure: " + status + "\r\n", CharsetUtil.UTF_8));
         response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain; charset=UTF-8");
 
-        // Close the connection as soon as the error message is sent.
-        ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE);
+        sendAndCleanupConnection(ctx, response, false);
     }
 
     /**
@@ -331,12 +330,32 @@ public class HttpStaticFileServerHandler extends SimpleChannelInboundHandler<Ful
      * @param ctx
      *            Context
      */
-    private static void sendNotModified(ChannelHandlerContext ctx) {
+    private static void sendNotModified(ChannelHandlerContext ctx, boolean keepAlive) {
         FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, NOT_MODIFIED);
         setDateHeader(response);
 
-        // Close the connection as soon as the error message is sent.
-        ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE);
+        sendAndCleanupConnection(ctx, response, keepAlive);
+    }
+
+    /**
+     * If Keep-Alive is disabled, attaches "Connection: close" header to the response
+     * and closes the connection after the response being sent.
+     */
+    private static void sendAndCleanupConnection(ChannelHandlerContext ctx, FullHttpResponse response,
+                                                 boolean keepAlive) {
+        HttpUtil.setContentLength(response, response.content().readableBytes());
+        if (!keepAlive) {
+            // We're going to close the connection as soon as the response is sent,
+            // so we should also make it clear for the client.
+            response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE);
+        }
+
+        ChannelFuture flushPromise = ctx.writeAndFlush(response);
+
+        if (!keepAlive) {
+            // Close the connection as soon as the response is sent.
+            flushPromise.addListener(ChannelFutureListener.CLOSE);
+        }
     }
 
     /**