diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index 7da3783771..c6899e3553 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -161,6 +161,7 @@ public class OkHttpChannelBuilder extends private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS; private int flowControlWindow = DEFAULT_FLOW_CONTROL_WINDOW; private boolean keepAliveWithoutCalls; + private int maxInboundMetadataSize = Integer.MAX_VALUE; protected OkHttpChannelBuilder(String host, int port) { this(GrpcUtil.authorityFromHostAndPort(host, port)); @@ -405,6 +406,25 @@ public class OkHttpChannelBuilder extends return this; } + /** + * Sets the maximum size of metadata allowed to be received. {@code Integer.MAX_VALUE} disables + * the enforcement. Defaults to no limit ({@code Integer.MAX_VALUE}). + * + *

The implementation does not currently limit memory usage; this value is checked only after + * the metadata is decoded from the wire. It does prevent large metadata from being passed to the + * application. + * + * @param bytes the maximum size of received metadata + * @return this + * @throws IllegalArgumentException if bytes is non-positive + * @since 1.17.0 + */ + public OkHttpChannelBuilder maxInboundMetadataSize(int bytes) { + Preconditions.checkArgument(bytes > 0, "maxInboundMetadataSize must be > 0"); + this.maxInboundMetadataSize = bytes; + return this; + } + @Override @Internal protected final ClientTransportFactory buildTransportFactory() { @@ -412,7 +432,7 @@ public class OkHttpChannelBuilder extends return new OkHttpTransportFactory(transportExecutor, scheduledExecutorService, createSocketFactory(), hostnameVerifier, connectionSpec, maxInboundMessageSize(), enableKeepAlive, keepAliveTimeNanos, keepAliveTimeoutNanos, flowControlWindow, - keepAliveWithoutCalls, transportTracerFactory); + keepAliveWithoutCalls, maxInboundMetadataSize, transportTracerFactory); } @Override @@ -491,6 +511,7 @@ public class OkHttpChannelBuilder extends private final long keepAliveTimeoutNanos; private final int flowControlWindow; private final boolean keepAliveWithoutCalls; + private final int maxInboundMetadataSize; private final ScheduledExecutorService timeoutService; private boolean closed; @@ -505,6 +526,7 @@ public class OkHttpChannelBuilder extends long keepAliveTimeoutNanos, int flowControlWindow, boolean keepAliveWithoutCalls, + int maxInboundMetadataSize, TransportTracer.Factory transportTracerFactory) { usingSharedScheduler = timeoutService == null; this.timeoutService = usingSharedScheduler @@ -518,6 +540,7 @@ public class OkHttpChannelBuilder extends this.keepAliveTimeoutNanos = keepAliveTimeoutNanos; this.flowControlWindow = flowControlWindow; this.keepAliveWithoutCalls = keepAliveWithoutCalls; + this.maxInboundMetadataSize = maxInboundMetadataSize; usingSharedExecutor = executor == null; this.transportTracerFactory = @@ -556,6 +579,7 @@ public class OkHttpChannelBuilder extends flowControlWindow, options.getProxyParameters(), tooManyPingsRunnable, + maxInboundMetadataSize, transportTracerFactory.create()); if (enableKeepAlive) { transport.enableKeepAlive( diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 91897474cb..9c4ec18f79 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -192,6 +192,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep private long keepAliveTimeoutNanos; private boolean keepAliveWithoutCalls; private final Runnable tooManyPingsRunnable; + private final int maxInboundMetadataSize; @GuardedBy("lock") private final TransportTracer transportTracer; @GuardedBy("lock") @@ -223,7 +224,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep Executor executor, @Nullable SSLSocketFactory sslSocketFactory, @Nullable HostnameVerifier hostnameVerifier, ConnectionSpec connectionSpec, int maxMessageSize, int initialWindowSize, @Nullable ProxyParameters proxy, - Runnable tooManyPingsRunnable, TransportTracer transportTracer) { + Runnable tooManyPingsRunnable, int maxInboundMetadataSize, TransportTracer transportTracer) { this.address = Preconditions.checkNotNull(address, "address"); this.defaultAuthority = authority; this.maxMessageSize = maxMessageSize; @@ -241,6 +242,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep this.proxy = proxy; this.tooManyPingsRunnable = Preconditions.checkNotNull(tooManyPingsRunnable, "tooManyPingsRunnable"); + this.maxInboundMetadataSize = maxInboundMetadataSize; this.transportTracer = Preconditions.checkNotNull(transportTracer); initTransportTracer(); } @@ -281,6 +283,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep this.proxy = null; this.tooManyPingsRunnable = Preconditions.checkNotNull(tooManyPingsRunnable, "tooManyPingsRunnable"); + this.maxInboundMetadataSize = Integer.MAX_VALUE; this.transportTracer = Preconditions.checkNotNull(transportTracer, "transportTracer"); initTransportTracer(); } @@ -1067,6 +1070,18 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep List

