From a44c33136f5d0830b053b4bcff8ef1343d33ae3b Mon Sep 17 00:00:00 2001 From: violetagg Date: Wed, 13 Mar 2019 15:28:28 +0200 Subject: [PATCH] Fix HttpUtil.isKeepAlive to behave correctly when Connection is a comma separated list (#8924) Motivation: According to the specification, the "Connection" header's syntax is: " The Connection header field's value has the following grammar: Connection = 1#connection-option connection-option = token Connection options are case-insensitive. " https://tools.ietf.org/html/rfc7230#section-6.1 This means that Connection's value can have at least one element or a comma separated list with elements When calculating whether the connection can remain open, HttpUtil.isKeepAlive(HttpMessage) should take this into account. Modifications: - Check for "close" and "keep-alive" in a comma separated list - Add unit test Result: HttpUtil.isKeepAlive(HttpMessage) works correctly when "Connection: Upgrade, close" --- .../io/netty/handler/codec/http/HttpUtil.java | 13 +++--------- .../handler/codec/http/HttpUtilTest.java | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java index 3541774695..e140b6ffd9 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java @@ -67,16 +67,9 @@ public final class HttpUtil { * {@link HttpVersion#isKeepAliveDefault()}. */ public static boolean isKeepAlive(HttpMessage message) { - CharSequence connection = message.headers().get(HttpHeaderNames.CONNECTION); - if (HttpHeaderValues.CLOSE.contentEqualsIgnoreCase(connection)) { - return false; - } - - if (message.protocolVersion().isKeepAliveDefault()) { - return !HttpHeaderValues.CLOSE.contentEqualsIgnoreCase(connection); - } else { - return HttpHeaderValues.KEEP_ALIVE.contentEqualsIgnoreCase(connection); - } + return !message.headers().containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE, true) && + (message.protocolVersion().isKeepAliveDefault() || + message.headers().containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE, true)); } /** diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java index b4a6d939b9..e05806a1ca 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java @@ -317,4 +317,25 @@ public class HttpUtilTest { "http:localhost/http_1_0"); assertFalse(HttpUtil.isKeepAlive(http10Message)); } + + @Test + public void testKeepAliveIfConnectionHeaderMultipleValues() { + HttpMessage http11Message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, + "http:localhost/http_1_1"); + http11Message.headers().set( + HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE + ", " + HttpHeaderValues.CLOSE); + assertFalse(HttpUtil.isKeepAlive(http11Message)); + + http11Message.headers().set( + HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE + ", Close"); + assertFalse(HttpUtil.isKeepAlive(http11Message)); + + http11Message.headers().set( + HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE + ", " + HttpHeaderValues.UPGRADE); + assertFalse(HttpUtil.isKeepAlive(http11Message)); + + http11Message.headers().set( + HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE + ", " + HttpHeaderValues.KEEP_ALIVE); + assertTrue(HttpUtil.isKeepAlive(http11Message)); + } }