Make sure that HttpObjectDecoder decodes the last HTTP message without 'Content-Length' header
- Fixes #1410 - Revert 1e5f266a3c2eb592b55387b4ef187ed0dabbf019 and provide a proper fix with a test
This commit is contained in:
parent
2088d1b491
commit
78d8f05c21
@ -21,6 +21,7 @@ import io.netty.channel.ChannelHandlerContext;
|
|||||||
import io.netty.channel.ChannelPipeline;
|
import io.netty.channel.ChannelPipeline;
|
||||||
import io.netty.channel.MessageList;
|
import io.netty.channel.MessageList;
|
||||||
import io.netty.handler.codec.DecoderResult;
|
import io.netty.handler.codec.DecoderResult;
|
||||||
|
import io.netty.handler.codec.PrematureChannelClosureException;
|
||||||
import io.netty.handler.codec.ReplayingDecoder;
|
import io.netty.handler.codec.ReplayingDecoder;
|
||||||
import io.netty.handler.codec.TooLongFrameException;
|
import io.netty.handler.codec.TooLongFrameException;
|
||||||
|
|
||||||
@ -420,6 +421,44 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, MessageList<Object> out) throws Exception {
|
||||||
|
decode(ctx, in, out);
|
||||||
|
|
||||||
|
// Handle the last unfinished message.
|
||||||
|
if (message != null) {
|
||||||
|
// Get the length of the content received so far for the last message.
|
||||||
|
HttpMessage message = this.message;
|
||||||
|
int actualContentLength;
|
||||||
|
if (content != null) {
|
||||||
|
actualContentLength = content.readableBytes();
|
||||||
|
} else {
|
||||||
|
actualContentLength = 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add the last message (and its content) to the output.
|
||||||
|
reset(out);
|
||||||
|
|
||||||
|
// Check if this situation where the connection has been closed before decoding the last message completely
|
||||||
|
// is expected or not. If unexpected, set decoder result as failure.
|
||||||
|
boolean prematureClosure;
|
||||||
|
if (isDecodingRequest()) {
|
||||||
|
// The last request did not wait for a response.
|
||||||
|
prematureClosure = true;
|
||||||
|
} else {
|
||||||
|
// Compare the length of the received content and the 'Content-Length' header.
|
||||||
|
// If the 'Content-Length' header is absent, the length of the content is determined by the end of the
|
||||||
|
// connection, so it is perfectly fine.
|
||||||
|
long expectedContentLength = HttpHeaders.getContentLength(message, -1);
|
||||||
|
prematureClosure = expectedContentLength >= 0 && actualContentLength != expectedContentLength;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (prematureClosure) {
|
||||||
|
message.setDecoderResult(DecoderResult.failure(new PrematureChannelClosureException()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
protected boolean isContentAlwaysEmpty(HttpMessage msg) {
|
protected boolean isContentAlwaysEmpty(HttpMessage msg) {
|
||||||
if (msg instanceof HttpResponse) {
|
if (msg instanceof HttpResponse) {
|
||||||
HttpResponse res = (HttpResponse) msg;
|
HttpResponse res = (HttpResponse) msg;
|
||||||
@ -450,7 +489,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
|
|||||||
reset(null);
|
reset(null);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected final void reset(MessageList<Object> out) {
|
private void reset(MessageList<Object> out) {
|
||||||
if (out != null) {
|
if (out != null) {
|
||||||
HttpMessage message = this.message;
|
HttpMessage message = this.message;
|
||||||
ByteBuf content = this.content;
|
ByteBuf content = this.content;
|
||||||
|
@ -16,9 +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.channel.ChannelHandlerContext;
|
|
||||||
import io.netty.channel.ChannelPipeline;
|
import io.netty.channel.ChannelPipeline;
|
||||||
import io.netty.channel.MessageList;
|
|
||||||
import io.netty.handler.codec.TooLongFrameException;
|
import io.netty.handler.codec.TooLongFrameException;
|
||||||
|
|
||||||
|
|
||||||
@ -119,20 +117,4 @@ public class HttpResponseDecoder extends HttpObjectDecoder {
|
|||||||
protected boolean isDecodingRequest() {
|
protected boolean isDecodingRequest() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
|
||||||
protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, MessageList<Object> out) throws Exception {
|
|
||||||
super.decodeLast(ctx, in, out);
|
|
||||||
MessageList<Object> msgs = MessageList.newInstance();
|
|
||||||
reset(msgs);
|
|
||||||
if (!msgs.isEmpty()) {
|
|
||||||
// Handle the case where the server sends an empty 200 response and close the connection
|
|
||||||
// https://github.com/netty/netty/issues/1410
|
|
||||||
HttpResponse response = (HttpResponse) msgs.get(0);
|
|
||||||
if (response.getStatus().code() == 200) {
|
|
||||||
out.add(msgs);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
msgs.recycle();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
@ -0,0 +1,44 @@
|
|||||||
|
/*
|
||||||
|
* Copyright 2013 The Netty Project
|
||||||
|
*
|
||||||
|
* The Netty Project licenses this file to you under the Apache License,
|
||||||
|
* version 2.0 (the "License"); you may not use this file except in compliance
|
||||||
|
* with the License. You may obtain a copy of the License at:
|
||||||
|
*
|
||||||
|
* http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||||
|
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||||
|
* License for the specific language governing permissions and limitations
|
||||||
|
* under the License.
|
||||||
|
*/
|
||||||
|
package io.netty.handler.codec.http;
|
||||||
|
|
||||||
|
import io.netty.buffer.Unpooled;
|
||||||
|
import io.netty.channel.embedded.EmbeddedChannel;
|
||||||
|
import io.netty.util.CharsetUtil;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
import static org.hamcrest.CoreMatchers.*;
|
||||||
|
import static org.junit.Assert.*;
|
||||||
|
|
||||||
|
public class HttpResponseDecoderTest {
|
||||||
|
@Test
|
||||||
|
public void testEmptyHeaderAndEmptyContent() {
|
||||||
|
EmbeddedChannel ch = new EmbeddedChannel(new HttpResponseDecoder());
|
||||||
|
ch.writeInbound(Unpooled.copiedBuffer("HTTP/1.1 200 OK\r\n\r\n", CharsetUtil.US_ASCII));
|
||||||
|
|
||||||
|
HttpResponse res = (HttpResponse) ch.readInbound();
|
||||||
|
assertThat(res.getProtocolVersion(), sameInstance(HttpVersion.HTTP_1_1));
|
||||||
|
assertThat(res.getStatus(), is(HttpResponseStatus.OK));
|
||||||
|
assertThat(ch.readInbound(), is(nullValue()));
|
||||||
|
|
||||||
|
assertThat(ch.finish(), is(true));
|
||||||
|
|
||||||
|
LastHttpContent content = (LastHttpContent) ch.readInbound();
|
||||||
|
assertThat(content.content().isReadable(), is(false));
|
||||||
|
|
||||||
|
assertThat(ch.readInbound(), is(nullValue()));
|
||||||
|
}
|
||||||
|
}
|
Loading…
x
Reference in New Issue
Block a user