diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java index f1f3efcb1d..2b54b0e4b2 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java @@ -14,9 +14,6 @@ */ package io.netty.handler.codec.http; -import static io.netty.util.AsciiString.containsContentEqualsIgnoreCase; -import static io.netty.util.AsciiString.containsAllContentEqualsIgnoreCase; - import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; @@ -30,7 +27,10 @@ import java.util.List; import static io.netty.handler.codec.http.HttpResponseStatus.SWITCHING_PROTOCOLS; import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1; +import static io.netty.util.AsciiString.containsAllContentEqualsIgnoreCase; +import static io.netty.util.AsciiString.containsContentEqualsIgnoreCase; import static io.netty.util.internal.ObjectUtil.checkNotNull; +import static io.netty.util.internal.StringUtil.COMMA; /** * A server-side handler that receives HTTP requests and optionally performs a protocol switch if @@ -284,16 +284,23 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator { } // Make sure the CONNECTION header is present. - CharSequence connectionHeader = request.headers().get(HttpHeaderNames.CONNECTION); - if (connectionHeader == null) { + List connectionHeaderValues = request.headers().getAll(HttpHeaderNames.CONNECTION); + + if (connectionHeaderValues == null) { return false; } + final StringBuilder concatenatedConnectionValue = new StringBuilder(connectionHeaderValues.size() * 10); + for (CharSequence connectionHeaderValue : connectionHeaderValues) { + concatenatedConnectionValue.append(connectionHeaderValue).append(COMMA); + } + concatenatedConnectionValue.setLength(concatenatedConnectionValue.length() - 1); + // Make sure the CONNECTION header contains UPGRADE as well as all protocol-specific headers. Collection requiredHeaders = upgradeCodec.requiredUpgradeHeaders(); - List values = splitHeader(connectionHeader); + List values = splitHeader(concatenatedConnectionValue); if (!containsContentEqualsIgnoreCase(values, HttpHeaderNames.UPGRADE) || - !containsAllContentEqualsIgnoreCase(values, requiredHeaders)) { + !containsAllContentEqualsIgnoreCase(values, requiredHeaders)) { return false; } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java index a544054b66..129f62d757 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java @@ -42,8 +42,15 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Tests for {@link CleartextHttp2ServerUpgradeHandler} @@ -112,47 +119,35 @@ public class CleartextHttp2ServerUpgradeHandlerTest { @Test public void upgrade() throws Exception { - setUpServerChannel(); - String upgradeString = "GET / HTTP/1.1\r\n" + - "Host: example.com\r\n" + - "Connection: Upgrade, HTTP2-Settings\r\n" + - "Upgrade: h2c\r\n" + - "HTTP2-Settings: AAMAAABkAAQAAP__\r\n\r\n"; - ByteBuf upgrade = Unpooled.copiedBuffer(upgradeString, CharsetUtil.US_ASCII); + "Host: example.com\r\n" + + "Connection: Upgrade, HTTP2-Settings\r\n" + + "Upgrade: h2c\r\n" + + "HTTP2-Settings: AAMAAABkAAQAAP__\r\n\r\n"; + validateClearTextUpgrade(upgradeString); + } - assertFalse(channel.writeInbound(upgrade)); + @Test + public void upgradeWithMultipleConnectionHeaders() { + String upgradeString = "GET / HTTP/1.1\r\n" + + "Host: example.com\r\n" + + "Connection: keep-alive\r\n" + + "Connection: Upgrade, HTTP2-Settings\r\n" + + "Upgrade: h2c\r\n" + + "HTTP2-Settings: AAMAAABkAAQAAP__\r\n\r\n"; + validateClearTextUpgrade(upgradeString); + } - assertEquals(1, userEvents.size()); - - Object userEvent = userEvents.get(0); - assertTrue(userEvent instanceof UpgradeEvent); - assertEquals("h2c", ((UpgradeEvent) userEvent).protocol()); - ReferenceCountUtil.release(userEvent); - - assertEquals(100, http2ConnectionHandler.connection().local().maxActiveStreams()); - assertEquals(65535, http2ConnectionHandler.connection().local().flowController().initialWindowSize()); - - assertEquals(1, http2ConnectionHandler.connection().numActiveStreams()); - assertNotNull(http2ConnectionHandler.connection().stream(1)); - - Http2Stream stream = http2ConnectionHandler.connection().stream(1); - assertEquals(State.HALF_CLOSED_REMOTE, stream.state()); - assertFalse(stream.isHeadersSent()); - - String expectedHttpResponse = "HTTP/1.1 101 Switching Protocols\r\n" + - "connection: upgrade\r\n" + - "upgrade: h2c\r\n\r\n"; - ByteBuf responseBuffer = channel.readOutbound(); - assertEquals(expectedHttpResponse, responseBuffer.toString(CharsetUtil.UTF_8)); - responseBuffer.release(); - - // Check that the preface was send (a.k.a the settings frame) - ByteBuf settingsBuffer = channel.readOutbound(); - assertNotNull(settingsBuffer); - settingsBuffer.release(); - - assertNull(channel.readOutbound()); + @Test + public void requiredHeadersInSeparateConnectionHeaders() { + String upgradeString = "GET / HTTP/1.1\r\n" + + "Host: example.com\r\n" + + "Connection: keep-alive\r\n" + + "Connection: HTTP2-Settings\r\n" + + "Connection: Upgrade\r\n" + + "Upgrade: h2c\r\n" + + "HTTP2-Settings: AAMAAABkAAQAAP__\r\n\r\n"; + validateClearTextUpgrade(upgradeString); } @Test @@ -254,4 +249,43 @@ public class CleartextHttp2ServerUpgradeHandlerTest { private static Http2Settings expectedSettings() { return new Http2Settings().maxConcurrentStreams(100).initialWindowSize(65535); } + + private void validateClearTextUpgrade(String upgradeString) { + setUpServerChannel(); + + ByteBuf upgrade = Unpooled.copiedBuffer(upgradeString, CharsetUtil.US_ASCII); + + assertFalse(channel.writeInbound(upgrade)); + + assertEquals(1, userEvents.size()); + + Object userEvent = userEvents.get(0); + assertTrue(userEvent instanceof UpgradeEvent); + assertEquals("h2c", ((UpgradeEvent) userEvent).protocol()); + ReferenceCountUtil.release(userEvent); + + assertEquals(100, http2ConnectionHandler.connection().local().maxActiveStreams()); + assertEquals(65535, http2ConnectionHandler.connection().local().flowController().initialWindowSize()); + + assertEquals(1, http2ConnectionHandler.connection().numActiveStreams()); + assertNotNull(http2ConnectionHandler.connection().stream(1)); + + Http2Stream stream = http2ConnectionHandler.connection().stream(1); + assertEquals(State.HALF_CLOSED_REMOTE, stream.state()); + assertFalse(stream.isHeadersSent()); + + String expectedHttpResponse = "HTTP/1.1 101 Switching Protocols\r\n" + + "connection: upgrade\r\n" + + "upgrade: h2c\r\n\r\n"; + ByteBuf responseBuffer = channel.readOutbound(); + assertEquals(expectedHttpResponse, responseBuffer.toString(CharsetUtil.UTF_8)); + responseBuffer.release(); + + // Check that the preface was send (a.k.a the settings frame) + ByteBuf settingsBuffer = channel.readOutbound(); + assertNotNull(settingsBuffer); + settingsBuffer.release(); + + assertNull(channel.readOutbound()); + } }