Skip to content

Commit

Permalink
netty: Support pseudo headers in all GrpcHttp2RequestHeaders methods
Browse files Browse the repository at this point in the history 
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 grpc#8981
  • Loading branch information
@ejona86
ejona86 committed Mar 23, 2022
1 parent 1234fd3 commit 8bc9b3b
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 25 deletions.
61 changes: 36 additions & 25 deletions netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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
}
}

Expand Down Expand Up @@ -418,8 +421,12 @@ public CharSequence scheme() {
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);
Expand All @@ -431,8 +438,12 @@ public List<CharSequence> 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;
Expand Down
125 changes: 125 additions & 0 deletions netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<byte[]> key = Key.of("bytes-bin", BINARY_BYTE_MARSHALLER);
Expand Down

0 comments on commit 8bc9b3b

Please sign in to comment.