Skip to content

Commit 9f17470

Browse files
petedmarshpetedmarsh
committed
okhttp: Honor peer HPACK table-size settings
Apply SETTINGS_HEADER_TABLE_SIZE to the outbound HPACK writer before acknowledging it, so the next header block emits the required dynamic table size update. Stop applying the peer encoder setting to the inbound decoder. Add framed unit coverage and bidirectional OkHttp-Netty regression tests. The tests verify that repeated calls remain on one transport. AI assistance: OpenAI Codex (GPT-5) was used to implement and test this grpc-okhttp compatibility fix.
1 parent eec3174 commit 9f17470

4 files changed

Lines changed: 203 additions & 7 deletions

File tree

‎interop-testing/src/test/java/io/grpc/testing/integration/HpackDynamicTableInteropTest.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
* Copyright 2026 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.testing.integration;
18+
19+
import static com.google.common.truth.Truth.assertThat;
20+
21+
import io.grpc.Attributes;
22+
import io.grpc.InsecureServerCredentials;
23+
import io.grpc.ManagedChannel;
24+
import io.grpc.Metadata;
25+
import io.grpc.Server;
26+
import io.grpc.ServerBuilder;
27+
import io.grpc.ServerCall;
28+
import io.grpc.ServerCallHandler;
29+
import io.grpc.ServerInterceptor;
30+
import io.grpc.ServerInterceptors;
31+
import io.grpc.ServerTransportFilter;
32+
import io.grpc.netty.NettyChannelBuilder;
33+
import io.grpc.netty.NettyServerBuilder;
34+
import io.grpc.okhttp.OkHttpChannelBuilder;
35+
import io.grpc.okhttp.OkHttpServerBuilder;
36+
import io.grpc.stub.MetadataUtils;
37+
import io.grpc.stub.StreamObserver;
38+
import io.grpc.testing.GrpcCleanupRule;
39+
import java.util.concurrent.TimeUnit;
40+
import java.util.concurrent.atomic.AtomicInteger;
41+
import java.util.concurrent.atomic.AtomicReference;
42+
import org.junit.Rule;
43+
import org.junit.Test;
44+
import org.junit.runner.RunWith;
45+
import org.junit.runners.JUnit4;
46+
47+
/** Interoperability tests for disabling the HPACK dynamic table. */
48+
@RunWith(JUnit4.class)
49+
public final class HpackDynamicTableInteropTest {
50+
private static final int CALL_COUNT = 3;
51+
private static final String REQUEST_METADATA_VALUE = "repeated-request-metadata-value";
52+
private static final String RESPONSE_METADATA_VALUE = "repeated-response-metadata-value";
53+
private static final Metadata.Key<String> REQUEST_METADATA_KEY =
54+
Metadata.Key.of("hpack-request-metadata", Metadata.ASCII_STRING_MARSHALLER);
55+
private static final Metadata.Key<String> RESPONSE_METADATA_KEY =
56+
Metadata.Key.of("hpack-response-metadata", Metadata.ASCII_STRING_MARSHALLER);
57+
private static final EmptyProtos.Empty EMPTY = EmptyProtos.Empty.getDefaultInstance();
58+
59+
@Rule public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule();
60+
61+
private final AtomicInteger serverTransportCount = new AtomicInteger();
62+
private final AtomicInteger requestsWithExpectedMetadata = new AtomicInteger();
63+
64+
@Test
65+
public void defaultOkHttpClient_interoperatesWithDisabledNettyServer() throws Exception {
66+
Server server = startServer(
67+
NettyServerBuilder.forPort(0, InsecureServerCredentials.create())
68+
.disableHpackDynamicTable());
69+
ManagedChannel channel = grpcCleanup.register(
70+
OkHttpChannelBuilder.forAddress("localhost", server.getPort())
71+
.usePlaintext()
72+
.build());
73+
74+
makeRepeatedCalls(channel);
75+
}
76+
77+
@Test
78+
public void disabledNettyClient_interoperatesWithDefaultOkHttpServer() throws Exception {
79+
Server server = startServer(
80+
OkHttpServerBuilder.forPort(0, InsecureServerCredentials.create()));
81+
ManagedChannel channel = grpcCleanup.register(
82+
NettyChannelBuilder.forAddress("localhost", server.getPort())
83+
.usePlaintext()
84+
.disableHpackDynamicTable()
85+
.build());
86+
87+
makeRepeatedCalls(channel);
88+
}
89+
90+
private Server startServer(ServerBuilder<?> serverBuilder) throws Exception {
91+
Metadata responseMetadata = new Metadata();
92+
responseMetadata.put(RESPONSE_METADATA_KEY, RESPONSE_METADATA_VALUE);
93+
94+
Server server = serverBuilder
95+
.addTransportFilter(new ServerTransportFilter() {
96+
@Override
97+
public Attributes transportReady(Attributes transportAttrs) {
98+
serverTransportCount.incrementAndGet();
99+
return transportAttrs;
100+
}
101+
})
102+
.addService(ServerInterceptors.intercept(
103+
new TestService(),
104+
new ServerInterceptor() {
105+
@Override
106+
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
107+
ServerCall<ReqT, RespT> call,
108+
Metadata headers,
109+
ServerCallHandler<ReqT, RespT> next) {
110+
if (REQUEST_METADATA_VALUE.equals(headers.get(REQUEST_METADATA_KEY))) {
111+
requestsWithExpectedMetadata.incrementAndGet();
112+
}
113+
return next.startCall(call, headers);
114+
}
115+
},
116+
MetadataUtils.newAttachMetadataServerInterceptor(responseMetadata)))
117+
.build();
118+
return grpcCleanup.register(server).start();
119+
}
120+
121+
private void makeRepeatedCalls(ManagedChannel channel) {
122+
Metadata requestMetadata = new Metadata();
123+
requestMetadata.put(REQUEST_METADATA_KEY, REQUEST_METADATA_VALUE);
124+
AtomicReference<Metadata> responseHeaders = new AtomicReference<>();
125+
AtomicReference<Metadata> responseTrailers = new AtomicReference<>();
126+
TestServiceGrpc.TestServiceBlockingStub stub = TestServiceGrpc.newBlockingStub(channel)
127+
.withInterceptors(
128+
MetadataUtils.newAttachHeadersInterceptor(requestMetadata),
129+
MetadataUtils.newCaptureMetadataInterceptor(responseHeaders, responseTrailers));
130+
131+
for (int i = 0; i < CALL_COUNT; i++) {
132+
assertThat(stub.withDeadlineAfter(10, TimeUnit.SECONDS).emptyCall(EMPTY)).isEqualTo(EMPTY);
133+
assertThat(responseHeaders.get()).isNotNull();
134+
assertThat(responseHeaders.get().get(RESPONSE_METADATA_KEY))
135+
.isEqualTo(RESPONSE_METADATA_VALUE);
136+
}
137+
assertThat(requestsWithExpectedMetadata.get()).isEqualTo(CALL_COUNT);
138+
assertThat(serverTransportCount.get()).isEqualTo(1);
139+
}
140+
141+
private static final class TestService extends TestServiceGrpc.TestServiceImplBase {
142+
@Override
143+
public void emptyCall(
144+
EmptyProtos.Empty request, StreamObserver<EmptyProtos.Empty> responseObserver) {
145+
responseObserver.onNext(EMPTY);
146+
responseObserver.onCompleted();
147+
}
148+
}
149+
}

