Skip to content

Commit 200100f

Browse files
umairk79AgraVator
authored andcommitted
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 68a6c72 commit 200100f

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
@@ -152,6 +152,9 @@ public class NettyClientTransportTest {
152152

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

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

@@ -625,9 +628,6 @@ public void maxHeaderListSizeShouldBeEnforcedOnClient() throws Exception {
625628

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

@@ -638,7 +638,7 @@ public void huffmanCodingShouldNotBePerformed() throws Exception {
638638

639639
Metadata headers = new Metadata();
640640
headers.put(Metadata.Key.of("test", Metadata.ASCII_STRING_MARSHALLER),
641-
longStringOfA);
641+
LONG_STRING_OF_A);
642642

643643
callMeMaybe(transport.start(clientTransportListener));
644644
verify(clientTransportListener, timeout(5000)).transportReady();
@@ -650,7 +650,7 @@ public void huffmanCodingShouldNotBePerformed() throws Exception {
650650
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
651651
throws Exception {
652652
if (msg instanceof ByteBuf) {
653-
if (((ByteBuf) msg).toString(StandardCharsets.UTF_8).contains(longStringOfA)) {
653+
if (((ByteBuf) msg).toString(StandardCharsets.UTF_8).contains(LONG_STRING_OF_A)) {
654654
foundExpectedHeaderBytes.set(true);
655655
}
656656
}
@@ -665,6 +665,47 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
665665
}
666666
}
667667

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

1160+
private void startServer(ServerListener serverListener) throws IOException {
1161+
startServer(100, GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE, serverListener);
1162+
}
1163+
11191164
private void startServer(int maxStreamsPerConnection, int maxHeaderListSize) throws IOException {
1165+
startServer(maxStreamsPerConnection, maxHeaderListSize, serverListener);
1166+
}
1167+
1168+
private void startServer(int maxStreamsPerConnection, int maxHeaderListSize,
1169+
ServerListener serverListener) throws IOException {
11201170
server =
11211171
new NettyServer(
11221172
TestUtils.testServerAddresses(new InetSocketAddress(0)),
@@ -1284,6 +1334,15 @@ private final class EchoServerListener implements ServerListener {
12841334
final List<NettyServerTransport> transports = new ArrayList<>();
12851335
final List<EchoServerStreamListener> streamListeners =
12861336
Collections.synchronizedList(new ArrayList<EchoServerStreamListener>());
1337+
Metadata responseHeaders;
1338+
1339+
public EchoServerListener() {
1340+
this(new Metadata());
1341+
}
1342+
1343+
public EchoServerListener(Metadata responseHeaders) {
1344+
this.responseHeaders = responseHeaders;
1345+
}
12871346

12881347
@Override
12891348
public ServerTransportListener transportCreated(final ServerTransport transport) {
@@ -1293,7 +1352,7 @@ public ServerTransportListener transportCreated(final ServerTransport transport)
12931352
public void streamCreated(ServerStream stream, String method, Metadata headers) {
12941353
EchoServerStreamListener listener = new EchoServerStreamListener(stream, headers);
12951354
stream.setListener(listener);
1296-
stream.writeHeaders(new Metadata(), true);
1355+
stream.writeHeaders(responseHeaders, true);
12971356
stream.request(1);
12981357
streamListeners.add(listener);
12991358
}

0 commit comments

Comments
 (0)