netty: Requests with Connection header are malformed
Although this is part of HTTP/2 and should have already been handled
already, it was noticed as part of RBAC work to avoid matching
hop-by-hop headers. See gRFC A41.
Also add a warning if creating Metadata.Key for "Connection". Use this
to try to help diagnose a client if it happens to blindly copy headers
from HTTP/1, as PROTOCOL_ERROR is hard to debug.
diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java
index e153fd5..9c2a222 100644
--- a/api/src/main/java/io/grpc/Metadata.java
+++ b/api/src/main/java/io/grpc/Metadata.java
@@ -40,6 +40,8 @@
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.NotThreadSafe;
@@ -54,6 +56,7 @@
*/
@NotThreadSafe
public final class Metadata {
+ private static final Logger logger = Logger.getLogger(Metadata.class.getName());
/**
* All binary headers should have this suffix in their names. Vice versa.
@@ -733,6 +736,15 @@
private static String validateName(String n, boolean pseudo) {
checkNotNull(n, "name");
checkArgument(!n.isEmpty(), "token must have at least 1 tchar");
+ if (n.equals("connection")) {
+ logger.log(
+ Level.WARNING,
+ "Metadata key is 'Connection', which should not be used. That is used by HTTP/1 for "
+ + "connection-specific headers which are not to be forwarded. There is probably an "
+ + "HTTP/1 conversion bug. Simply removing the Connection header is not enough; you "
+ + "should remove all headers it references as well. See RFC 7230 section 6.1",
+ new RuntimeException("exception to show backtrace"));
+ }
for (int i = 0; i < n.length(); i++) {
char tChar = n.charAt(i);
if (pseudo && tChar == ':' && i == 0) {
diff --git a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
index 4bdef93..04cf7f4 100644
--- a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
+++ b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
@@ -146,6 +146,11 @@
}
@Override
+ public boolean contains(CharSequence name) {
+ return get(name) != null;
+ }
+
+ @Override
public CharSequence status() {
return get(Http2Headers.PseudoHeaderName.STATUS.value());
}
diff --git a/netty/src/main/java/io/grpc/netty/NettyServerHandler.java b/netty/src/main/java/io/grpc/netty/NettyServerHandler.java
index 6fca656..ee21129 100644
--- a/netty/src/main/java/io/grpc/netty/NettyServerHandler.java
+++ b/netty/src/main/java/io/grpc/netty/NettyServerHandler.java
@@ -26,6 +26,7 @@
import static io.grpc.netty.Utils.HTTP_METHOD;
import static io.grpc.netty.Utils.TE_HEADER;
import static io.grpc.netty.Utils.TE_TRAILERS;
+import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION;
import static io.netty.handler.codec.http2.DefaultHttp2LocalFlowController.DEFAULT_WINDOW_UPDATE_RATIO;
import com.google.common.annotations.VisibleForTesting;
@@ -375,6 +376,12 @@
private void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers)
throws Http2Exception {
try {
+ // Connection-specific header fields makes a request malformed. Ideally this would be handled
+ // by Netty. RFC 7540 section 8.1.2.2
+ if (headers.contains(CONNECTION)) {
+ resetStream(ctx, streamId, Http2Error.PROTOCOL_ERROR.code(), ctx.newPromise());
+ return;
+ }
// Remove the leading slash of the path and get the fully qualified method name
CharSequence path = headers.path();
diff --git a/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java b/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
index 961f983..efae5c0 100644
--- a/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
+++ b/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
@@ -538,6 +538,25 @@
}
@Test
+ public void headersWithConnectionHeaderShouldFail() throws Exception {
+ manualSetUp();
+ Http2Headers headers = new DefaultHttp2Headers()
+ .method(HTTP_METHOD)
+ .set(CONTENT_TYPE_HEADER, CONTENT_TYPE_GRPC)
+ .set(AsciiString.of("connection"), CONTENT_TYPE_GRPC)
+ .path(new AsciiString("/foo/bar"));
+ ByteBuf headersFrame = headersFrame(STREAM_ID, headers);
+ channelRead(headersFrame);
+
+ verifyWrite()
+ .writeRstStream(
+ eq(ctx()),
+ eq(STREAM_ID),
+ eq(Http2Error.PROTOCOL_ERROR.code()),
+ any(ChannelPromise.class));
+ }
+
+ @Test
public void keepAliveManagerOnDataReceived_headersRead() throws Exception {
manualSetUp();
ByteBuf headersFrame = headersFrame(STREAM_ID, new DefaultHttp2Headers());
