From 9ada30b25dc183edac14eb0b1d57a1b157aa98be Mon Sep 17 00:00:00 2001 From: zpencer Date: Fri, 27 Apr 2018 16:01:16 -0700 Subject: [PATCH] (low priority) core,netty,interop-testing: stabilize maxInboundMessageSize API (#4399) On server side, `maxMessageSize` is deprecated for `maxInboundMessageSize` to match the channel builder. Update usages to use new setter. --- .../java/io/grpc/ManagedChannelBuilder.java | 11 ++++--- core/src/main/java/io/grpc/ServerBuilder.java | 31 +++++++++++++++++++ .../integration/TestServiceServer.java | 2 +- .../integration/AutoWindowSizingOnTest.java | 3 +- .../Http2NettyLocalChannelTest.java | 2 +- .../testing/integration/Http2NettyTest.java | 2 +- .../testing/integration/Http2OkHttpTest.java | 2 +- .../integration/TransportCompressionTest.java | 2 +- .../io/grpc/netty/NettyServerBuilder.java | 14 +++++++-- 9 files changed, 56 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/io/grpc/ManagedChannelBuilder.java b/core/src/main/java/io/grpc/ManagedChannelBuilder.java index af9c05705f..df259123cb 100644 --- a/core/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/core/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -16,6 +16,7 @@ package io.grpc; +import com.google.common.base.Preconditions; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -293,14 +294,14 @@ public abstract class ManagedChannelBuilder> *

This method is advisory, and implementations may decide to not enforce this. Currently, * the only known transport to not enforce this is {@code InProcessTransport}. * - * @param max the maximum number of bytes a single message can be. - * @throws IllegalArgumentException if max is negative. + * @param bytes the maximum number of bytes a single message can be. * @return this + * @throws IllegalArgumentException if bytes is negative. * @since 1.1.0 */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2307") - public T maxInboundMessageSize(int max) { - // intentional nop + public T maxInboundMessageSize(int bytes) { + // intentional noop rather than throw, this method is only advisory. + Preconditions.checkArgument(bytes >= 0, "bytes must be >= 0"); return thisT(); } diff --git a/core/src/main/java/io/grpc/ServerBuilder.java b/core/src/main/java/io/grpc/ServerBuilder.java index 397010be8c..6814660063 100644 --- a/core/src/main/java/io/grpc/ServerBuilder.java +++ b/core/src/main/java/io/grpc/ServerBuilder.java @@ -16,6 +16,7 @@ package io.grpc; +import com.google.common.base.Preconditions; import java.io.File; import java.io.InputStream; import java.util.concurrent.Executor; @@ -203,6 +204,27 @@ public abstract class ServerBuilder> { throw new UnsupportedOperationException(); } + /** + * Sets the maximum message size allowed to be received on the server. If not called, + * defaults to 4 MiB. The default provides protection to servers who haven't considered the + * possibility of receiving large messages while trying to be large enough to not be hit in normal + * usage. + * + *

This method is advisory, and implementations may decide to not enforce this. Currently, + * the only known transport to not enforce this is {@code InProcessServer}. + * + * @param bytes the maximum number of bytes a single message can be. + * @return this + * @throws IllegalArgumentException if bytes is negative. + * @throws UnsupportedOperationException if unsupported. + * @since 1.13.0 + */ + public T maxInboundMessageSize(int bytes) { + // intentional noop rather than throw, this method is only advisory. + Preconditions.checkArgument(bytes >= 0, "bytes must be >= 0"); + return thisT(); + } + /** * Builds a server using the given parameters. * @@ -213,4 +235,13 @@ public abstract class ServerBuilder> { * @since 1.0.0 */ public abstract Server build(); + + /** + * Returns the correctly typed version of the builder. + */ + private T thisT() { + @SuppressWarnings("unchecked") + T thisT = (T) this; + return thisT; + } } diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java index d34b7dbeae..b95289a0aa 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java @@ -147,7 +147,7 @@ public class TestServiceServer { server = NettyServerBuilder.forPort(port) .sslContext(sslContext) - .maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .addService( ServerInterceptors.intercept( new TestServiceImpl(executor), TestServiceImpl.interceptors())) diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java index 0d6f9fa02f..02f3378339 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java @@ -37,7 +37,8 @@ public class AutoWindowSizingOnTest extends AbstractInteropTest { @Override protected AbstractServerImplBuilder getServerBuilder() { - return NettyServerBuilder.forPort(0).maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); + return NettyServerBuilder.forPort(0) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); } @Override diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java index 44fe6f0c82..1c5b8e2f2d 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java @@ -38,7 +38,7 @@ public class Http2NettyLocalChannelTest extends AbstractInteropTest { return NettyServerBuilder .forAddress(new LocalAddress("in-process-1")) .flowControlWindow(65 * 1024) - .maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .channelType(LocalServerChannel.class); } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java index da1e6471ea..12acce484e 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java @@ -46,7 +46,7 @@ public class Http2NettyTest extends AbstractInteropTest { try { return NettyServerBuilder.forPort(0) .flowControlWindow(65 * 1024) - .maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(GrpcSslContexts .forServer(TestUtils.loadCert("server1.pem"), TestUtils.loadCert("server1.key")) .clientAuth(ClientAuth.REQUIRE) diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java index 4fba9567cb..c0b45ec8c2 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java @@ -78,7 +78,7 @@ public class Http2OkHttpTest extends AbstractInteropTest { contextBuilder.ciphers(TestUtils.preferredTestCiphers(), SupportedCipherSuiteFilter.INSTANCE); return NettyServerBuilder.forPort(0) .flowControlWindow(65 * 1024) - .maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(contextBuilder.build()); } catch (IOException ex) { throw new RuntimeException(ex); diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java index 0fd25d5738..704f4869a6 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java @@ -86,7 +86,7 @@ public class TransportCompressionTest extends AbstractInteropTest { @Override protected AbstractServerImplBuilder getServerBuilder() { return NettyServerBuilder.forPort(0) - .maxMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .compressorRegistry(compressors) .decompressorRegistry(decompressors) .intercept(new ServerInterceptor() { diff --git a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java index ae7463ff76..18217c5be1 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java @@ -259,10 +259,20 @@ public final class NettyServerBuilder extends AbstractServerImplBuilder= 0, "maxMessageSize must be >= 0"); - this.maxMessageSize = maxMessageSize; + return maxInboundMessageSize(maxMessageSize); + } + + /** {@inheritDoc} */ + @Override + public NettyServerBuilder maxInboundMessageSize(int bytes) { + checkArgument(bytes >= 0, "bytes must be >= 0"); + this.maxMessageSize = bytes; return this; }