Skip to content

Commit e646a86

Browse files
authored
netty: Include pseudo-headers in GrpcHttp2RequestHeaders.iterator() (v1.79.x backport) (#13126)
Backport of #13118 to v1.79.x. --- fixes b/556157371
1 parent 4d5ccfc commit e646a86

3 files changed

Lines changed: 129 additions & 1 deletion

File tree

‎netty/src/main/java/io/grpc/netty/GrpcHttp2HeadersUtils.java‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,13 +306,18 @@ protected static void appendNameAndValue(StringBuilder builder, CharSequence nam
306306

307307
private List<Map.Entry<CharSequence, CharSequence>> namesAndValuesToImmutableList() {
308308
ArrayList<Map.Entry<CharSequence, CharSequence>> list = new ArrayList<>(values.length);
309+
addNamesAndValuesToList(list);
310+
return Collections.unmodifiableList(list);
311+
}
312+
313+
protected final void addNamesAndValuesToList(
314+
List<Map.Entry<CharSequence, CharSequence>> list) {
309315
for (int i = 0; i < namesAndValuesIdx; i += 2) {
310316
String name = new String(namesAndValues[i], US_ASCII);
311317
// If binary headers, the value is base64 encoded.
312318
AsciiString value = values[i / 2];
313319
list.add(new AbstractMap.SimpleImmutableEntry<CharSequence, CharSequence>(name, value));
314320
}
315-
return Collections.unmodifiableList(list);
316321
}
317322

318323
protected final String namesAndValuesToString() {
@@ -494,6 +499,27 @@ public int size() {
494499
return size;
495500
}
496501

502+
@Override
503+
public Iterator<Map.Entry<CharSequence, CharSequence>> iterator() {
504+
List<Map.Entry<CharSequence, CharSequence>> list = new ArrayList<>(size());
505+
addIfPresent(list, PATH_HEADER, path);
506+
addIfPresent(list, AUTHORITY_HEADER, authority);
507+
addIfPresent(list, METHOD_HEADER, method);
508+
addIfPresent(list, SCHEME_HEADER, scheme);
509+
addIfPresent(list, TE_HEADER, te);
510+
addNamesAndValuesToList(list);
511+
return Collections.unmodifiableList(list).iterator();
512+
}
513+
514+
private static void addIfPresent(
515+
List<Map.Entry<CharSequence, CharSequence>> list, AsciiString name, AsciiString value) {
516+
if (value != null) {
517+
list.add(
518+
new AbstractMap.SimpleImmutableEntry<CharSequence, CharSequence>(
519+
name.toString(), value));
520+
}
521+
}
522+
497523
@Override
498524
public String toString() {
499525
StringBuilder builder = new StringBuilder(getClass().getSimpleName()).append('[');

‎netty/src/test/java/io/grpc/netty/GrpcHttp2HeadersUtilsTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,42 @@ public void headerIteartor_nonEmpty() {
328328
assertThat(it.hasNext()).isFalse();
329329
}
330330

331+
@Test
332+
public void headerIterator_withPseudoHeadersAndTe() {
333+
Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
334+
http2Headers.add(AsciiString.of(":path"), AsciiString.of("mypath"));
335+
http2Headers.add(AsciiString.of(":authority"), AsciiString.of("myauthority"));
336+
http2Headers.add(AsciiString.of(":method"), AsciiString.of("mymethod"));
337+
http2Headers.add(AsciiString.of(":scheme"), AsciiString.of("myscheme"));
338+
http2Headers.add(AsciiString.of("te"), AsciiString.of("trailers"));
339+
http2Headers.add(AsciiString.of("custom"), AsciiString.of("val"));
340+
Iterator<Map.Entry<CharSequence, CharSequence>> it = http2Headers.iterator();
341+
342+
assertNextEntry(it, ":path", AsciiString.of("mypath"));
343+
assertNextEntry(it, ":authority", AsciiString.of("myauthority"));
344+
assertNextEntry(it, ":method", AsciiString.of("mymethod"));
345+
assertNextEntry(it, ":scheme", AsciiString.of("myscheme"));
346+
assertNextEntry(it, "te", AsciiString.of("trailers"));
347+
assertNextEntry(it, "custom", AsciiString.of("val"));
348+
assertThat(it.hasNext()).isFalse();
349+
}
350+
351+
@Test
352+
public void headerIterator_nullPseudoHeadersSkipped() {
353+
Http2Headers http2Headers = new GrpcHttp2RequestHeaders(2);
354+
http2Headers.add(AsciiString.of(":path"), AsciiString.of("mypath"));
355+
http2Headers.add(AsciiString.of(":authority"), AsciiString.of("myauthority"));
356+
http2Headers.remove(AsciiString.of(":authority"));
357+
http2Headers.add(AsciiString.of(":scheme"), AsciiString.of("myscheme"));
358+
http2Headers.add(AsciiString.of("custom"), AsciiString.of("val"));
359+
Iterator<Map.Entry<CharSequence, CharSequence>> it = http2Headers.iterator();
360+
361+
assertNextEntry(it, ":path", AsciiString.of("mypath"));
362+
assertNextEntry(it, ":scheme", AsciiString.of("myscheme"));
363+
assertNextEntry(it, "custom", AsciiString.of("val"));
364+
assertThat(it.hasNext()).isFalse();
365+
}
366+
331367
private static void assertNextEntry(
332368
Iterator<Map.Entry<CharSequence, CharSequence>> it, CharSequence key, CharSequence value) {
333369
Map.Entry<CharSequence, CharSequence> entry = it.next();

‎netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import io.netty.channel.ChannelHandlerContext;
8080
import io.netty.channel.ChannelPromise;
8181
import io.netty.handler.codec.http.HttpHeaderNames;
82+
import io.netty.handler.codec.http2.DefaultHttp2FrameWriter;
8283
import io.netty.handler.codec.http2.DefaultHttp2Headers;
8384
import io.netty.handler.codec.http2.Http2CodecUtil;
8485
import io.netty.handler.codec.http2.Http2Error;
@@ -742,6 +743,71 @@ public void headersWithOnlyHostBecomesAuthority() throws Exception {
742743
Truth.assertThat(metadataCaptor.getValue().get(hostKey)).isNull();
743744
}
744745

746+
@Test
747+
public void trailersWithPseudoHeadersShouldFail() throws Exception {
748+
manualSetUp();
749+
createStream();
750+
751+
Http2Headers trailers = new DefaultHttp2Headers()
752+
.method(HTTP_METHOD)
753+
.set(CONTENT_TYPE_HEADER, CONTENT_TYPE_GRPC)
754+
.set(TE_HEADER, TE_TRAILERS)
755+
.path(new AsciiString("/smuggled/method"));
756+
channelRead(trailersFrame(STREAM_ID, trailers));
757+
758+
verifyWrite()
759+
.writeRstStream(
760+
eq(ctx()),
761+
eq(STREAM_ID),
762+
eq(Http2Error.PROTOCOL_ERROR.code()),
763+
any(ChannelPromise.class));
764+
verify(transportListener, times(1))
765+
.streamCreated(any(ServerStream.class), anyString(), any(Metadata.class));
766+
ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
767+
verify(streamListener).closed(captor.capture());
768+
assertEquals(Code.INTERNAL, captor.getValue().getCode());
769+
verify(streamListener, never()).halfClosed();
770+
}
771+
772+
@Test
773+
public void trailersWithPseudoHeadersAfterRejectedHeadersShouldFail() throws Exception {
774+
manualSetUp();
775+
Http2Headers badHeaders = new DefaultHttp2Headers()
776+
.method(HTTP_METHOD)
777+
.path(new AsciiString("/foo/bar"));
778+
channelRead(headersFrame(STREAM_ID, badHeaders));
779+
verifyWrite()
780+
.writeHeaders(
781+
eq(ctx()),
782+
eq(STREAM_ID),
783+
any(Http2Headers.class),
784+
eq(0),
785+
eq(false),
786+
any(ChannelPromise.class));
787+
788+
Http2Headers trailers = new DefaultHttp2Headers()
789+
.method(HTTP_METHOD)
790+
.set(CONTENT_TYPE_HEADER, CONTENT_TYPE_GRPC)
791+
.set(TE_HEADER, TE_TRAILERS)
792+
.path(new AsciiString("/smuggled/method"));
793+
channelRead(trailersFrame(STREAM_ID, trailers));
794+
795+
verifyWrite()
796+
.writeRstStream(
797+
eq(ctx()),
798+
eq(STREAM_ID),
799+
eq(Http2Error.PROTOCOL_ERROR.code()),
800+
any(ChannelPromise.class));
801+
verify(transportListener, never())
802+
.streamCreated(any(ServerStream.class), anyString(), any(Metadata.class));
803+
}
804+
805+
private ByteBuf trailersFrame(int streamId, Http2Headers headers) {
806+
ChannelHandlerContext ctx = newMockContext();
807+
new DefaultHttp2FrameWriter().writeHeaders(ctx, streamId, headers, 0, true, newPromise());
808+
return captureWrite(ctx);
809+
}
810+
745811
@Test
746812
public void keepAliveManagerOnDataReceived_headersRead() throws Exception {
747813
manualSetUp();

0 commit comments

Comments
 (0)