Repository: netty/netty
Issue: SpdyHttpEncoder does not correctly set isLast() when sending a FullHttpMessage
Labels: defect

For a FullHttpMessage with no content, two SPDY frames are sent, one SpdySynStreamFrame(last=false) plus one SpdyDataFrame with content and last=true.  For known empty messages, last should be set to true in the SpdySynStreamFrame.


Issue comments:
fixed with 80f92f90cbc9a8267cad3f338641982b53478e8a


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java
index 3862e2aedb3..a8e3eefd262 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java
@@ -18,6 +18,7 @@
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.MessageToMessageEncoder;
 import io.netty.handler.codec.UnsupportedMessageTypeException;
+import io.netty.handler.codec.http.FullHttpMessage;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.HttpContent;
 import io.netty.handler.codec.http.HttpHeaders;
@@ -141,6 +142,7 @@ public SpdyHttpEncoder(int version) {
     protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> out) throws Exception {
 
         boolean valid = false;
+        boolean last = false;
 
         if (msg instanceof HttpRequest) {
 
@@ -148,6 +150,7 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
             SpdySynStreamFrame spdySynStreamFrame = createSynStreamFrame(httpRequest);
             out.add(spdySynStreamFrame);
 
+            last = spdySynStreamFrame.isLast();
             valid = true;
         }
         if (msg instanceof HttpResponse) {
@@ -155,15 +158,17 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
             HttpResponse httpResponse = (HttpResponse) msg;
             if (httpResponse.headers().contains(SpdyHttpHeaders.Names.ASSOCIATED_TO_STREAM_ID)) {
                 SpdySynStreamFrame spdySynStreamFrame = createSynStreamFrame(httpResponse);
+                last = spdySynStreamFrame.isLast();
                 out.add(spdySynStreamFrame);
             } else {
                 SpdySynReplyFrame spdySynReplyFrame = createSynReplyFrame(httpResponse);
+                last = spdySynReplyFrame.isLast();
                 out.add(spdySynReplyFrame);
             }
 
             valid = true;
         }
-        if (msg instanceof HttpContent) {
+        if (msg instanceof HttpContent && !last) {
 
             HttpContent chunk = (HttpContent) msg;
 
@@ -255,6 +260,7 @@ private SpdySynStreamFrame createSynStreamFrame(HttpMessage httpMessage)
             spdySynStreamFrame.headers().add(entry.getKey(), entry.getValue());
         }
         currentStreamId = spdySynStreamFrame.getStreamId();
+        spdySynStreamFrame.setLast(isLast(httpMessage));
 
         return spdySynStreamFrame;
     }
@@ -284,8 +290,25 @@ private SpdySynReplyFrame createSynReplyFrame(HttpResponse httpResponse)
         }
 
         currentStreamId = streamID;
-        spdySynReplyFrame.setLast(false);
+        spdySynReplyFrame.setLast(isLast(httpResponse));
 
         return spdySynReplyFrame;
     }
+
+    /**
+     * Checks if the given HTTP message should be considered as a last SPDY frame.
+     *
+     * @param httpMessage check this HTTP message
+     * @return whether the given HTTP message should generate a <em>last</em> SPDY frame.
+     */
+    private static boolean isLast(HttpMessage httpMessage) {
+        if (httpMessage instanceof FullHttpMessage) {
+            FullHttpMessage fullMessage = (FullHttpMessage) httpMessage;
+            if (fullMessage.trailingHeaders().isEmpty() && !fullMessage.content().isReadable()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }


Test patch: