Repository: netty/netty
Issue: SpdyHttpDecoder broken
Labels: defect

At some point in the porting to netty 4 the SpdyHttpDecoder stopped writing RstStreams. As an example, the following code:

// If a client receives a response with a truncated header block,
// reply with a RST_STREAM with error code INTERNAL_ERROR.
if (spdySynStreamFrame.isTruncated()) {
    SpdyRstStreamFrame spdyRstStreamFrame =
    new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.INTERNAL_ERROR);
    out.add(spdyRstStreamFrame);
}

should have been

// If a client receives a response with a truncated header block,
// reply with a RST_STREAM with error code INTERNAL_ERROR.
if (spdySynStreamFrame.isTruncated()) {
    SpdyRstStreamFrame spdyRstStreamFrame =
    new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.INTERNAL_ERROR);
    ctx.write(spdyRstStreamFrame); // and possibly ctx.flush() also now
}


Issue comments:
I will try to go through the codec carefully and see if anything else is broken.

So you will issue a pullreq?

Am 10.07.2013 um 19:34 schrieb Jeff Pinner notifications@github.com:

> I will try to go through the codec carefully and see if anything else is broken.
> 
> —
> Reply to this email directly or view it on GitHub.

Working on it now.

Alright thanks... will review pullreq once you issued it

ups sorry... closed it by mistake

Issued pull request #1554. 

@jpinner merged #1554 ... Thanks!


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
index f9a991916eb..7041cfda79a 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
@@ -108,7 +108,8 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                 if (associatedToStreamId == 0) {
                     SpdyRstStreamFrame spdyRstStreamFrame =
                         new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.INVALID_STREAM);
-                    out.add(spdyRstStreamFrame);
+                    ctx.writeAndFlush(spdyRstStreamFrame);
+                    return;
                 }
 
                 String URL = SpdyHeaders.getUrl(spdyVersion, spdySynStreamFrame);
@@ -118,7 +119,8 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                 if (URL == null) {
                     SpdyRstStreamFrame spdyRstStreamFrame =
                         new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.PROTOCOL_ERROR);
-                    out.add(spdyRstStreamFrame);
+                    ctx.writeAndFlush(spdyRstStreamFrame);
+                    return;
                 }
 
                 // If a client receives a response with a truncated header block,
@@ -126,7 +128,8 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                 if (spdySynStreamFrame.isTruncated()) {
                     SpdyRstStreamFrame spdyRstStreamFrame =
                     new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.INTERNAL_ERROR);
-                    out.add(spdyRstStreamFrame);
+                    ctx.writeAndFlush(spdyRstStreamFrame);
+                    return;
                 }
 
                 try {
@@ -149,7 +152,7 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                 } catch (Exception e) {
                     SpdyRstStreamFrame spdyRstStreamFrame =
                         new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.PROTOCOL_ERROR);
-                    out.add(spdyRstStreamFrame);
+                    ctx.writeAndFlush(spdyRstStreamFrame);
                 }
             } else {
                 // SYN_STREAM frames initiated by the client are HTTP requests
@@ -163,7 +166,8 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                             spdySynReplyFrame,
                             HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE);
                     SpdyHeaders.setVersion(spdyVersion, spdySynReplyFrame, HttpVersion.HTTP_1_0);
-                    out.add(spdySynReplyFrame);
+                    ctx.writeAndFlush(spdySynReplyFrame);
+                    return;
                 }
 
                 try {
@@ -186,7 +190,7 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                     spdySynReplyFrame.setLast(true);
                     SpdyHeaders.setStatus(spdyVersion, spdySynReplyFrame, HttpResponseStatus.BAD_REQUEST);
                     SpdyHeaders.setVersion(spdyVersion, spdySynReplyFrame, HttpVersion.HTTP_1_0);
-                    out.add(spdySynReplyFrame);
+                    ctx.writeAndFlush(spdySynReplyFrame);
                 }
             }
 
@@ -200,7 +204,8 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
             if (spdySynReplyFrame.isTruncated()) {
                 SpdyRstStreamFrame spdyRstStreamFrame =
                         new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.INTERNAL_ERROR);
-                out.add(spdyRstStreamFrame);
+                ctx.writeAndFlush(spdyRstStreamFrame);
+                return;
             }
 
             try {
@@ -221,7 +226,7 @@ protected void decode(ChannelHandlerContext ctx, SpdyFrame msg, List<Object> out
                 // the client must reply with a RST_STREAM frame indicating a PROTOCOL_ERROR
                 SpdyRstStreamFrame spdyRstStreamFrame =
                     new DefaultSpdyRstStreamFrame(streamId, SpdyStreamStatus.PROTOCOL_ERROR);
-                out.add(spdyRstStreamFrame);
+                ctx.writeAndFlush(spdyRstStreamFrame);
             }
 
         } else if (msg instanceof SpdyHeadersFrame) {


Test patch: