From 3b237339c7b0b5e60b6331cac9e23650e97b1e8c Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Mon, 13 Sep 2021 17:15:45 -0700 Subject: [PATCH] core: discard outbound content-length header (#8522) Since netty version v4.1.67, content-lenght header validation will be enforced. So once grpc upgrades netty to that version or above, RPCs with invalid content-length header will fail. Some libraries such as HTTP to gRPC adapters blindly copy all HTTP headers to gRPC metadata, but the content-length header is one of those that shouldn't be forwarded because gRPC uses different encoding. This mistake has already been in existence for a long time. Discard outbound content-length headers in gRPC, so that users who encounter invalid content-length issue when upgrading grpc-java version on server/client side would be able to workaround by upgrading grpc-java on client/server side as well without fixing the HTTP adapter. --- .../main/java/io/grpc/internal/ClientCallImpl.java | 2 ++ core/src/main/java/io/grpc/internal/GrpcUtil.java | 3 +++ .../main/java/io/grpc/internal/ServerCallImpl.java | 2 ++ .../java/io/grpc/internal/ClientCallImplTest.java | 9 +++++++++ .../java/io/grpc/internal/ServerCallImplTest.java | 11 +++++++++++ 5 files changed, 27 insertions(+) diff --git a/core/src/main/java/io/grpc/internal/ClientCallImpl.java b/core/src/main/java/io/grpc/internal/ClientCallImpl.java index dd17244e2a..6f850ade66 100644 --- a/core/src/main/java/io/grpc/internal/ClientCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ClientCallImpl.java @@ -24,6 +24,7 @@ import static io.grpc.Contexts.statusFromCancelled; import static io.grpc.Status.DEADLINE_EXCEEDED; import static io.grpc.internal.GrpcUtil.CONTENT_ACCEPT_ENCODING_KEY; import static io.grpc.internal.GrpcUtil.CONTENT_ENCODING_KEY; +import static io.grpc.internal.GrpcUtil.CONTENT_LENGTH_KEY; import static io.grpc.internal.GrpcUtil.MESSAGE_ACCEPT_ENCODING_KEY; import static io.grpc.internal.GrpcUtil.MESSAGE_ENCODING_KEY; import static java.lang.Math.max; @@ -163,6 +164,7 @@ final class ClientCallImpl extends ClientCall { DecompressorRegistry decompressorRegistry, Compressor compressor, boolean fullStreamDecompression) { + headers.discardAll(CONTENT_LENGTH_KEY); headers.discardAll(MESSAGE_ENCODING_KEY); if (compressor != Codec.Identity.NONE) { headers.put(MESSAGE_ENCODING_KEY, compressor.getMessageEncoding()); diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index 98e1d00585..55e2cd8153 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -109,6 +109,9 @@ public final class GrpcUtil { public static final Metadata.Key CONTENT_ACCEPT_ENCODING_KEY = InternalMetadata.keyOf(GrpcUtil.CONTENT_ACCEPT_ENCODING, new AcceptEncodingMarshaller()); + static final Metadata.Key CONTENT_LENGTH_KEY = + Metadata.Key.of("content-length", Metadata.ASCII_STRING_MARSHALLER); + private static final class AcceptEncodingMarshaller implements TrustedAsciiMarshaller { @Override public byte[] toAsciiString(byte[] value) { diff --git a/core/src/main/java/io/grpc/internal/ServerCallImpl.java b/core/src/main/java/io/grpc/internal/ServerCallImpl.java index f82d87cade..deba21c315 100644 --- a/core/src/main/java/io/grpc/internal/ServerCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerCallImpl.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static io.grpc.internal.GrpcUtil.ACCEPT_ENCODING_SPLITTER; +import static io.grpc.internal.GrpcUtil.CONTENT_LENGTH_KEY; import static io.grpc.internal.GrpcUtil.MESSAGE_ACCEPT_ENCODING_KEY; import static io.grpc.internal.GrpcUtil.MESSAGE_ENCODING_KEY; @@ -107,6 +108,7 @@ final class ServerCallImpl extends ServerCall { checkState(!sendHeadersCalled, "sendHeaders has already been called"); checkState(!closeCalled, "call is closed"); + headers.discardAll(CONTENT_LENGTH_KEY); headers.discardAll(MESSAGE_ENCODING_KEY); if (compressor == null) { compressor = Codec.Identity.NONE; diff --git a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java index 0e5e5f5059..ecf7a90b13 100644 --- a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java @@ -469,6 +469,15 @@ public class ClientCallImplTest { assertNull(m.get(GrpcUtil.MESSAGE_ENCODING_KEY)); } + @Test + public void prepareHeaders_ignoreContentLength() { + Metadata m = new Metadata(); + m.put(GrpcUtil.CONTENT_LENGTH_KEY, "123"); + ClientCallImpl.prepareHeaders(m, decompressorRegistry, Codec.Identity.NONE, false); + + assertNull(m.get(GrpcUtil.CONTENT_LENGTH_KEY)); + } + @Test public void prepareHeaders_acceptedMessageEncodingsAdded() { Metadata m = new Metadata(); diff --git a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java index ea49b94e8a..edf303a0bc 100644 --- a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java @@ -17,6 +17,7 @@ package io.grpc.internal; import static com.google.common.base.Charsets.UTF_8; +import static io.grpc.internal.GrpcUtil.CONTENT_LENGTH_KEY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -151,6 +152,16 @@ public class ServerCallImplTest { verify(stream).writeHeaders(headers); } + @Test + public void sendHeader_contentLengthDiscarded() { + Metadata headers = new Metadata(); + headers.put(CONTENT_LENGTH_KEY, "123"); + call.sendHeaders(headers); + + verify(stream).writeHeaders(headers); + assertNull(headers.get(CONTENT_LENGTH_KEY)); + } + @Test public void sendHeader_failsOnSecondCall() { call.sendHeaders(new Metadata());