Handle an empty ByteBuf specially in HttpObjectEncoder

Related: #2983

Motivation:

It is a well known idiom to write an empty buffer and add a listener to
its future to close a channel when the last byte has been written out:

  ChannelFuture f = channel.writeAndFlush(Unpooled.EMPTY_BUFFER);
  f.addListener(ChannelFutureListener.CLOSE);

When HttpObjectEncoder is in the pipeline, this still works, but it
silently raises an IllegalStateException, because HttpObjectEncoder does
not allow writing a ByteBuf when it is expecting an HttpMessage.

Modifications:

- Handle an empty ByteBuf specially in HttpObjectEncoder, so that
  writing an empty buffer does not fail even if the pipeline contains an
  HttpObjectEncoder
- Add a test

Result:

An exception is not triggered anymore by HttpObjectEncoder, when a user
attempts to write an empty buffer.
This commit is contained in:
Trustin Lee 2014-10-22 14:39:31 +09:00
parent 67c68ef8ba
commit 789e323b79
2 changed files with 39 additions and 0 deletions

View File

@ -73,7 +73,20 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
buf.writeBytes(CRLF);
state = HttpHeaders.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK;
}
// Bypass the encoder in case of an empty buffer, so that the following idiom works:
//
// ch.write(Unpooled.EMPTY_BUFFER).addListener(ChannelFutureListener.CLOSE);
//
// See https://github.com/netty/netty/issues/2983 for more information.
if (msg instanceof ByteBuf && !((ByteBuf) msg).isReadable()) {
out.add(EMPTY_BUFFER);
return;
}
if (msg instanceof HttpContent || msg instanceof ByteBuf || msg instanceof FileRegion) {
if (state == ST_INIT) {
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
}

View File

@ -16,6 +16,7 @@
package io.netty.handler.codec.http;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.FileRegion;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.CharsetUtil;
@ -24,6 +25,7 @@ import org.junit.Test;
import java.io.IOException;
import java.nio.channels.WritableByteChannel;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
public class HttpResponseEncoderTest {
@ -118,4 +120,28 @@ public class HttpResponseEncoderTest {
return false;
}
}
@Test
public void testEmptyBufferBypass() throws Exception {
EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseEncoder());
// Test writing an empty buffer works when the encoder is at ST_INIT.
channel.writeOutbound(Unpooled.EMPTY_BUFFER);
ByteBuf buffer = channel.readOutbound();
assertThat(buffer, is(sameInstance(Unpooled.EMPTY_BUFFER)));
// Leave the ST_INIT state.
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
assertTrue(channel.writeOutbound(response));
buffer = channel.readOutbound();
assertEquals("HTTP/1.1 200 OK\r\n\r\n", buffer.toString(CharsetUtil.US_ASCII));
buffer.release();
// Test writing an empty buffer works when the encoder is not at ST_INIT.
channel.writeOutbound(Unpooled.EMPTY_BUFFER);
buffer = channel.readOutbound();
assertThat(buffer, is(sameInstance(Unpooled.EMPTY_BUFFER)));
assertFalse(channel.finish());
}
}