Skip to content

Commit 6fae71b

Browse files
authored
netty: disable Huffman coding in server response headers (#12357)
Follow up to PR #10563 Previously, we disabled Huffman encoding on the `NettyClientHandler` to improve header encoding performance. This change also ensures consistency in the header encoding strategy across both client and server components.
1 parent c1f3287 commit 6fae71b

File tree

2 files changed

+70
-7
lines changed

2 files changed

+70
-7
lines changed

netty/src/main/java/io/grpc/netty/NettyServerHandler.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
import io.netty.handler.codec.http2.DefaultHttp2FrameReader;
7070
import io.netty.handler.codec.http2.DefaultHttp2FrameWriter;
7171
import io.netty.handler.codec.http2.DefaultHttp2Headers;
72+
import io.netty.handler.codec.http2.DefaultHttp2HeadersEncoder;
7273
import io.netty.handler.codec.http2.DefaultHttp2LocalFlowController;
7374
import io.netty.handler.codec.http2.DefaultHttp2RemoteFlowController;
7475
import io.netty.handler.codec.http2.EmptyHttp2Headers;
@@ -85,6 +86,7 @@
8586
import io.netty.handler.codec.http2.Http2FrameWriter;
8687
import io.netty.handler.codec.http2.Http2Headers;
8788
import io.netty.handler.codec.http2.Http2HeadersDecoder;
89+
import io.netty.handler.codec.http2.Http2HeadersEncoder;
8890
import io.netty.handler.codec.http2.Http2InboundFrameLogger;
8991
import io.netty.handler.codec.http2.Http2LifecycleManager;
9092
import io.netty.handler.codec.http2.Http2OutboundFrameLogger;
@@ -179,8 +181,10 @@ static NettyServerHandler newHandler(
179181
Http2HeadersDecoder headersDecoder = new GrpcHttp2ServerHeadersDecoder(maxHeaderListSize);
180182
Http2FrameReader frameReader = new Http2InboundFrameLogger(
181183
new DefaultHttp2FrameReader(headersDecoder), frameLogger);
184+
Http2HeadersEncoder encoder = new DefaultHttp2HeadersEncoder(
185+
Http2HeadersEncoder.NEVER_SENSITIVE, false, 16, Integer.MAX_VALUE);
182186
Http2FrameWriter frameWriter =
183-
new Http2OutboundFrameLogger(new DefaultHttp2FrameWriter(), frameLogger);
187+
new Http2OutboundFrameLogger(new DefaultHttp2FrameWriter(encoder), frameLogger);
184188
return newHandler(
185189
channelUnused,
186190
frameReader,

netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ public class NettyClientTransportTest {
151151

152152
private static final SslContext SSL_CONTEXT = createSslContext();
153153

154+
@SuppressWarnings("InlineMeInliner") // Requires Java 11
155+
private static final String LONG_STRING_OF_A = Strings.repeat("a", 128);
156+
154157
@Mock
155158
private ManagedClientTransport.Listener clientTransportListener;
156159

@@ -624,9 +627,6 @@ public void maxHeaderListSizeShouldBeEnforcedOnClient() throws Exception {
624627

625628
@Test
626629
public void huffmanCodingShouldNotBePerformed() throws Exception {
627-
@SuppressWarnings("InlineMeInliner") // Requires Java 11
628-
String longStringOfA = Strings.repeat("a", 128);
629-
630630
negotiator = ProtocolNegotiators.serverPlaintext();
631631
startServer();
632632

@@ -637,7 +637,7 @@ public void huffmanCodingShouldNotBePerformed() throws Exception {
637637

638638
Metadata headers = new Metadata();
639639
headers.put(Metadata.Key.of("test", Metadata.ASCII_STRING_MARSHALLER),
640-
longStringOfA);
640+
LONG_STRING_OF_A);
641641

642642
callMeMaybe(transport.start(clientTransportListener));
643643
verify(clientTransportListener, timeout(5000)).transportReady();
@@ -649,7 +649,7 @@ public void huffmanCodingShouldNotBePerformed() throws Exception {
649649
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
650650
throws Exception {
651651
if (msg instanceof ByteBuf) {
652-
if (((ByteBuf) msg).toString(StandardCharsets.UTF_8).contains(longStringOfA)) {
652+
if (((ByteBuf) msg).toString(StandardCharsets.UTF_8).contains(LONG_STRING_OF_A)) {
653653
foundExpectedHeaderBytes.set(true);
654654
}
655655
}
@@ -664,6 +664,47 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
664664
}
665665
}
666666

667+
@Test
668+
public void huffmanCodingShouldNotBePerformedOnServer() throws Exception {
669+
negotiator = ProtocolNegotiators.serverPlaintext();
670+
671+
Metadata responseHeaders = new Metadata();
672+
responseHeaders.put(Metadata.Key.of("test", Metadata.ASCII_STRING_MARSHALLER),
673+
LONG_STRING_OF_A);
674+
675+
startServer(new EchoServerListener(responseHeaders));
676+
677+
NettyClientTransport transport = newTransport(ProtocolNegotiators.plaintext(),
678+
DEFAULT_MAX_MESSAGE_SIZE, GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE, null, false,
679+
TimeUnit.SECONDS.toNanos(10L), TimeUnit.SECONDS.toNanos(1L),
680+
new ReflectiveChannelFactory<>(NioSocketChannel.class), group);
681+
682+
callMeMaybe(transport.start(clientTransportListener));
683+
verify(clientTransportListener, timeout(5000)).transportReady();
684+
685+
AtomicBoolean foundExpectedHeaderBytes = new AtomicBoolean(false);
686+
687+
// Add a handler to the client pipeline to inspect server's response
688+
transport.channel().pipeline().addFirst(new ChannelDuplexHandler() {
689+
@Override
690+
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
691+
if (msg instanceof ByteBuf) {
692+
String data = ((ByteBuf) msg).toString(StandardCharsets.UTF_8);
693+
if (data.contains(LONG_STRING_OF_A)) {
694+
foundExpectedHeaderBytes.set(true);
695+
}
696+
}
697+
super.channelRead(ctx, msg);
698+
}
699+
});
700+
701+
new Rpc(transport).halfClose().waitForResponse();
702+
703+
if (!foundExpectedHeaderBytes.get()) {
704+
fail("expected to find UTF-8 encoded 'a's in the response header sent by the server");
705+
}
706+
}
707+
667708
@Test
668709
public void maxHeaderListSizeShouldBeEnforcedOnServer() throws Exception {
669710
startServer(100, 1);
@@ -1115,7 +1156,16 @@ private void startServer() throws IOException {
11151156
startServer(100, GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE);
11161157
}
11171158

1159+
private void startServer(ServerListener serverListener) throws IOException {
1160+
startServer(100, GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE, serverListener);
1161+
}
1162+
11181163
private void startServer(int maxStreamsPerConnection, int maxHeaderListSize) throws IOException {
1164+
startServer(maxStreamsPerConnection, maxHeaderListSize, serverListener);
1165+
}
1166+
1167+
private void startServer(int maxStreamsPerConnection, int maxHeaderListSize,
1168+
ServerListener serverListener) throws IOException {
11191169
server =
11201170
new NettyServer(
11211171
TestUtils.testServerAddresses(new InetSocketAddress(0)),
@@ -1283,6 +1333,15 @@ private final class EchoServerListener implements ServerListener {
12831333
final List<NettyServerTransport> transports = new ArrayList<>();
12841334
final List<EchoServerStreamListener> streamListeners =
12851335
Collections.synchronizedList(new ArrayList<EchoServerStreamListener>());
1336+
Metadata responseHeaders;
1337+
1338+
public EchoServerListener() {
1339+
this(new Metadata());
1340+
}
1341+
1342+
public EchoServerListener(Metadata responseHeaders) {
1343+
this.responseHeaders = responseHeaders;
1344+
}
12861345

12871346
@Override
12881347
public ServerTransportListener transportCreated(final ServerTransport transport) {
@@ -1292,7 +1351,7 @@ public ServerTransportListener transportCreated(final ServerTransport transport)
12921351
public void streamCreated(ServerStream stream, String method, Metadata headers) {
12931352
EchoServerStreamListener listener = new EchoServerStreamListener(stream, headers);
12941353
stream.setListener(listener);
1295-
stream.writeHeaders(new Metadata(), true);
1354+
stream.writeHeaders(responseHeaders, true);
12961355
stream.request(1);
12971356
streamListeners.add(listener);
12981357
}

0 commit comments

Comments
 (0)