From a85635f74c9574efd5d98e6cb717c1635bcc6831 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Wed, 27 Jan 2010 02:38:17 +0000 Subject: [PATCH] * Added HttpVersion.isKeepAliveDefault() to handle the 'Connection' header in a more robust manner * Added HttpVersion constructors with the default keep alive flag - old constructors were deprecated due to ambiguity * Moved HttpMessage.is/setKeepAlive() to HttpHeaders and deprecated the original method --- .../file/HttpStaticFileServerHandler.java | 2 +- .../http/snoop/HttpRequestHandler.java | 4 +- .../websocket/WebSocketServerHandler.java | 2 +- .../codec/http/DefaultHttpMessage.java | 40 +------------- .../codec/http/DefaultHttpRequest.java | 4 +- .../codec/http/DefaultHttpResponse.java | 4 +- .../netty/handler/codec/http/HttpHeaders.java | 54 +++++++++++++++++++ .../netty/handler/codec/http/HttpMessage.java | 34 +----------- .../netty/handler/codec/http/HttpVersion.java | 39 ++++++++++++-- .../handler/codec/rtsp/RtspVersions.java | 4 +- 10 files changed, 100 insertions(+), 87 deletions(-) diff --git a/src/main/java/org/jboss/netty/example/http/file/HttpStaticFileServerHandler.java b/src/main/java/org/jboss/netty/example/http/file/HttpStaticFileServerHandler.java index c3b115a590..5d05e0af01 100644 --- a/src/main/java/org/jboss/netty/example/http/file/HttpStaticFileServerHandler.java +++ b/src/main/java/org/jboss/netty/example/http/file/HttpStaticFileServerHandler.java @@ -96,7 +96,7 @@ public class HttpStaticFileServerHandler extends SimpleChannelUpstreamHandler { ChannelFuture writeFuture = ch.write(new ChunkedFile(raf, 0, fileLength, 8192)); // Decide whether to close the connection or not. - if (!request.isKeepAlive()) { + if (!isKeepAlive(request)) { // Close the connection when the whole content is written out. writeFuture.addListener(ChannelFutureListener.CLOSE); } diff --git a/src/main/java/org/jboss/netty/example/http/snoop/HttpRequestHandler.java b/src/main/java/org/jboss/netty/example/http/snoop/HttpRequestHandler.java index 3b4dc67c20..e82eb9ab3c 100644 --- a/src/main/java/org/jboss/netty/example/http/snoop/HttpRequestHandler.java +++ b/src/main/java/org/jboss/netty/example/http/snoop/HttpRequestHandler.java @@ -125,7 +125,7 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler { private void writeResponse(MessageEvent e) { // Decide whether to close the connection or not. - boolean keepAlive = request.isKeepAlive(); + boolean keepAlive = isKeepAlive(request); // Build the response object. HttpResponse response = new DefaultHttpResponse(HTTP_1_1, OK); @@ -154,7 +154,7 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler { // Write the response. ChannelFuture future = e.getChannel().write(response); - + System.out.println(response.toString()); // Close the non-keep-alive connection after the write operation is done. if (!keepAlive) { future.addListener(ChannelFutureListener.CLOSE); diff --git a/src/main/java/org/jboss/netty/example/http/websocket/WebSocketServerHandler.java b/src/main/java/org/jboss/netty/example/http/websocket/WebSocketServerHandler.java index 37474798a3..b0d1be337f 100644 --- a/src/main/java/org/jboss/netty/example/http/websocket/WebSocketServerHandler.java +++ b/src/main/java/org/jboss/netty/example/http/websocket/WebSocketServerHandler.java @@ -140,7 +140,7 @@ public class WebSocketServerHandler extends SimpleChannelUpstreamHandler { // Send the response and close the connection if necessary. ChannelFuture f = ctx.getChannel().write(res); - if (!req.isKeepAlive() || res.getStatus().getCode() != 200) { + if (!isKeepAlive(req) || res.getStatus().getCode() != 200) { f.addListener(ChannelFutureListener.CLOSE); } } diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java index 943a36b07c..b124aa674c 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java @@ -21,8 +21,6 @@ import java.util.Set; import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.buffer.ChannelBuffers; -import org.jboss.netty.handler.codec.http.HttpHeaders.Names; -import org.jboss.netty.handler.codec.http.HttpHeaders.Values; import org.jboss.netty.util.internal.StringUtil; /** @@ -88,43 +86,9 @@ public class DefaultHttpMessage implements HttpMessage { } } + @Deprecated public boolean isKeepAlive() { - HttpVersion version = getProtocolVersion(); - if (!version.getProtocolName().equals("HTTP")) { - return false; - } - - String connection = getHeader(Names.CONNECTION); - if (HttpHeaders.Values.CLOSE.equalsIgnoreCase(connection)) { - return false; - } - - if (version.equals(HttpVersion.HTTP_1_0) && - !HttpHeaders.Values.KEEP_ALIVE.equalsIgnoreCase(connection)) { - return false; - } - return true; - } - - public void setKeepAlive(boolean keepAlive) { - HttpVersion version = getProtocolVersion(); - if (!version.getProtocolName().equals("HTTP")) { - return; - } - - if (version.equals(HttpVersion.HTTP_1_0)) { - if (keepAlive) { - setHeader(Names.CONNECTION, Values.KEEP_ALIVE); - } else { - removeHeader(Names.CONNECTION); - } - } else { - if (keepAlive) { - removeHeader(Names.CONNECTION); - } else { - setHeader(Names.CONNECTION, Values.CLOSE); - } - } + return HttpHeaders.isKeepAlive(this); } public void clearHeaders() { diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpRequest.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpRequest.java index 518d5e5b1a..4017aa757b 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpRequest.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpRequest.java @@ -69,9 +69,7 @@ public class DefaultHttpRequest extends DefaultHttpMessage implements HttpReques public String toString() { StringBuilder buf = new StringBuilder(); buf.append(getClass().getSimpleName()); - buf.append("(keepAlive: "); - buf.append(isKeepAlive()); - buf.append(", chunked: "); + buf.append("(chunked: "); buf.append(isChunked()); buf.append(')'); buf.append(StringUtil.NEWLINE); diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpResponse.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpResponse.java index 8196bc989e..a14be4ec0d 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpResponse.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpResponse.java @@ -55,9 +55,7 @@ public class DefaultHttpResponse extends DefaultHttpMessage implements HttpRespo public String toString() { StringBuilder buf = new StringBuilder(); buf.append(getClass().getSimpleName()); - buf.append("(keepAlive: "); - buf.append(isKeepAlive()); - buf.append(", chunked: "); + buf.append("(chunked: "); buf.append(isChunked()); buf.append(')'); buf.append(StringUtil.NEWLINE); diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpHeaders.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpHeaders.java index 2579aa509b..24736fa927 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpHeaders.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpHeaders.java @@ -441,6 +441,60 @@ public class HttpHeaders { return defaultValue; } + /** + * Returns {@code true} if and only if the connection can remain open and + * thus 'kept alive'. This methods respects the value of the + * {@code "Connection"} header first and then the return value of + * {@link HttpVersion#isKeepAliveDefault()}. + */ + public static boolean isKeepAlive(HttpMessage message) { + String connection = message.getHeader(Names.CONNECTION); + if (Values.CLOSE.equalsIgnoreCase(connection)) { + return false; + } + + if (message.getProtocolVersion().isKeepAliveDefault()) { + return !Values.CLOSE.equalsIgnoreCase(connection); + } else { + return Values.KEEP_ALIVE.equalsIgnoreCase(connection); + } + } + + /** + * Sets the value of the {@code "Connection"} header depending on the + * protocol version of the specified message. This method sets or removes + * the {@code "Connection"} header depending on what the default keep alive + * mode of the message's protocol version is, as specified by + * {@link HttpVersion#isKeepAliveDefault()}. + * + */ + public static void setKeepAlive(HttpMessage message, boolean keepAlive) { + if (message.getProtocolVersion().isKeepAliveDefault()) { + if (keepAlive) { + message.removeHeader(Names.CONNECTION); + } else { + message.setHeader(Names.CONNECTION, Values.CLOSE); + } + } else { + if (keepAlive) { + message.setHeader(Names.CONNECTION, Values.KEEP_ALIVE); + } else { + message.removeHeader(Names.CONNECTION); + } + } + } + // TODO Document me public static void setContentLength(HttpMessage message, long value) { diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java index 5535bdd7be..44d4ce251b 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java @@ -164,38 +164,8 @@ public interface HttpMessage { void setChunked(boolean chunked); /** - * Returns {@code true} if and only if the connection can remain open and - * thus 'kept alive'. In HTTP, this property is determined by the value of the - * {@code "Connection"} header and the protocol version of this message. - *

- * Please note that the default implementation of this method only - * understands HTTP. If the protocol version of this message indicates - * other derived protocols such as RTSP and ICAP, it will return false by - * default. + * @deprecated Use {@link HttpHeaders#isKeepAlive(HttpMessage)} instead. */ + @Deprecated boolean isKeepAlive(); - - /** - * Sets the value of the {@code "Connection"} header depending on the - * protocol version of this message. The default implementation sets or - * removes the {@code "Connection"} header with the following rules: - *

- * Please note that the default implementation of this method only - * understands HTTP. If the protocol version of this message indicates - * other derived protocols such as RTSP and ICAP, it will do nothing by - * default. - */ - void setKeepAlive(boolean keepAlive); } diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpVersion.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpVersion.java index f5338a7baa..02be42163f 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpVersion.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpVersion.java @@ -38,12 +38,12 @@ public class HttpVersion implements Comparable { /** * HTTP/1.0 */ - public static final HttpVersion HTTP_1_0 = new HttpVersion("HTTP", 1, 0); + public static final HttpVersion HTTP_1_0 = new HttpVersion("HTTP", 1, 0, false); /** * HTTP/1.1 */ - public static final HttpVersion HTTP_1_1 = new HttpVersion("HTTP", 1, 1); + public static final HttpVersion HTTP_1_1 = new HttpVersion("HTTP", 1, 1, true); /** * Returns an existing or new {@link HttpVersion} instance which matches to @@ -65,13 +65,22 @@ public class HttpVersion implements Comparable { if (text.equals("HTTP/1.0")) { return HTTP_1_0; } - return new HttpVersion(text); + return new HttpVersion(text, true); } private final String protocolName; private final int majorVersion; private final int minorVersion; private final String text; + private final boolean keepAliveDefault; + + /** + * @deprecated Use {@link #HttpVersion(String, boolean)} instead. + */ + @Deprecated + public HttpVersion(String text) { + this(text, true); + } /** * Creates a new HTTP version with the specified version string. You will @@ -80,7 +89,7 @@ public class HttpVersion implements Comparable { * RTSP and * ICAP. */ - public HttpVersion(String text) { + public HttpVersion(String text, boolean keepAliveDefault) { if (text == null) { throw new NullPointerException("text"); } @@ -99,6 +108,16 @@ public class HttpVersion implements Comparable { majorVersion = Integer.parseInt(m.group(2)); minorVersion = Integer.parseInt(m.group(3)); this.text = protocolName + '/' + majorVersion + '.' + minorVersion; + this.keepAliveDefault = keepAliveDefault; + } + + /** + * @deprecated Use {@link #HttpVersion(String, int, int, boolean)} instead. + */ + @Deprecated + public HttpVersion( + String protocolName, int majorVersion, int minorVersion) { + this(protocolName, majorVersion, minorVersion, true); } /** @@ -109,7 +128,8 @@ public class HttpVersion implements Comparable { * ICAP */ public HttpVersion( - String protocolName, int majorVersion, int minorVersion) { + String protocolName, int majorVersion, int minorVersion, + boolean keepAliveDefault) { if (protocolName == null) { throw new NullPointerException("protocolName"); } @@ -137,6 +157,7 @@ public class HttpVersion implements Comparable { this.majorVersion = majorVersion; this.minorVersion = minorVersion; text = protocolName + '/' + majorVersion + '.' + minorVersion; + this.keepAliveDefault = keepAliveDefault; } /** @@ -167,6 +188,14 @@ public class HttpVersion implements Comparable { return text; } + /** + * Returns {@code true} if and only if the connection is kept alive unless + * the {@code "Connection"} header is set to {@code "close"} explicitly. + */ + public boolean isKeepAliveDefault() { + return keepAliveDefault; + } + /** * Returns the full protocol version text such as {@code "HTTP/1.0"}. */ diff --git a/src/main/java/org/jboss/netty/handler/codec/rtsp/RtspVersions.java b/src/main/java/org/jboss/netty/handler/codec/rtsp/RtspVersions.java index e3a330857c..9523485352 100644 --- a/src/main/java/org/jboss/netty/handler/codec/rtsp/RtspVersions.java +++ b/src/main/java/org/jboss/netty/handler/codec/rtsp/RtspVersions.java @@ -30,7 +30,7 @@ public final class RtspVersions { /** * RTSP/1.0 */ - public static final HttpVersion RTSP_1_0 = new HttpVersion("RTSP", 1, 0); + public static final HttpVersion RTSP_1_0 = new HttpVersion("RTSP", 1, 0, true); /** * Returns an existing or new {@link HttpVersion} instance which matches to @@ -48,7 +48,7 @@ public final class RtspVersions { return RTSP_1_0; } - return new HttpVersion(text); + return new HttpVersion(text, true); } private RtspVersions() {