Skip to content

Commit 472bf9b

Browse files
authored
netty: Include pseudo-headers in GrpcHttp2RequestHeaders.iterator() (v1.81.x backport) (#13124)
Backport of #13118 to v1.81.x. --- fixes b/556157371
1 parent c68b9c1 commit 472bf9b

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
@@ -80,6 +80,7 @@
8080
import io.netty.channel.ChannelHandlerContext;
8181
import io.netty.channel.ChannelPromise;
8282
import io.netty.handler.codec.http.HttpHeaderNames;
83+
import io.netty.handler.codec.http2.DefaultHttp2FrameWriter;
8384
import io.netty.handler.codec.http2.DefaultHttp2Headers;
8485
import io.netty.handler.codec.http2.Http2CodecUtil;
8586
import io.netty.handler.codec.http2.Http2Error;
@@ -756,6 +757,71 @@ public void headersWithOnlyHostBecomesAuthority() throws Exception {
756757
Truth.assertThat(metadataCaptor.getValue().get(hostKey)).isNull();
757758
}
758759

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

0 commit comments

Comments
 (0)