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

Include Http 1 request in error message

Browse Source 
Motivation:

When An HTTP server is listening in plaintext mode, it doesn't have
a chance to negotiate "h2" in the tls handshake.  HTTP 1 clients
that are not expecting an HTTP2 server will accidentally a request
that isn't an upgrade, which the HTTP/2 decoder will not
understand.  The decoder treats the bytes as hex and adds them to
the error message.

These error messages are hard to understand by humans, and result
in extra, manual work to decode.

Modification:

If the first bytes of the request are not the preface, the decoder
will now see if they are an HTTP/1 request first.  If so, the error
message will include the method and path of the original request in
the error message.

In case the path is long, the decoder will check up to the first
1024 bytes to see if it matches.  This could be a DoS vector if
tons of bad requests or other garbage come in.  A future optimization
would be to treat the first few bytes as an AsciiString and not do
any Charset decoding.  ByteBuf.toCharSequence alludes to such an
optimization.

The code has been left simple for the time being.

Result:

Faster identification of errant HTTP requests.
This commit is contained in:
Carl Mastrangelo 2017-01-25 14:37:45 -08:00 committed by Scott Mitchell
parent 735d6dd636
commit ead9938980
3 changed files with 44 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
View File

@ -158,6 +158,22 @@ public final class ByteBufUtil {
return hashCode;
}
/**
* Returns the reader index of needle in haystack, or -1 if needle is not in haystack.
*/
public static int indexOf(ByteBuf needle, ByteBuf haystack) {
// TODO: maybe use Boyer Moore for efficiency.
int attempts = haystack.readableBytes() - needle.readableBytes() + 1;
for (int i = 0; i < attempts; i++) {
if (equals(needle, needle.readerIndex(),
haystack, haystack.readerIndex() + i,
needle.readableBytes())) {
return haystack.readerIndex() + i;
}
}
return -1;
}
/**
* Returns {@code true} if and only if the two specified buffers are
* identical to each other for {@code length} bytes starting at {@code aStartIndex}

11
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
View File

@ -16,6 +16,7 @@ package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
@ -25,6 +26,7 @@ import io.netty.handler.codec.ByteToMessageDecoder;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException;
import io.netty.handler.codec.http2.Http2Exception.StreamException;
import io.netty.util.CharsetUtil;
import io.netty.util.concurrent.ScheduledFuture;
import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger;
@ -68,6 +70,8 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
private static final Http2Headers HEADERS_TOO_LARGE_HEADERS = ReadOnlyHttp2Headers.serverHeaders(false,
HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE.codeAsText());
private static final ByteBuf HTTP_1_X_BUF = Unpooled.unreleasableBuffer(
Unpooled.wrappedBuffer(new byte[] {'H', 'T', 'T', 'P', '/', '1', '.'})).asReadOnly();
private final Http2ConnectionDecoder decoder;
private final Http2ConnectionEncoder encoder;
@ -272,6 +276,13 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
if (bytesRead == 0 || !ByteBufUtil.equals(in, in.readerIndex(),
clientPrefaceString, clientPrefaceString.readerIndex(),
bytesRead)) {
int maxSearch = 1024; // picked because 512 is too little, and 2048 too much
int http1Index =
ByteBufUtil.indexOf(HTTP_1_X_BUF, in.slice(in.readerIndex(), min(in.readableBytes(), maxSearch)));
if (http1Index != -1) {
String chunk = in.toString(in.readerIndex(), http1Index - in.readerIndex(), CharsetUtil.US_ASCII);
throw connectionError(PROTOCOL_ERROR, "Unexpected HTTP/1.x request: %s", chunk);
}
String receivedBytes = hexDump(in, in.readerIndex(),
min(in.readableBytes(), clientPrefaceString.readableBytes()));
throw connectionError(PROTOCOL_ERROR, "HTTP/2 client preface string missing or corrupt. " +

17
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
View File

@ -27,6 +27,7 @@ import io.netty.channel.ChannelPromise;
import io.netty.channel.DefaultChannelPromise;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http2.Http2CodecUtil.SimpleChannelPromiseAggregator;
import io.netty.util.CharsetUtil;
import io.netty.util.ReferenceCountUtil;
import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.GenericFutureListener;
@ -52,6 +53,7 @@ import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.handler.codec.http2.Http2Stream.State.CLOSED;
import static io.netty.handler.codec.http2.Http2Stream.State.IDLE;
import static io.netty.handler.codec.http2.Http2TestUtil.newVoidPromise;
import static io.netty.util.CharsetUtil.US_ASCII;
import static io.netty.util.CharsetUtil.UTF_8;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@ -127,6 +129,8 @@ public class Http2ConnectionHandlerTest {
@Mock
private Http2FrameWriter frameWriter;
private String goAwayDebugCap;
@SuppressWarnings("unchecked")
@Before
public void setup() throws Exception {
@ -144,6 +148,7 @@ public class Http2ConnectionHandlerTest {
@Override
public ChannelFuture answer(InvocationOnMock invocation) throws Throwable {
ByteBuf buf = invocation.getArgumentAt(3, ByteBuf.class);
goAwayDebugCap = buf.toString(UTF_8);
buf.release();
return future;
}
@ -242,6 +247,18 @@ public class Http2ConnectionHandlerTest {
assertEquals(0, captor.getValue().refCnt());
}
@Test
public void serverReceivingHttp1ClientPrefaceStringShouldIncludePreface() throws Exception {
when(connection.isServer()).thenReturn(true);
handler = newHandler();
handler.channelRead(ctx, copiedBuffer("GET /path HTTP/1.1", US_ASCII));
ArgumentCaptor<ByteBuf> captor = ArgumentCaptor.forClass(ByteBuf.class);
verify(frameWriter).writeGoAway(eq(ctx), eq(0), eq(PROTOCOL_ERROR.code()),
captor.capture(), eq(promise));
assertEquals(0, captor.getValue().refCnt());
assertTrue(goAwayDebugCap.contains("/path"));
}
@Test
public void serverReceivingClientPrefaceStringFollowedByNonSettingsShouldHandleException()
throws Exception {