diff --git a/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java b/netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java index df7875fc7ae..4023fd1218f 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 @@ public Http2Headers add(CharSequence csName, CharSequence csValue) { 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 @@ public Http2Headers add(CharSequence csName, CharSequence csValue) { @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 @@ public CharSequence scheme() { public List 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 @@ public List getAll(CharSequence csName) { 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 11488b752f1..48c6320f4c6 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.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 void decode_emptyHeaders() throws Http2Exception { 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 key = Key.of("bytes-bin", BINARY_BYTE_MARSHALLER);