netty: Support pseudo headers in all GrpcHttp2RequestHeaders methods

The previous code assumed that only gRPC would be using these methods.
But twice now Netty has made a change (generally relating to security)
that used a method for pseudo headers that previously wasn't supported.
Let's stop the whack-a-mole and just implement them all.

This restores compatibility with Netty 4.1.75.Final. Fixes #8981

2 files changed, 161 insertions, 25 deletions

diff --git a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
index df7875fc7..4023fd121 100644
--- a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
+++ b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
@@ -340,7 +340,12 @@ class GrpcHttp2HeadersUtils {
       AsciiString name = validateName(requireAsciiString(csName));
       AsciiString value = requireAsciiString(csValue);
       if (isPseudoHeader(name)) {
-        addPseudoHeader(name, value);
+        AsciiString previous = getPseudoHeader(name);
+        if (previous != null) {
+          PlatformDependent.throwException(
+              connectionError(PROTOCOL_ERROR, "Duplicate %s header", name));
+        }
+        setPseudoHeader(name, value);
         return this;
       }
       if (equals(TE_HEADER, name)) {
@@ -353,44 +358,42 @@ class GrpcHttp2HeadersUtils {
     @Override
     public CharSequence get(CharSequence csName) {
       AsciiString name = requireAsciiString(csName);
-      checkArgument(!isPseudoHeader(name), "Use direct accessor methods for pseudo headers.");
+      if (isPseudoHeader(name)) {
+        return getPseudoHeader(name);
+      }
       if (equals(TE_HEADER, name)) {
         return te;
       }
       return get(name);
     }
 
-    private void addPseudoHeader(CharSequence csName, CharSequence csValue) {
-      AsciiString name = requireAsciiString(csName);
-      AsciiString value = requireAsciiString(csValue);
+    private AsciiString getPseudoHeader(AsciiString name) {
+      if (equals(PATH_HEADER, name)) {
+        return path;
+      } else if (equals(AUTHORITY_HEADER, name)) {
+        return authority;
+      } else if (equals(METHOD_HEADER, name)) {
+        return method;
+      } else if (equals(SCHEME_HEADER, name)) {
+        return scheme;
+      } else {
+        return null;
+      }
+    }
 
+    private void setPseudoHeader(AsciiString name, AsciiString value) {
       if (equals(PATH_HEADER, name)) {
-        if (path != null) {
-          PlatformDependent.throwException(
-              connectionError(PROTOCOL_ERROR, "Duplicate :path header"));
-        }
         path = value;
       } else if (equals(AUTHORITY_HEADER, name)) {
-        if (authority != null) {
-          PlatformDependent.throwException(
-              connectionError(PROTOCOL_ERROR, "Duplicate :authority header"));
-        }
         authority = value;
       } else if (equals(METHOD_HEADER, name)) {
-        if (method != null) {
-          PlatformDependent.throwException(
-              connectionError(PROTOCOL_ERROR, "Duplicate :method header"));
-        }
         method = value;
       } else if (equals(SCHEME_HEADER, name)) {
-        if (scheme != null) {
-          PlatformDependent.throwException(
-              connectionError(PROTOCOL_ERROR, "Duplicate :scheme header"));
-        }
         scheme = value;
       } else {
         PlatformDependent.throwException(
             connectionError(PROTOCOL_ERROR, "Illegal pseudo-header '%s' in request.", name));
+        throw new AssertionError(); // Make flow control obvious to javac
       }
     }
 
@@ -418,8 +421,12 @@ class GrpcHttp2HeadersUtils {
     public List<CharSequence> getAll(CharSequence csName) {
       AsciiString name = requireAsciiString(csName);
       if (isPseudoHeader(name)) {
-        // This code should never be reached.
-        throw new IllegalArgumentException("Use direct accessor methods for pseudo headers.");
+        AsciiString value = getPseudoHeader(name);
+        if (value == null) {
+          return Collections.emptyList();
+        } else {
+          return Collections.singletonList(value);
+        }
       }
       if (equals(TE_HEADER, name)) {
         return Collections.singletonList((CharSequence) te);
@@ -431,8 +438,12 @@ class GrpcHttp2HeadersUtils {
     public boolean remove(CharSequence csName) {
       AsciiString name = requireAsciiString(csName);
       if (isPseudoHeader(name)) {
-        // This code should never be reached.
-        throw new IllegalArgumentException("Use direct accessor methods for pseudo headers.");
+        if (getPseudoHeader(name) == null) {
+          return false;
+        } else {
+          setPseudoHeader(name, null);
+          return true;
+        }
       }
       if (equals(TE_HEADER, name)) {
         boolean wasPresent = te != null;
diff --git a/netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java b/netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java
index 11488b752..48c6320f4 100644
--- a/netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java
+++ b/netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java
@@ -22,6 +22,7 @@ import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE;
 import static io.netty.util.AsciiString.of;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.common.collect.Iterables;
 import com.google.common.io.BaseEncoding;
@@ -133,6 +134,130 @@ public class GrpcHttp2HeadersUtilsTest {
     assertThat(decodedHeaders.toString()).contains("[]");
   }
 
+  // contains() is used by Netty 4.1.75+. https://github.com/grpc/grpc-java/issues/8981
+  // Just implement everything pseudo headers for all methods; too many recent breakages.
+  @Test
+  public void grpcHttp2RequestHeaders_pseudoHeaders_notPresent() {
+    Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
+    assertThat(http2Headers.get(AsciiString.of(":path"))).isNull();
+    assertThat(http2Headers.get(AsciiString.of(":authority"))).isNull();
+    assertThat(http2Headers.get(AsciiString.of(":method"))).isNull();
+    assertThat(http2Headers.get(AsciiString.of(":scheme"))).isNull();
+    assertThat(http2Headers.get(AsciiString.of(":status"))).isNull();
+
+    assertThat(http2Headers.getAll(AsciiString.of(":path"))).isEmpty();
+    assertThat(http2Headers.getAll(AsciiString.of(":authority"))).isEmpty();
+    assertThat(http2Headers.getAll(AsciiString.of(":method"))).isEmpty();
+    assertThat(http2Headers.getAll(AsciiString.of(":scheme"))).isEmpty();
+    assertThat(http2Headers.getAll(AsciiString.of(":status"))).isEmpty();
+
+    assertThat(http2Headers.contains(AsciiString.of(":path"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":authority"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":method"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":scheme"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":status"))).isFalse();
+
+    assertThat(http2Headers.remove(AsciiString.of(":path"))).isFalse();
+    assertThat(http2Headers.remove(AsciiString.of(":authority"))).isFalse();
+    assertThat(http2Headers.remove(AsciiString.of(":method"))).isFalse();
+    assertThat(http2Headers.remove(AsciiString.of(":scheme"))).isFalse();
+    assertThat(http2Headers.remove(AsciiString.of(":status"))).isFalse();
+  }
+
+  @Test
+  public void grpcHttp2RequestHeaders_pseudoHeaders_present() {
+    Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
+    http2Headers.add(AsciiString.of(":path"), AsciiString.of("mypath"));
+    http2Headers.add(AsciiString.of(":authority"), AsciiString.of("myauthority"));
+    http2Headers.add(AsciiString.of(":method"), AsciiString.of("mymethod"));
+    http2Headers.add(AsciiString.of(":scheme"), AsciiString.of("myscheme"));
+
+    assertThat(http2Headers.get(AsciiString.of(":path"))).isEqualTo(AsciiString.of("mypath"));
+    assertThat(http2Headers.get(AsciiString.of(":authority")))
+        .isEqualTo(AsciiString.of("myauthority"));
+    assertThat(http2Headers.get(AsciiString.of(":method"))).isEqualTo(AsciiString.of("mymethod"));
+    assertThat(http2Headers.get(AsciiString.of(":scheme"))).isEqualTo(AsciiString.of("myscheme"));
+
+    assertThat(http2Headers.getAll(AsciiString.of(":path")))
+        .containsExactly(AsciiString.of("mypath"));
+    assertThat(http2Headers.getAll(AsciiString.of(":authority")))
+        .containsExactly(AsciiString.of("myauthority"));
+    assertThat(http2Headers.getAll(AsciiString.of(":method")))
+        .containsExactly(AsciiString.of("mymethod"));
+    assertThat(http2Headers.getAll(AsciiString.of(":scheme")))
+        .containsExactly(AsciiString.of("myscheme"));
+
+    assertThat(http2Headers.contains(AsciiString.of(":path"))).isTrue();
+    assertThat(http2Headers.contains(AsciiString.of(":authority"))).isTrue();
+    assertThat(http2Headers.contains(AsciiString.of(":method"))).isTrue();
+    assertThat(http2Headers.contains(AsciiString.of(":scheme"))).isTrue();
+
+    assertThat(http2Headers.remove(AsciiString.of(":path"))).isTrue();
+    assertThat(http2Headers.remove(AsciiString.of(":authority"))).isTrue();
+    assertThat(http2Headers.remove(AsciiString.of(":method"))).isTrue();
+    assertThat(http2Headers.remove(AsciiString.of(":scheme"))).isTrue();
+
+    assertThat(http2Headers.contains(AsciiString.of(":path"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":authority"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":method"))).isFalse();
+    assertThat(http2Headers.contains(AsciiString.of(":scheme"))).isFalse();
+  }
+
+  @Test
+  public void grpcHttp2RequestHeaders_pseudoHeaders_set() {
+    Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
+    http2Headers.set(AsciiString.of(":path"), AsciiString.of("mypath"));
+    http2Headers.set(AsciiString.of(":authority"), AsciiString.of("myauthority"));
+    http2Headers.set(AsciiString.of(":method"), AsciiString.of("mymethod"));
+    http2Headers.set(AsciiString.of(":scheme"), AsciiString.of("myscheme"));
+
+    assertThat(http2Headers.getAll(AsciiString.of(":path")))
+        .containsExactly(AsciiString.of("mypath"));
+    assertThat(http2Headers.getAll(AsciiString.of(":authority")))
+        .containsExactly(AsciiString.of("myauthority"));
+    assertThat(http2Headers.getAll(AsciiString.of(":method")))
+        .containsExactly(AsciiString.of("mymethod"));
+    assertThat(http2Headers.getAll(AsciiString.of(":scheme")))
+        .containsExactly(AsciiString.of("myscheme"));
+
+    http2Headers.set(AsciiString.of(":path"), AsciiString.of("mypath2"));
+    http2Headers.set(AsciiString.of(":authority"), AsciiString.of("myauthority2"));
+    http2Headers.set(AsciiString.of(":method"), AsciiString.of("mymethod2"));
+    http2Headers.set(AsciiString.of(":scheme"), AsciiString.of("myscheme2"));
+
+    assertThat(http2Headers.getAll(AsciiString.of(":path")))
+        .containsExactly(AsciiString.of("mypath2"));
+    assertThat(http2Headers.getAll(AsciiString.of(":authority")))
+        .containsExactly(AsciiString.of("myauthority2"));
+    assertThat(http2Headers.getAll(AsciiString.of(":method")))
+        .containsExactly(AsciiString.of("mymethod2"));
+    assertThat(http2Headers.getAll(AsciiString.of(":scheme")))
+        .containsExactly(AsciiString.of("myscheme2"));
+  }
+
+  @Test
+  public void grpcHttp2RequestHeaders_pseudoHeaders_addWhenPresent_throws() {
+    Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
+    http2Headers.add(AsciiString.of(":path"), AsciiString.of("mypath"));
+    try {
+      http2Headers.add(AsciiString.of(":path"), AsciiString.of("mypath2"));
+      fail("Expected exception");
+    } catch (Exception ex) {
+      // expected
+    }
+  }
+
+  @Test
+  public void grpcHttp2RequestHeaders_pseudoHeaders_addInvalid_throws() {
+    Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
+    try {
+      http2Headers.add(AsciiString.of(":status"), AsciiString.of("mystatus"));
+      fail("Expected exception");
+    } catch (Exception ex) {
+      // expected
+    }
+  }
+
   @Test
   public void dupBinHeadersWithComma() {
     Key<byte[]> key = Key.of("bytes-bin", BINARY_BYTE_MARSHALLER);