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:
parent
b4604cdbbc
commit
a1d7a84271
@ -74,7 +74,20 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
|
|||||||
buf.writeBytes(CRLF);
|
buf.writeBytes(CRLF);
|
||||||
state = HttpHeaderUtil.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK;
|
state = HttpHeaderUtil.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 (msg instanceof HttpContent || msg instanceof ByteBuf || msg instanceof FileRegion) {
|
||||||
|
|
||||||
if (state == ST_INIT) {
|
if (state == ST_INIT) {
|
||||||
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
|
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
|
||||||
}
|
}
|
||||||
|
@ -16,6 +16,7 @@
|
|||||||
package io.netty.handler.codec.http;
|
package io.netty.handler.codec.http;
|
||||||
|
|
||||||
import io.netty.buffer.ByteBuf;
|
import io.netty.buffer.ByteBuf;
|
||||||
|
import io.netty.buffer.Unpooled;
|
||||||
import io.netty.channel.FileRegion;
|
import io.netty.channel.FileRegion;
|
||||||
import io.netty.channel.embedded.EmbeddedChannel;
|
import io.netty.channel.embedded.EmbeddedChannel;
|
||||||
import io.netty.util.CharsetUtil;
|
import io.netty.util.CharsetUtil;
|
||||||
@ -24,6 +25,7 @@ import org.junit.Test;
|
|||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.nio.channels.WritableByteChannel;
|
import java.nio.channels.WritableByteChannel;
|
||||||
|
|
||||||
|
import static org.hamcrest.Matchers.*;
|
||||||
import static org.junit.Assert.*;
|
import static org.junit.Assert.*;
|
||||||
|
|
||||||
public class HttpResponseEncoderTest {
|
public class HttpResponseEncoderTest {
|
||||||
@ -119,4 +121,28 @@ public class HttpResponseEncoderTest {
|
|||||||
return false;
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user