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

Fix h2c upgrade failure when multiple connection headers are present in upgrade request (#8848)

Browse Source 
Motivation:

When more than one connection header is present in h2c upgrade request, upgrade fails. This is to fix that.

Modification:
In HttpServerUpgradeHandler's upgrade() method, check whether any of the connection header value is upgrade, not just the first header value which might return a different value other than upgrade.

Result:
Fixes #8846.

With this PR, now when multiple connection headers are sent with the upgrade request, upgrade will not fail.
This commit is contained in:
Rukshani Athapathu 2019-02-12 21:35:30 +05:30 committed by Norman Maurer
parent fa6a8cb09c
commit c68e85b749
2 changed files with 88 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java
View File

@ -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<String> 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<CharSequence> requiredHeaders = upgradeCodec.requiredUpgradeHeaders();
List<CharSequence> values = splitHeader(connectionHeader);
List<CharSequence> values = splitHeader(concatenatedConnectionValue);
if (!containsContentEqualsIgnoreCase(values, HttpHeaderNames.UPGRADE) ||
!containsAllContentEqualsIgnoreCase(values, requiredHeaders)) {
!containsAllContentEqualsIgnoreCase(values, requiredHeaders)) {
return false;
}

114
codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java
View File

@ -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());
}
}