diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java index b744bca3116..7d192b16943 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java @@ -951,13 +951,13 @@ public void settings(boolean clearPrevious, Settings settings) { @Override public void ping(boolean ack, int payload1, int payload2) { - if (!keepAliveEnforcer.pingAcceptable()) { - abruptShutdown(ErrorCode.ENHANCE_YOUR_CALM, "too_many_pings", - Status.RESOURCE_EXHAUSTED.withDescription("Too many pings from client"), false); - return; - } long payload = (((long) payload1) << 32) | (payload2 & 0xffffffffL); if (!ack) { + if (!keepAliveEnforcer.pingAcceptable()) { + abruptShutdown(ErrorCode.ENHANCE_YOUR_CALM, "too_many_pings", + Status.RESOURCE_EXHAUSTED.withDescription("Too many pings from client"), false); + return; + } frameLogger.logPing(OkHttpFrameLogger.Direction.INBOUND, payload); synchronized (lock) { frameWriter.ping(true, payload1, payload2); diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java index 4d2744dc9c7..00db6e1d339 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java @@ -1268,6 +1268,31 @@ public void keepAliveEnforcer_noticesActive() throws Exception { eq(ByteString.encodeString("too_many_pings", GrpcUtil.US_ASCII))); } + @Test + public void keepAliveEnforcer_doesNotEnforcePingAcks() throws Exception { + serverBuilder.permitKeepAliveTime(1, TimeUnit.HOURS) + .permitKeepAliveWithoutCalls(true); + initTransport(); + handshake(); + + for (int i = 0; i < KeepAliveEnforcer.MAX_PING_STRIKES + 2; i++) { + int serverPingId = 0xDEAD + i; + clientFrameWriter.ping(true, serverPingId, 0); + clientFrameWriter.flush(); + } + + for (int i = 0; i < KeepAliveEnforcer.MAX_PING_STRIKES; i++) { + pingPong(); + } + + pingPongId++; + clientFrameWriter.ping(false, pingPongId, 0); + clientFrameWriter.flush(); + assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue(); + verify(clientFramesRead).goAway(0, ErrorCode.ENHANCE_YOUR_CALM, + ByteString.encodeString("too_many_pings", GrpcUtil.US_ASCII)); + } + @Test public void maxConcurrentCallsPerConnection_failsWithRst() throws Exception { int maxConcurrentCallsPerConnection = 1;