headerBlock, HeadersMode headersMode) { boolean unknownStream = false; + Status failedStatus = null; + if (maxInboundMetadataSize != Integer.MAX_VALUE) { + int metadataSize = headerBlockSize(headerBlock); + if (metadataSize > maxInboundMetadataSize) { + failedStatus = Status.RESOURCE_EXHAUSTED.withDescription( + String.format( + "Response %s metadata larger than %d: %d", + inFinished ? "trailer" : "header", + maxInboundMetadataSize, + metadataSize)); + } + } synchronized (lock) { OkHttpClientStream stream = streams.get(streamId); if (stream == null) { @@ -1076,7 +1091,14 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep unknownStream = true; } } else { - stream.transportState().transportHeadersReceived(headerBlock, inFinished); + if (failedStatus == null) { + stream.transportState().transportHeadersReceived(headerBlock, inFinished); + } else { + if (!inFinished) { + frameWriter.rstStream(streamId, ErrorCode.CANCEL); + } + stream.transportState().transportReportStatus(failedStatus, false, new Metadata()); + } } } if (unknownStream) { @@ -1085,6 +1107,17 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep } } + private int headerBlockSize(List
headerBlock) { + // Calculate as defined for SETTINGS_MAX_HEADER_LIST_SIZE in RFC 7540 ยง6.5.2. + long size = 0; + for (int i = 0; i < headerBlock.size(); i++) { + Header header = headerBlock.get(i); + size += 32 + header.name.size() + header.value.size(); + } + size = Math.min(size, Integer.MAX_VALUE); + return (int) size; + } + @Override public void rstStream(int streamId, ErrorCode errorCode) { Status status = toGrpcStatus(errorCode).augmentDescription("Rst Stream"); diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index 620c34ec01..c2a7b60339 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -135,6 +135,7 @@ public class OkHttpClientTransportTest { private static final String NO_USER = null; private static final String NO_PW = null; private static final int DEFAULT_START_STREAM_ID = 3; + private static final int DEFAULT_MAX_INBOUND_METADATA_SIZE = Integer.MAX_VALUE; @Rule public final Timeout globalTimeout = Timeout.seconds(10); @@ -245,6 +246,7 @@ public class OkHttpClientTransportTest { INITIAL_WINDOW_SIZE, NO_PROXY, tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, transportTracer); String s = clientTransport.toString(); assertTrue("Unexpected: " + s, s.contains("OkHttpClientTransport")); @@ -1518,6 +1520,7 @@ public class OkHttpClientTransportTest { INITIAL_WINDOW_SIZE, NO_PROXY, tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, transportTracer); String host = clientTransport.getOverridenHost(); @@ -1541,6 +1544,7 @@ public class OkHttpClientTransportTest { INITIAL_WINDOW_SIZE, NO_PROXY, tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, new TransportTracer()); ManagedClientTransport.Listener listener = mock(ManagedClientTransport.Listener.class); @@ -1573,6 +1577,7 @@ public class OkHttpClientTransportTest { new ProxyParameters( (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW), tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, transportTracer); clientTransport.start(transportListener); @@ -1624,6 +1629,7 @@ public class OkHttpClientTransportTest { new ProxyParameters( (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW), tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, transportTracer); clientTransport.start(transportListener); @@ -1674,6 +1680,7 @@ public class OkHttpClientTransportTest { new ProxyParameters( (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW), tooManyPingsRunnable, + DEFAULT_MAX_INBOUND_METADATA_SIZE, transportTracer); clientTransport.start(transportListener); diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java index 9e4177f706..02f864b4df 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java @@ -20,6 +20,7 @@ import io.grpc.ServerStreamTracer; import io.grpc.internal.AccessProtectedHack; import io.grpc.internal.ClientTransportFactory; import io.grpc.internal.FakeClock; +import io.grpc.internal.GrpcUtil; import io.grpc.internal.InternalServer; import io.grpc.internal.ManagedClientTransport; import io.grpc.internal.testing.AbstractTransportTest; @@ -41,6 +42,7 @@ public class OkHttpTransportTest extends AbstractTransportTest { .forAddress("localhost", 0) .usePlaintext() .setTransportTracerFactory(fakeClockTransportTracer) + .maxInboundMetadataSize(GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE) .buildTransportFactory(); @After @@ -99,15 +101,10 @@ public class OkHttpTransportTest extends AbstractTransportTest { return true; } - // not yet implemented @Override @org.junit.Test @org.junit.Ignore - public void clientChecksInboundMetadataSize_header() {} - - // not yet implemented - @Override - @org.junit.Test - @org.junit.Ignore - public void clientChecksInboundMetadataSize_trailer() {} + public void clientChecksInboundMetadataSize_trailer() { + // Server-side is flaky due to https://github.com/netty/netty/pull/8332 + } }