diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index e153fd55691..9c2a2227f8c 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 BitSet generateValidTChars() { 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 4bdef93ad04..04cf7f4195f 100644 --- a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java +++ b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java @@ -145,6 +145,11 @@ protected CharSequence get(AsciiString name) { return null; } + @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 6fca656e795..ee21129b67a 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 @@ public void run() { 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 961f983d9cd..efae5c04989 100644 --- a/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java @@ -537,6 +537,25 @@ public void headersSupportExtensionContentType() throws Exception { stream = streamCaptor.getValue(); } + @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();