andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a bug where the data that follows an HTTP upgrade response is lost

Browse Source 
Related issue: #2179 and #2173

Motivation:

When a WebSocket server responds to a WebSocket handshake request with
its first WebSocket frame, it is sometimes swallowed by
HttpMessageDecoder.  This issue has been fixed by
4f6ccbbb78 in 4 and 5, but was not.

Modification:

Backport the UPGRADED state and the fix for #2173 to HttpMessageDecoder

Result:

The data sent by the server following an HTTP upgrade response is not
lost anymore.
This commit is contained in:
Trustin Lee 2014-07-28 16:07:27 -07:00
parent b35b25aedb
commit e71cbb9308
2 changed files with 98 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java
View File

@ -21,6 +21,7 @@ import org.jboss.netty.channel.Channel;
import org.jboss.netty.channel.ChannelHandlerContext;
import org.jboss.netty.channel.ChannelPipeline;
import org.jboss.netty.handler.codec.frame.TooLongFrameException;
import org.jboss.netty.handler.codec.http.HttpMessageDecoder.State;
import org.jboss.netty.handler.codec.replay.ReplayingDecoder;
import java.util.List;
@ -98,7 +99,7 @@ import java.util.List;
* implement all abstract methods properly.
* @apiviz.landmark
*/
public abstract class HttpMessageDecoder extends ReplayingDecoder<HttpMessageDecoder.State> {
public abstract class HttpMessageDecoder extends ReplayingDecoder<State> {
private final int maxInitialLineLength;
private final int maxHeaderSize;
@ -126,7 +127,8 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<HttpMessageDec
READ_CHUNKED_CONTENT,
READ_CHUNKED_CONTENT_AS_CHUNKS,
READ_CHUNK_DELIMITER,
READ_CHUNK_FOOTER
READ_CHUNK_FOOTER,
UPGRADED
}
/**
@ -397,6 +399,18 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<HttpMessageDec
return trailer;
}
}
case UPGRADED: {
int readableBytes = actualReadableBytes();
if (readableBytes > 0) {
// Keep on consuming as otherwise we may trigger an DecoderException,
// other handler will replace this codec with the upgraded protocol codec to
// take the traffic over at some point then.
// See https://github.com/netty/netty/issues/2173
return buffer.readBytes(actualReadableBytes());
} else {
return null;
}
}
default: {
throw new Error("Shouldn't reach here.");
}
@ -439,6 +453,14 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<HttpMessageDec
}
this.message = null;
if (!isDecodingRequest()) {
HttpResponse res = (HttpResponse) message;
if (res != null && res.getStatus().getCode() == 101) {
checkpoint(State.UPGRADED);
return message;
}
}
checkpoint(State.SKIP_CONTROL_CHARS);
return message;
}

74
src/test/java/org/jboss/netty/handler/codec/http/HttpResponseDecoderTest.java Normal file
View File

@ -0,0 +1,74 @@
/*
* Copyright 2014 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 org.jboss.netty.handler.codec.http;
import org.jboss.netty.buffer.ChannelBuffers;
import org.jboss.netty.handler.codec.embedder.DecoderEmbedder;
import org.junit.Test;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
public class HttpResponseDecoderTest {
@Test
public void testWebSocketResponse() {
byte[] data = ("HTTP/1.1 101 WebSocket Protocol Handshake\r\n" +
"Upgrade: WebSocket\r\n" +
"Connection: Upgrade\r\n" +
"Sec-WebSocket-Origin: http://localhost:8080\r\n" +
"Sec-WebSocket-Location: ws://localhost/some/path\r\n" +
"\r\n" +
"1234567812345678").getBytes();
DecoderEmbedder<Object> ch = new DecoderEmbedder<Object>(new HttpResponseDecoder());
ch.offer(ChannelBuffers.wrappedBuffer(data));
HttpResponse res = (HttpResponse) ch.poll();
assertThat(res.getProtocolVersion(), sameInstance(HttpVersion.HTTP_1_1));
assertThat(res.getStatus(), is(HttpResponseStatus.SWITCHING_PROTOCOLS));
assertThat(res.getContent().readableBytes(), is(16));
assertThat(ch.finish(), is(false));
assertThat(ch.poll(), is(nullValue()));
}
// See https://github.com/netty/netty/issues/2173
@Test
public void testWebSocketResponseWithDataFollowing() {
byte[] data = ("HTTP/1.1 101 WebSocket Protocol Handshake\r\n" +
"Upgrade: WebSocket\r\n" +
"Connection: Upgrade\r\n" +
"Sec-WebSocket-Origin: http://localhost:8080\r\n" +
"Sec-WebSocket-Location: ws://localhost/some/path\r\n" +
"\r\n" +
"1234567812345678").getBytes();
byte[] otherData = {1, 2, 3, 4};
DecoderEmbedder<Object> ch = new DecoderEmbedder<Object>(new HttpResponseDecoder());
ch.offer(ChannelBuffers.wrappedBuffer(data, otherData));
HttpResponse res = (HttpResponse) ch.poll();
assertThat(res.getProtocolVersion(), sameInstance(HttpVersion.HTTP_1_1));
assertThat(res.getStatus(), is(HttpResponseStatus.SWITCHING_PROTOCOLS));
assertThat(res.getContent().readableBytes(), is(16));
assertThat(ch.finish(), is(true));
assertEquals(ch.poll(), ChannelBuffers.wrappedBuffer(otherData));
}
}