‎okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,11 @@ int maxDynamicTableByteCount() {
155155
}
156156

157157
/**
158-
* Called by the reader when the peer sent {@link Settings#HEADER_TABLE_SIZE}.
159-
* While this establishes the maximum dynamic table size, the
160-
* {@link #maxDynamicTableByteCount} set during processing may limit the
161-
* table size to a smaller amount.
158+
* Updates the limit for header blocks received from the peer. This corresponds to a
159+
* {@link Settings#HEADER_TABLE_SIZE} advertised by the local endpoint, not one received from
160+
* the peer. While this establishes the maximum dynamic table size, the
161+
* {@link #maxDynamicTableByteCount} set during processing may limit the table size to a smaller
162+
* amount.
162163
* <p> Evicts entries or clears the table as needed.
163164
*/
164165
void headerTableSizeSetting(int headerTableSizeSetting) {

‎okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,6 @@ private void readSettings(Handler handler, int length, byte flags, int streamId)
312312
settings.set(id, 0, value);
313313
}
314314
handler.settings(false, settings);
315-
if (settings.getHeaderTableSize() >= 0) {
316-
hpackReader.headerTableSizeSetting(settings.getHeaderTableSize());
317-
}
318315
}
319316

320317
private void readPushPromise(Handler handler, int length, byte flags, int streamId)
@@ -397,6 +394,10 @@ static final class Writer implements io.grpc.okhttp.internal.framed.FrameWriter
397394
@Override public synchronized void ackSettings(io.grpc.okhttp.internal.framed.Settings peerSettings) throws IOException {
398395
if (closed) throw new IOException("closed");
399396
this.maxFrameSize = peerSettings.getMaxFrameSize(maxFrameSize);
397+
int headerTableSize = peerSettings.getHeaderTableSize();
398+
if (headerTableSize >= 0) {
399+
hpackWriter.resizeHeaderTable(headerTableSize);
400+
}
400401
int length = 0;
401402
byte type = TYPE_SETTINGS;
402403
byte flags = FLAG_ACK;

‎okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/Http2Test.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static io.grpc.okhttp.internal.framed.Http2.FLAG_PADDED;
2121
import static io.grpc.okhttp.internal.framed.Http2.TYPE_DATA;
2222
import static org.junit.Assert.assertEquals;
23+
import static org.junit.Assert.assertTrue;
2324
import static org.mockito.ArgumentMatchers.eq;
2425
import static org.mockito.Mockito.verify;
2526

@@ -75,6 +76,50 @@ public void dataFramePadding() throws IOException {
7576
assertEquals(2037 - 125, bufferIn.size());
7677
}
7778

79+
@Test
80+
public void ackSettingsHeaderTableSizeZeroUpdatesWriter() throws IOException {
81+
Buffer sink = new Buffer();
82+
Http2.Writer writer = new Http2.Writer(sink, true);
83+
Settings settings = new Settings().set(Settings.HEADER_TABLE_SIZE, 0, 0);
84+
85+
writer.ackSettings(settings);
86+
assertEquals(9, sink.size());
87+
sink.skip(9); // SETTINGS ACK frame.
88+
89+
writer.headers(false, 3, Arrays.asList(new Header("custom-key", "custom-value")));
90+
sink.skip(9); // HEADERS frame header.
91+
92+
assertEquals(0x20, sink.readByte() & 0xff); // Dynamic table size update to zero.
93+
}
94+
95+
@Test
96+
public void ackSettingsWithoutHeaderTableSizeDoesNotUpdateWriter() throws IOException {
97+
Buffer sink = new Buffer();
98+
Http2.Writer writer = new Http2.Writer(sink, true);
99+
100+
writer.ackSettings(new Settings());
101+
assertEquals(9, sink.size());
102+
sink.skip(9); // SETTINGS ACK frame.
103+
104+
writer.headers(false, 3, Arrays.asList(new Header("custom-key", "custom-value")));
105+
sink.skip(9); // HEADERS frame header.
106+
107+
assertEquals(0x40, sink.readByte() & 0xff); // Literal with incremental indexing.
108+
}
109+
110+
@Test
111+
public void peerHeaderTableSizeDoesNotChangeInboundDecoder() throws IOException {
112+
Buffer frames = new Buffer();
113+
Http2.Writer peerWriter = new Http2.Writer(frames, false);
114+
peerWriter.settings(new Settings().set(Settings.HEADER_TABLE_SIZE, 0, 0));
115+
Http2.Reader reader = new Http2.Reader(frames, 4096, true);
116+
117+
assertTrue(reader.nextFrame(mockHandler));
118+
119+
// The peer's setting limits our encoder; it does not limit decoding the peer's headers.
120+
assertEquals(4096, reader.hpackReader.maxDynamicTableByteCount());
121+
}
122+
78123
private Buffer createData(int flag, int length, int paddingLength) throws IOException {
79124
Buffer sink = new Buffer();
80125
writeLength(sink, length);

0 commit comments

Comments
 (0)