Skip to content

Commit 6591220

Browse files
authored
netty: Include pseudo-headers in GrpcHttp2RequestHeaders.iterator() (v1.83.x backport) (#13122)
Backport of #13118 to v1.83.x. --- fixes b/556157371
1 parent f391e41 commit 6591220

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;
@@ -764,6 +765,71 @@ public void headersWithOnlyHostBecomesAuthority() throws Exception {
764765
Truth.assertThat(metadataCaptor.getValue().get(hostKey)).isNull();
765766
}
766767

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

0 commit comments

Comments
 (0)