Don't disable HttpObjectDecoder on upgrade from HTTP/1.x to HTTP/1.x over TLS

This change allows to upgrade a plain HTTP 1.x connection to TLS
according to RFC 2817. Switching the transport layer to TLS should be
possible without removing HttpClientCodec from the pipeline,
because HTTP/1.x layer of the protocol remains untouched by the switch
and the HttpClientCodec state must be retained for proper
handling the remainder of the response message,
per RFC 2817 requirement in point 3.3:

  Once the TLS handshake completes successfully, the server MUST
  continue with the response to the original request.

After this commit, the upgrade can be established by simply
inserting an SslHandler at the front of the pipeline after receiving
101 SWITCHING PROTOCOLS response, exactly as described in SslHander
documentation.

Modifications:
- Don't set HttpObjectDecoder into UPGRADED state if
  101 SWITCHING_PROTOCOLS response contains HTTP/1.0 or HTTP/1.1 in
  the protocol stack described by the Upgrade header.
- Skip pairing comparison for 101 SWITCHING_PROTOCOLS, similar
  to 100 CONTINUE, since 101 is not the final response to the original
  request and the final response is expected after TLS handshake.

Fixes #7293.
This commit is contained in:
Piotr Kołaczkowski 2017-10-13 12:40:53 +02:00 committed by Norman Maurer
parent 58e74e9fee
commit 7995afee8f
3 changed files with 47 additions and 3 deletions

View File

@ -223,8 +223,8 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
@Override @Override
protected boolean isContentAlwaysEmpty(HttpMessage msg) { protected boolean isContentAlwaysEmpty(HttpMessage msg) {
final int statusCode = ((HttpResponse) msg).status().code(); final int statusCode = ((HttpResponse) msg).status().code();
if (statusCode == 100) { if (statusCode == 100 || statusCode == 101) {
// 100-continue response should be excluded from paired comparison. // 100-continue and 101 switching protocols response should be excluded from paired comparison.
return true; return true;
} }

View File

@ -484,6 +484,20 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
return false; return false;
} }
/**
* Returns true if the server switched to a different protocol than HTTP/1.0 or HTTP/1.1, e.g. HTTP/2 or Websocket.
* Returns false if the upgrade happened in a different layer, e.g. upgrade from HTTP/1.1 to HTTP/1.1 over TLS.
*/
protected boolean isSwitchingToNonHttp1Protocol(HttpResponse msg) {
if (msg.status().code() != HttpResponseStatus.SWITCHING_PROTOCOLS.code()) {
return false;
}
String newProtocol = msg.headers().get(HttpHeaderNames.UPGRADE);
return newProtocol == null ||
!newProtocol.contains(HttpVersion.HTTP_1_0.text()) &&
!newProtocol.contains(HttpVersion.HTTP_1_1.text());
}
/** /**
* Resets the state of the decoder so that it is ready to decode a new message. * Resets the state of the decoder so that it is ready to decode a new message.
* This method is useful for handling a rejected request with {@code Expect: 100-continue} header. * This method is useful for handling a rejected request with {@code Expect: 100-continue} header.
@ -503,7 +517,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
trailer = null; trailer = null;
if (!isDecodingRequest()) { if (!isDecodingRequest()) {
HttpResponse res = (HttpResponse) message; HttpResponse res = (HttpResponse) message;
if (res != null && res.status().code() == 101) { if (res != null && isSwitchingToNonHttp1Protocol(res)) {
currentState = State.UPGRADED; currentState = State.UPGRADED;
return; return;
} }

View File

@ -282,4 +282,34 @@ public class HttpClientCodecTest {
return receivedCount; return receivedCount;
} }
} }
@Test
public void testDecodesFinalResponseAfterSwitchingProtocols() {
String SWITCHING_PROTOCOLS_RESPONSE = "HTTP/1.1 101 Switching Protocols\r\n" +
"Connection: Upgrade\r\n" +
"Upgrade: TLS/1.2, HTTP/1.1\r\n\r\n";
HttpClientCodec codec = new HttpClientCodec(4096, 8192, 8192, true);
EmbeddedChannel ch = new EmbeddedChannel(codec, new HttpObjectAggregator(1024));
HttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/");
request.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
request.headers().set(HttpHeaderNames.UPGRADE, "TLS/1.2");
assertTrue("Channel outbound write failed.", ch.writeOutbound(request));
assertTrue("Channel inbound write failed.",
ch.writeInbound(Unpooled.copiedBuffer(SWITCHING_PROTOCOLS_RESPONSE, CharsetUtil.ISO_8859_1)));
Object switchingProtocolsResponse = ch.readInbound();
assertNotNull("No response received", switchingProtocolsResponse);
assertThat("Response was not decoded", switchingProtocolsResponse, instanceOf(FullHttpResponse.class));
((FullHttpResponse) switchingProtocolsResponse).release();
assertTrue("Channel inbound write failed",
ch.writeInbound(Unpooled.copiedBuffer(RESPONSE, CharsetUtil.ISO_8859_1)));
Object finalResponse = ch.readInbound();
assertNotNull("No response received", finalResponse);
assertThat("Response was not decoded", finalResponse, instanceOf(FullHttpResponse.class));
((FullHttpResponse) finalResponse).release();
assertTrue("Channel finish failed", ch.finishAndReleaseAll());
}
} }