From 0e45e04041941efe7a7cd0bd50d9133bc58aac2a Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 19 Jul 2022 14:41:34 -0700 Subject: [PATCH] Avoid accidental locale-sensitive String.format() %s is fairly safe (requires a Formattable to use Locale), so %d is the main risk item. Places that really didn't need to use String.format() were converted to plain string concatenation. Logging locations were generally converted to using the log infrastructure's delayed formatting, which is generally locale-sensitive but we're okay with that. That wasn't done in okhttp, however, because Android frequently doesn't use MessageFormat so we'd lose the parameters. Everywhere else was explicitly defined to be Locale.US, to be consistent independent of the default system locale. --- .../io/grpc/inprocess/InProcessTransport.java | 4 ++++ .../io/grpc/internal/AbstractClientStream.java | 3 +-- .../java/io/grpc/internal/ClientCallImpl.java | 3 ++- .../io/grpc/internal/DelayedClientCall.java | 4 +++- .../main/java/io/grpc/internal/JsonUtil.java | 5 ++++- .../java/io/grpc/internal/MessageDeframer.java | 9 +++++---- .../java/io/grpc/internal/MessageFramer.java | 10 +++++++--- .../io/grpc/internal/ProxyDetectorImpl.java | 2 +- .../io/grpc/internal/MessageDeframerTest.java | 3 ++- .../io/grpc/internal/MessageFramerTest.java | 4 +++- .../integration/NettyClientInteropServlet.java | 3 +++ .../integration/AbstractInteropTest.java | 18 ++++++++++++------ .../testing/integration/StressTestClient.java | 5 +++-- .../testing/integration/XdsTestServer.java | 12 +++++------- .../java/io/grpc/netty/NettyClientHandler.java | 11 +++++------ .../java/io/grpc/netty/NettyServerHandler.java | 6 ++---- .../io/grpc/okhttp/OkHttpClientTransport.java | 11 ++++++++--- .../java/io/grpc/okhttp/internal/Headers.java | 3 +++ .../io/grpc/okhttp/internal/framed/Http2.java | 7 ++++--- .../internal/testing/TestStreamTracer.java | 3 +++ .../main/java/io/grpc/xds/ClientXdsClient.java | 4 ++-- .../grpc/xds/ClusterResolverLoadBalancer.java | 3 ++- xds/src/main/java/io/grpc/xds/FaultFilter.java | 2 ++ .../xds/RingHashLoadBalancerProviderTest.java | 4 +++- 24 files changed, 89 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/io/grpc/inprocess/InProcessTransport.java b/core/src/main/java/io/grpc/inprocess/InProcessTransport.java index f1aa514909..1c2ac3df22 100644 --- a/core/src/main/java/io/grpc/inprocess/InProcessTransport.java +++ b/core/src/main/java/io/grpc/inprocess/InProcessTransport.java @@ -66,6 +66,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.IdentityHashMap; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -240,6 +241,7 @@ final class InProcessTransport implements ServerTransport, ConnectionClientTrans // statuscodes.md is updated. Status status = Status.RESOURCE_EXHAUSTED.withDescription( String.format( + Locale.US, "Request metadata larger than %d: %d", serverMaxInboundMetadataSize, metadataSize)); @@ -554,6 +556,7 @@ final class InProcessTransport implements ServerTransport, ConnectionClientTrans // Status, which may need to be updated if statuscodes.md is updated. Status failedStatus = Status.RESOURCE_EXHAUSTED.withDescription( String.format( + Locale.US, "Response header metadata larger than %d: %d", clientMaxInboundMetadataSize, metadataSize)); @@ -593,6 +596,7 @@ final class InProcessTransport implements ServerTransport, ConnectionClientTrans // Status, which may need to be updated if statuscodes.md is updated. status = Status.RESOURCE_EXHAUSTED.withDescription( String.format( + Locale.US, "Response header metadata larger than %d: %d", clientMaxInboundMetadataSize, metadataSize)); diff --git a/core/src/main/java/io/grpc/internal/AbstractClientStream.java b/core/src/main/java/io/grpc/internal/AbstractClientStream.java index 531206b29c..4ef743bf96 100644 --- a/core/src/main/java/io/grpc/internal/AbstractClientStream.java +++ b/core/src/main/java/io/grpc/internal/AbstractClientStream.java @@ -331,8 +331,7 @@ public abstract class AbstractClientStream extends AbstractStream if (compressedStream) { deframeFailed( Status.INTERNAL - .withDescription( - String.format("Full stream and gRPC message encoding cannot both be set")) + .withDescription("Full stream and gRPC message encoding cannot both be set") .asRuntimeException()); return; } diff --git a/core/src/main/java/io/grpc/internal/ClientCallImpl.java b/core/src/main/java/io/grpc/internal/ClientCallImpl.java index db1a992b96..31286c2bd7 100644 --- a/core/src/main/java/io/grpc/internal/ClientCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ClientCallImpl.java @@ -358,12 +358,13 @@ final class ClientCallImpl extends ClientCall { long effectiveTimeout = max(0, effectiveDeadline.timeRemaining(TimeUnit.NANOSECONDS)); StringBuilder builder = new StringBuilder(String.format( + Locale.US, "Call timeout set to '%d' ns, due to context deadline.", effectiveTimeout)); if (callDeadline == null) { builder.append(" Explicit call timeout was not set."); } else { long callTimeout = callDeadline.timeRemaining(TimeUnit.NANOSECONDS); - builder.append(String.format(" Explicit call timeout was '%d' ns.", callTimeout)); + builder.append(String.format(Locale.US, " Explicit call timeout was '%d' ns.", callTimeout)); } log.fine(builder.toString()); diff --git a/core/src/main/java/io/grpc/internal/DelayedClientCall.java b/core/src/main/java/io/grpc/internal/DelayedClientCall.java index 89442e4bf9..df56fc0c5f 100644 --- a/core/src/main/java/io/grpc/internal/DelayedClientCall.java +++ b/core/src/main/java/io/grpc/internal/DelayedClientCall.java @@ -98,12 +98,14 @@ public class DelayedClientCall extends ClientCall { StringBuilder builder = new StringBuilder( String.format( + Locale.US, "Call timeout set to '%d' ns, due to context deadline.", remainingNanos)); if (deadline == null) { builder.append(" Explicit call timeout was not set."); } else { long callTimeout = deadline.timeRemaining(TimeUnit.NANOSECONDS); - builder.append(String.format(" Explicit call timeout was '%d' ns.", callTimeout)); + builder.append(String.format( + Locale.US, " Explicit call timeout was '%d' ns.", callTimeout)); } logger.fine(builder.toString()); } diff --git a/core/src/main/java/io/grpc/internal/JsonUtil.java b/core/src/main/java/io/grpc/internal/JsonUtil.java index 117135fe63..65f7cf5649 100644 --- a/core/src/main/java/io/grpc/internal/JsonUtil.java +++ b/core/src/main/java/io/grpc/internal/JsonUtil.java @@ -20,6 +20,7 @@ import static com.google.common.math.LongMath.checkedAdd; import java.text.ParseException; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -244,7 +245,8 @@ public class JsonUtil { for (int i = 0; i < rawList.size(); i++) { if (!(rawList.get(i) instanceof Map)) { throw new ClassCastException( - String.format("value %s for idx %d in %s is not object", rawList.get(i), i, rawList)); + String.format( + Locale.US, "value %s for idx %d in %s is not object", rawList.get(i), i, rawList)); } } return (List>) rawList; @@ -260,6 +262,7 @@ public class JsonUtil { if (!(rawList.get(i) instanceof String)) { throw new ClassCastException( String.format( + Locale.US, "value '%s' for idx %d in '%s' is not string", rawList.get(i), i, rawList)); } } diff --git a/core/src/main/java/io/grpc/internal/MessageDeframer.java b/core/src/main/java/io/grpc/internal/MessageDeframer.java index 534398315e..f6da1d0a67 100644 --- a/core/src/main/java/io/grpc/internal/MessageDeframer.java +++ b/core/src/main/java/io/grpc/internal/MessageDeframer.java @@ -28,6 +28,7 @@ import java.io.Closeable; import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.Locale; import java.util.zip.DataFormatException; import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; @@ -386,7 +387,7 @@ public class MessageDeframer implements Closeable, Deframer { requiredLength = nextFrame.readInt(); if (requiredLength < 0 || requiredLength > maxInboundMessageSize) { throw Status.RESOURCE_EXHAUSTED.withDescription( - String.format("gRPC message exceeds maximum size %d: %d", + String.format(Locale.US, "gRPC message exceeds maximum size %d: %d", maxInboundMessageSize, requiredLength)) .asRuntimeException(); } @@ -516,9 +517,9 @@ public class MessageDeframer implements Closeable, Deframer { private void verifySize() { if (count > maxMessageSize) { - throw Status.RESOURCE_EXHAUSTED.withDescription(String.format( - "Decompressed gRPC message exceeds maximum size %d", - maxMessageSize)).asRuntimeException(); + throw Status.RESOURCE_EXHAUSTED + .withDescription("Decompressed gRPC message exceeds maximum size " + maxMessageSize) + .asRuntimeException(); } } } diff --git a/core/src/main/java/io/grpc/internal/MessageFramer.java b/core/src/main/java/io/grpc/internal/MessageFramer.java index 2042bddca0..93d35250a0 100644 --- a/core/src/main/java/io/grpc/internal/MessageFramer.java +++ b/core/src/main/java/io/grpc/internal/MessageFramer.java @@ -34,6 +34,7 @@ import java.io.OutputStream; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import javax.annotation.Nullable; /** @@ -172,7 +173,8 @@ public class MessageFramer implements Framer { if (maxOutboundMessageSize >= 0 && written > maxOutboundMessageSize) { throw Status.RESOURCE_EXHAUSTED .withDescription( - String.format("message too large %d > %d", written , maxOutboundMessageSize)) + String.format( + Locale.US, "message too large %d > %d", written , maxOutboundMessageSize)) .asRuntimeException(); } writeBufferChain(bufferChain, false); @@ -192,7 +194,8 @@ public class MessageFramer implements Framer { if (maxOutboundMessageSize >= 0 && written > maxOutboundMessageSize) { throw Status.RESOURCE_EXHAUSTED .withDescription( - String.format("message too large %d > %d", written , maxOutboundMessageSize)) + String.format( + Locale.US, "message too large %d > %d", written , maxOutboundMessageSize)) .asRuntimeException(); } @@ -215,7 +218,8 @@ public class MessageFramer implements Framer { if (maxOutboundMessageSize >= 0 && messageLength > maxOutboundMessageSize) { throw Status.RESOURCE_EXHAUSTED .withDescription( - String.format("message too large %d > %d", messageLength , maxOutboundMessageSize)) + String.format( + Locale.US, "message too large %d > %d", messageLength , maxOutboundMessageSize)) .asRuntimeException(); } headerScratch.clear(); diff --git a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java b/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java index 3e7dd010e2..f66dbff13a 100644 --- a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java +++ b/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java @@ -133,7 +133,7 @@ class ProxyDetectorImpl implements ProxyDetector { // let url be null log.log( Level.WARNING, - String.format("failed to create URL for Authenticator: %s %s", protocol, host)); + "failed to create URL for Authenticator: {0} {1}", new Object[] {protocol, host}); } // TODO(spencerfang): consider using java.security.AccessController here return Authenticator.requestPasswordAuthentication( diff --git a/core/src/test/java/io/grpc/internal/MessageDeframerTest.java b/core/src/test/java/io/grpc/internal/MessageDeframerTest.java index 5edf64ef85..6d2c27d6b1 100644 --- a/core/src/test/java/io/grpc/internal/MessageDeframerTest.java +++ b/core/src/test/java/io/grpc/internal/MessageDeframerTest.java @@ -49,6 +49,7 @@ import java.io.OutputStream; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Locale; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; import org.junit.Before; @@ -507,7 +508,7 @@ public class MessageDeframerTest { for (int i = 0; i < count; i++) { assertEquals("inboundMessage(" + i + ")", tracer.nextInboundEvent()); assertEquals( - String.format("inboundMessageRead(%d, %d, -1)", i, sizes[i * 2]), + String.format(Locale.US, "inboundMessageRead(%d, %d, -1)", i, sizes[i * 2]), tracer.nextInboundEvent()); expectedWireSize += sizes[i * 2]; expectedUncompressedSize += sizes[i * 2 + 1]; diff --git a/core/src/test/java/io/grpc/internal/MessageFramerTest.java b/core/src/test/java/io/grpc/internal/MessageFramerTest.java index 91698acced..752efad2dc 100644 --- a/core/src/test/java/io/grpc/internal/MessageFramerTest.java +++ b/core/src/test/java/io/grpc/internal/MessageFramerTest.java @@ -34,6 +34,7 @@ import java.io.ByteArrayInputStream; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.Arrays; +import java.util.Locale; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -382,7 +383,8 @@ public class MessageFramerTest { for (int i = 0; i < count; i++) { assertEquals("outboundMessage(" + i + ")", tracer.nextOutboundEvent()); assertEquals( - String.format("outboundMessageSent(%d, %d, %d)", i, sizes[i * 2], sizes[i * 2 + 1]), + String.format( + Locale.US, "outboundMessageSent(%d, %d, %d)", i, sizes[i * 2], sizes[i * 2 + 1]), tracer.nextOutboundEvent()); expectedWireSize += sizes[i * 2]; expectedUncompressedSize += sizes[i * 2 + 1]; diff --git a/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java b/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java index 6af0222714..48978fac0b 100644 --- a/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java +++ b/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java @@ -28,6 +28,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.text.SimpleDateFormat; import java.util.Calendar; +import java.util.Locale; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.logging.Handler; @@ -102,6 +103,7 @@ public final class NettyClientInteropServlet extends HttpServlet { resp.setStatus(200); writer.println( String.format( + Locale.US, "PASS! Tests ran %d, tests ignored %d", result.getRunCount(), result.getIgnoreCount())); @@ -109,6 +111,7 @@ public final class NettyClientInteropServlet extends HttpServlet { resp.setStatus(500); writer.println( String.format( + Locale.US, "FAILED! Tests ran %d, tests failed %d, tests ignored %d", result.getRunCount(), result.getFailureCount(), diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java index d9a93873c2..05505f7413 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java @@ -115,6 +115,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; @@ -2058,6 +2059,7 @@ public abstract class AbstractInteropTest { .get().getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); System.err.print( String.format( + Locale.US, "soak iteration: %d elapsed_ms: %d peer: %s", i, result.getLatencyMs(), peer != null ? peer.toString() : "null")); if (!result.getStatus().equals(Status.OK)) { @@ -2066,8 +2068,7 @@ public abstract class AbstractInteropTest { } else if (result.getLatencyMs() > maxAcceptablePerIterationLatencyMs) { totalFailures++; System.err.println( - String.format( - " exceeds max acceptable latency: %d", maxAcceptablePerIterationLatencyMs)); + " exceeds max acceptable latency: " + maxAcceptablePerIterationLatencyMs); } else { System.err.println(" succeeded"); } @@ -2082,6 +2083,7 @@ public abstract class AbstractInteropTest { soakChannel.awaitTermination(10, TimeUnit.SECONDS); System.err.println( String.format( + Locale.US, "soak test ran: %d / %d iterations\n" + "total failures: %d\n" + "max failures threshold: %d\n" @@ -2102,6 +2104,7 @@ public abstract class AbstractInteropTest { // check if we timed out String timeoutErrorMessage = String.format( + Locale.US, "soak test consumed all %d seconds of time and quit early, only " + "having ran %d out of desired %d iterations.", overallTimeoutSeconds, @@ -2111,6 +2114,7 @@ public abstract class AbstractInteropTest { // check if we had too many failures String tooManyFailuresErrorMessage = String.format( + Locale.US, "soak test total failures: %d exceeds max failures threshold: %d.", totalFailures, maxFailures); assertTrue(tooManyFailuresErrorMessage, totalFailures <= maxFailures); @@ -2353,9 +2357,10 @@ public abstract class AbstractInteropTest { long uncompressedSentSize = 0; int seqNo = 0; for (MessageLite msg : sentMessages) { - assertThat(tracer.nextOutboundEvent()).isEqualTo(String.format("outboundMessage(%d)", seqNo)); + assertThat(tracer.nextOutboundEvent()) + .isEqualTo(String.format(Locale.US, "outboundMessage(%d)", seqNo)); assertThat(tracer.nextOutboundEvent()).matches( - String.format("outboundMessageSent\\(%d, -?[0-9]+, -?[0-9]+\\)", seqNo)); + String.format(Locale.US, "outboundMessageSent\\(%d, -?[0-9]+, -?[0-9]+\\)", seqNo)); seqNo++; uncompressedSentSize += msg.getSerializedSize(); } @@ -2363,9 +2368,10 @@ public abstract class AbstractInteropTest { long uncompressedReceivedSize = 0; seqNo = 0; for (MessageLite msg : receivedMessages) { - assertThat(tracer.nextInboundEvent()).isEqualTo(String.format("inboundMessage(%d)", seqNo)); + assertThat(tracer.nextInboundEvent()) + .isEqualTo(String.format(Locale.US, "inboundMessage(%d)", seqNo)); assertThat(tracer.nextInboundEvent()).matches( - String.format("inboundMessageRead\\(%d, -?[0-9]+, -?[0-9]+\\)", seqNo)); + String.format(Locale.US, "inboundMessageRead\\(%d, -?[0-9]+, -?[0-9]+\\)", seqNo)); uncompressedReceivedSize += msg.getSerializedSize(); seqNo++; } diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/StressTestClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/StressTestClient.java index 6669d2700d..0aa5b04b3c 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/StressTestClient.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/StressTestClient.java @@ -54,6 +54,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; @@ -231,8 +232,8 @@ public class StressTestClient { ManagedChannel channel = createChannel(address); channels.add(channel); for (int j = 0; j < stubsPerChannel; j++) { - String gaugeName = - String.format("/stress_test/server_%d/channel_%d/stub_%d/qps", serverIdx, i, j); + String gaugeName = String.format( + Locale.US, "/stress_test/server_%d/channel_%d/stub_%d/qps", serverIdx, i, j); Worker worker = new Worker(channel, testCaseWeightPairs, durationSecs, gaugeName); diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestServer.java b/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestServer.java index 2118f96713..19a3c44259 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestServer.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestServer.java @@ -310,9 +310,7 @@ public final class XdsTestServer { } catch (NumberFormatException e) { newCall.close( Status.INVALID_ARGUMENT.withDescription( - String.format( - "Invalid format for grpc-previous-rpc-attempts header (%s)", - attemptNumHeader)), + "Invalid format for grpc-previous-rpc-attempts header: " + attemptNumHeader), new Metadata()); return noopListener; } @@ -329,7 +327,7 @@ public final class XdsTestServer { } else { newCall.close( Status.INVALID_ARGUMENT.withDescription( - String.format("Invalid format for rpc-behavior header (%s)", callBehavior)), + "Invalid format for rpc-behavior header: " + callBehavior), new Metadata() ); return noopListener; @@ -344,7 +342,7 @@ public final class XdsTestServer { } catch (NumberFormatException e) { newCall.close( Status.INVALID_ARGUMENT.withDescription( - String.format("Invalid format for rpc-behavior header (%s)", callBehavior)), + "Invalid format for rpc-behavior header: " + callBehavior), new Metadata()); return noopListener; } catch (InterruptedException e) { @@ -364,7 +362,7 @@ public final class XdsTestServer { } catch (NumberFormatException e) { newCall.close( Status.INVALID_ARGUMENT.withDescription( - String.format("Invalid format for rpc-behavior header (%s)", callBehavior)), + "Invalid format for rpc-behavior header: " + callBehavior), new Metadata()); return noopListener; } @@ -390,7 +388,7 @@ public final class XdsTestServer { } catch (NumberFormatException e) { newCall.close( Status.INVALID_ARGUMENT.withDescription( - String.format("Invalid format for rpc-behavior header (%s)", callBehavior)), + "Invalid format for rpc-behavior header: " + callBehavior), new Metadata()); return noopListener; } diff --git a/netty/src/main/java/io/grpc/netty/NettyClientHandler.java b/netty/src/main/java/io/grpc/netty/NettyClientHandler.java index 80d11e5485..2fe4d65bfa 100644 --- a/netty/src/main/java/io/grpc/netty/NettyClientHandler.java +++ b/netty/src/main/java/io/grpc/netty/NettyClientHandler.java @@ -951,17 +951,16 @@ class NettyClientHandler extends AbstractNettyHandler { Http2Ping p = ping; if (ackPayload == flowControlPing().payload()) { flowControlPing().updateWindow(); - if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, String.format("Window: %d", - decoder().flowController().initialWindowSize(connection().connectionStream()))); - } + logger.log(Level.FINE, "Window: {0}", + decoder().flowController().initialWindowSize(connection().connectionStream())); } else if (p != null) { if (p.payload() == ackPayload) { p.complete(); ping = null; } else { - logger.log(Level.WARNING, String.format( - "Received unexpected ping ack. Expecting %d, got %d", p.payload(), ackPayload)); + logger.log(Level.WARNING, + "Received unexpected ping ack. Expecting {0}, got {1}", + new Object[] {p.payload(), ackPayload}); } } else { logger.warning("Received unexpected ping ack. No ping outstanding"); diff --git a/netty/src/main/java/io/grpc/netty/NettyServerHandler.java b/netty/src/main/java/io/grpc/netty/NettyServerHandler.java index 902b54f376..1dc73fa5e0 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServerHandler.java +++ b/netty/src/main/java/io/grpc/netty/NettyServerHandler.java @@ -894,10 +894,8 @@ class NettyServerHandler extends AbstractNettyHandler { } if (data == flowControlPing().payload()) { flowControlPing().updateWindow(); - if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, String.format("Window: %d", - decoder().flowController().initialWindowSize(connection().connectionStream()))); - } + logger.log(Level.FINE, "Window: {0}", + decoder().flowController().initialWindowSize(connection().connectionStream())); } else if (data == GRACEFUL_SHUTDOWN_PING) { if (gracefulShutdown == null) { // this should never happen diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 32f263f658..81af4f30d4 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -80,6 +80,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -642,7 +643,8 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep // Prepare headers and request method line Request proxyRequest = createHttpProxyRequest(address, proxyUsername, proxyPassword); HttpUrl url = proxyRequest.httpUrl(); - String requestLine = String.format("CONNECT %s:%d HTTP/1.1", url.host(), url.port()); + String requestLine = + String.format(Locale.US, "CONNECT %s:%d HTTP/1.1", url.host(), url.port()); // Write request to socket sink.writeUtf8(requestLine).writeUtf8("\r\n"); @@ -674,6 +676,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep // ignored } String message = String.format( + Locale.US, "Response returned from proxy was not successful (expected 2xx, got %d %s). " + "Response body:\n%s", statusLine.code, statusLine.message, body.readUtf8()); @@ -1185,6 +1188,7 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep if (metadataSize > maxInboundMetadataSize) { failedStatus = Status.RESOURCE_EXHAUSTED.withDescription( String.format( + Locale.US, "Response %s metadata larger than %d: %d", inFinished ? "trailer" : "header", maxInboundMetadataSize, @@ -1300,8 +1304,9 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep p = ping; ping = null; } else { - log.log(Level.WARNING, String.format("Received unexpected ping ack. " - + "Expecting %d, got %d", ping.payload(), ackPayload)); + log.log(Level.WARNING, String.format( + Locale.US, "Received unexpected ping ack. Expecting %d, got %d", + ping.payload(), ackPayload)); } } else { log.warning("Received unexpected ping ack. No ping outstanding"); diff --git a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Headers.java b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Headers.java index 4723a87dfb..115f996400 100644 --- a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Headers.java +++ b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Headers.java @@ -22,6 +22,7 @@ package io.grpc.okhttp.internal; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Locale; /** * The header fields of a single HTTP message. Values are uninterpreted strings; @@ -132,6 +133,7 @@ public final class Headers { char c = name.charAt(i); if (c <= '\u001f' || c >= '\u007f') { throw new IllegalArgumentException(String.format( + Locale.US, "Unexpected char %#04x at %d in header name: %s", (int) c, i, name)); } } @@ -140,6 +142,7 @@ public final class Headers { char c = value.charAt(i); if (c <= '\u001f' || c >= '\u007f') { throw new IllegalArgumentException(String.format( + Locale.US, "Unexpected char %#04x at %d in header value: %s", (int) c, i, value)); } } diff --git a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java index e30264b9e3..3a8c41c628 100644 --- a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java +++ b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java @@ -23,6 +23,7 @@ import com.google.errorprone.annotations.FormatMethod; import io.grpc.okhttp.internal.Protocol; import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.logging.Logger; import okio.Buffer; @@ -590,12 +591,12 @@ public final class Http2 implements Variant { @FormatMethod private static IllegalArgumentException illegalArgument(String message, Object... args) { - throw new IllegalArgumentException(format(message, args)); + throw new IllegalArgumentException(format(Locale.US, message, args)); } @FormatMethod private static IOException ioException(String message, Object... args) throws IOException { - throw new IOException(format(message, args)); + throw new IOException(format(Locale.US, message, args)); } /** @@ -684,7 +685,7 @@ public final class Http2 implements Variant { static String formatHeader(boolean inbound, int streamId, int length, byte type, byte flags) { String formattedType = type < TYPES.length ? TYPES[type] : format("0x%02x", type); String formattedFlags = formatFlags(type, flags); - return format("%s 0x%08x %5d %-13s %s", inbound ? "<<" : ">>", streamId, length, + return format(Locale.US, "%s 0x%08x %5d %-13s %s", inbound ? "<<" : ">>", streamId, length, formattedType, formattedFlags); } diff --git a/testing/src/main/java/io/grpc/internal/testing/TestStreamTracer.java b/testing/src/main/java/io/grpc/internal/testing/TestStreamTracer.java index 444537962e..1d0061f612 100644 --- a/testing/src/main/java/io/grpc/internal/testing/TestStreamTracer.java +++ b/testing/src/main/java/io/grpc/internal/testing/TestStreamTracer.java @@ -18,6 +18,7 @@ package io.grpc.internal.testing; import io.grpc.Status; import io.grpc.StreamTracer; +import java.util.Locale; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -180,6 +181,7 @@ public interface TestStreamTracer { int seqNo, long optionalWireSize, long optionalUncompressedSize) { outboundEvents.add( String.format( + Locale.US, "outboundMessageSent(%d, %d, %d)", seqNo, optionalWireSize, optionalUncompressedSize)); } @@ -189,6 +191,7 @@ public interface TestStreamTracer { int seqNo, long optionalWireSize, long optionalUncompressedSize) { inboundEvents.add( String.format( + Locale.US, "inboundMessageRead(%d, %d, %d)", seqNo, optionalWireSize, optionalUncompressedSize)); } diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index c448a82211..e577e8ce18 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -1795,8 +1795,8 @@ final class ClientXdsClient extends XdsClient implements XdsResponseHandler, Res "Cluster " + clusterName + ": LOGICAL DNS clusters socket_address must have port_value"); } - String dnsHostName = - String.format("%s:%d", socketAddress.getAddress(), socketAddress.getPortValue()); + String dnsHostName = String.format( + Locale.US, "%s:%d", socketAddress.getAddress(), socketAddress.getPortValue()); return StructOrError.fromStruct(CdsUpdate.forLogicalDns( clusterName, dnsHostName, lrsServerInfo, maxConcurrentRequests, upstreamTlsContext)); } diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java index 151156612e..108f406ade 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java @@ -60,6 +60,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -470,7 +471,7 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { } } if ("".equals(foundName)) { - foundName = String.format("%s[child%d]", name, priorityNameGenId++); + foundName = String.format(Locale.US, "%s[child%d]", name, priorityNameGenId++); } for (Locality locality : todo.get(priority)) { newNames.put(locality, foundName); diff --git a/xds/src/main/java/io/grpc/xds/FaultFilter.java b/xds/src/main/java/io/grpc/xds/FaultFilter.java index 2abef58d93..d46b3d30f5 100644 --- a/xds/src/main/java/io/grpc/xds/FaultFilter.java +++ b/xds/src/main/java/io/grpc/xds/FaultFilter.java @@ -48,6 +48,7 @@ import io.grpc.xds.FaultConfig.FaultAbort; import io.grpc.xds.FaultConfig.FaultDelay; import io.grpc.xds.Filter.ClientInterceptorBuilder; import io.grpc.xds.ThreadSafeRandom.ThreadSafeRandomImpl; +import java.util.Locale; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -255,6 +256,7 @@ final class FaultFilter implements Filter, ClientInterceptorBuilder { // only sent out after the delay. There could be a race between local and // remote, but it is rather rare.) String description = String.format( + Locale.US, "Deadline exceeded after up to %d ns of fault-injected delay", finalDelayNanos); if (status.getDescription() != null) { diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerProviderTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerProviderTest.java index 2d84b8285c..9f972eaef9 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerProviderTest.java @@ -31,6 +31,7 @@ import io.grpc.internal.JsonParser; import io.grpc.xds.RingHashLoadBalancer.RingHashConfig; import java.io.IOException; import java.lang.Thread.UncaughtExceptionHandler; +import java.util.Locale; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -117,7 +118,8 @@ public class RingHashLoadBalancerProviderTest { @Test public void parseLoadBalancingConfig_invalid_ringTooLarge() throws IOException { long ringSize = RingHashLoadBalancerProvider.MAX_RING_SIZE + 1; - String lbConfig = String.format("{\"minRingSize\" : 10, \"maxRingSize\" : %d}", ringSize); + String lbConfig = + String.format(Locale.US, "{\"minRingSize\" : 10, \"maxRingSize\" : %d}", ringSize); ConfigOrError configOrError = provider.parseLoadBalancingPolicyConfig(parseJsonObject(lbConfig)); assertThat(configOrError.getError()).isNotNull();