Specify a locale for upper/lower case conversions

None of these conversions should use the arbitrary system locale. Error Prone will help prevent these getting introduced in the future. Fixes #10372
grpc · Mar 27, 2024 · e630593 · e630593
1 parent 37263b7
commit e630593
Show file tree

Hide file tree

Showing 12 changed files with 48 additions and 26 deletions.
diff --git a/authz/src/main/java/io/grpc/authz/AuthorizationPolicyTranslator.java b/authz/src/main/java/io/grpc/authz/AuthorizationPolicyTranslator.java
@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 /**
@@ -77,7 +78,7 @@ private static Permission parseHeader(Map<String, ?> header) throws IllegalArgum
     }
     if (key.charAt(0) == ':'
         || key.startsWith("grpc-")
-        || UNSUPPORTED_HEADERS.contains(key.toLowerCase())) {
+        || UNSUPPORTED_HEADERS.contains(key.toLowerCase(Locale.ROOT))) {
       throw new IllegalArgumentException(String.format("Unsupported \"key\" %s", key));
     }
     List<String> valuesList = JsonUtil.getListOfStrings(header, "values");

diff --git a/benchmarks/src/main/java/io/grpc/benchmarks/Transport.java b/benchmarks/src/main/java/io/grpc/benchmarks/Transport.java
@@ -16,6 +16,8 @@
 
 package io.grpc.benchmarks;
 
+import java.util.Locale;
+
 /**
  * All of the supported transports.
  */
@@ -64,11 +66,16 @@ public static String getDescriptionString() {
       if (!first) {
         builder.append("\n");
       }
-      builder.append(transport.name().toLowerCase());
+      builder.append(transport);
       builder.append(": ");
       builder.append(transport.description);
       first = false;
     }
     return builder.toString();
   }
+
+  @Override
+  public String toString() {
+    return name().toLowerCase(Locale.ROOT);
+  }
 }
diff --git a/benchmarks/src/main/java/io/grpc/benchmarks/qps/ClientConfiguration.java b/benchmarks/src/main/java/io/grpc/benchmarks/qps/ClientConfiguration.java
@@ -31,6 +31,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
+import java.util.Locale;
 import java.util.Set;
 
 /**
@@ -102,7 +103,7 @@ protected ClientConfiguration build0(ClientConfiguration config) {
       if (config.tls) {
         if (!config.transport.tlsSupported) {
           throw new IllegalArgumentException(
-              "Transport " + config.transport.name().toLowerCase() + " does not support TLS.");
+              "Transport " + config.transport + " does not support TLS.");
         }
       }
 
@@ -166,10 +167,10 @@ protected void setClientValue(ClientConfiguration config, String value) {
         config.testca = parseBoolean(value);
       }
     },
-    TRANSPORT("STR", Transport.getDescriptionString(), DEFAULT.transport.name().toLowerCase()) {
+    TRANSPORT("STR", Transport.getDescriptionString(), DEFAULT.transport.toString()) {
       @Override
       protected void setClientValue(ClientConfiguration config, String value) {
-        config.transport = Transport.valueOf(value.toUpperCase());
+        config.transport = Transport.valueOf(value.toUpperCase(Locale.ROOT));
       }
     },
     DURATION("SECONDS", "Duration of the benchmark.", "" + DEFAULT.duration) {
@@ -236,7 +237,7 @@ protected void setClientValue(ClientConfiguration config, String value) {
 
     @Override
     public String getName() {
-      return name().toLowerCase();
+      return name().toLowerCase(Locale.ROOT);
     }
 
     @Override

diff --git a/benchmarks/src/main/java/io/grpc/benchmarks/qps/ServerConfiguration.java b/benchmarks/src/main/java/io/grpc/benchmarks/qps/ServerConfiguration.java
@@ -29,6 +29,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 
 /**
  * Configuration options for benchmark servers.
@@ -69,7 +70,7 @@ protected Collection<Param> getParams() {
     protected ServerConfiguration build0(ServerConfiguration config) {
       if (config.tls && !config.transport.tlsSupported) {
         throw new IllegalArgumentException(
-            "TLS unsupported with the " + config.transport.name().toLowerCase() + " transport");
+            "TLS unsupported with the " + config.transport + " transport");
       }
 
       // Verify that the address type is correct for the transport type.
@@ -109,6 +110,11 @@ public enum Transport {
       this.socketAddressValidator = socketAddressValidator;
     }
 
+    @Override
+    public String toString() {
+      return name().toLowerCase(Locale.ROOT);
+    }
+
     /**
      * Validates the given address for this transport.
      *
@@ -128,7 +134,7 @@ static String getDescriptionString() {
         if (!first) {
           builder.append("\n");
         }
-        builder.append(transport.name().toLowerCase());
+        builder.append(transport);
         builder.append(": ");
         builder.append(transport.description);
         first = false;
@@ -158,10 +164,10 @@ protected void setServerValue(ServerConfiguration config, String value) {
         config.tls = parseBoolean(value);
       }
     },
-    TRANSPORT("STR", Transport.getDescriptionString(), DEFAULT.transport.name().toLowerCase()) {
+    TRANSPORT("STR", Transport.getDescriptionString(), DEFAULT.transport.toString()) {
       @Override
       protected void setServerValue(ServerConfiguration config, String value) {
-        config.transport = Transport.valueOf(value.toUpperCase());
+        config.transport = Transport.valueOf(value.toUpperCase(Locale.ROOT));
       }
     },
     DIRECTEXECUTOR("", "Don't use a threadpool for RPC calls, instead execute calls directly "
@@ -197,7 +203,7 @@ protected void setServerValue(ServerConfiguration config, String value) {
 
     @Override
     public String getName() {
-      return name().toLowerCase();
+      return name().toLowerCase(Locale.ROOT);
     }
 
     @Override

diff --git a/build.gradle b/build.gradle
@@ -253,11 +253,6 @@ subprojects {
             options.errorprone.check("JavaUtilDate", CheckSeverity.OFF)
             // The warning fails to provide a source location
             options.errorprone.check("MissingSummary", CheckSeverity.OFF)
-
-            // TODO(https://github.com/grpc/grpc-java/issues/10372): remove when fixed.
-            if (JavaVersion.current().isJava11Compatible()) {
-              options.errorprone.check("StringCaseLocaleUsage", CheckSeverity.OFF)
-            }
         }
         tasks.named("compileTestJava").configure {
             // LinkedList doesn't hurt much in tests and has lots of usages

diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/Http2TestCases.java b/interop-testing/src/main/java/io/grpc/testing/integration/Http2TestCases.java
@@ -17,6 +17,7 @@
 package io.grpc.testing.integration;
 
 import com.google.common.base.Preconditions;
+import java.util.Locale;
 
 /**
  * Enum of HTTP/2 interop test cases.
@@ -49,7 +50,7 @@ public String description() {
   public static Http2TestCases fromString(String s) {
     Preconditions.checkNotNull(s, "s");
     try {
-      return Http2TestCases.valueOf(s.toUpperCase());
+      return Http2TestCases.valueOf(s.toUpperCase(Locale.ROOT));
     } catch (IllegalArgumentException ex) {
       throw new IllegalArgumentException("Invalid test case: " + s);
     }

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
@@ -434,7 +434,7 @@ private static String serverAddressesToString(List<InetSocketAddress> addresses)
   private static String validTestCasesHelpText() {
     StringBuilder builder = new StringBuilder();
     for (TestCases testCase : TestCases.values()) {
-      String strTestcase = testCase.name().toLowerCase();
+      String strTestcase = testCase.toString();
       builder.append("\n      ")
           .append(strTestcase)
           .append(": ")

diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestCases.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestCases.java
@@ -17,6 +17,7 @@
 package io.grpc.testing.integration;
 
 import com.google.common.base.Preconditions;
+import java.util.Locale;
 
 /**
  * Enum of interop test cases.
@@ -79,6 +80,11 @@ public String description() {
    */
   public static TestCases fromString(String s) {
     Preconditions.checkNotNull(s, "s");
-    return TestCases.valueOf(s.toUpperCase());
+    return TestCases.valueOf(s.toUpperCase(Locale.ROOT));
+  }
+
+  @Override
+  public String toString() {
+    return name().toLowerCase(Locale.ROOT);
   }
 }
diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java
@@ -697,7 +697,7 @@ protected int operationTimeoutMillis() {
   private static String validTestCasesHelpText() {
     StringBuilder builder = new StringBuilder();
     for (TestCases testCase : TestCases.values()) {
-      String strTestcase = testCase.name().toLowerCase();
+      String strTestcase = testCase.toString();
       builder.append("\n      ")
           .append(strTestcase)
           .append(": ")

diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java
@@ -257,7 +257,7 @@ private static StructOrError<CdsUpdate.Builder> parseNonAggregateCluster(
         edsServiceName = edsClusterConfig.getServiceName();
       }
       // edsServiceName is required if the CDS resource has an xdstp name.
-      if ((edsServiceName == null) && clusterName.toLowerCase().startsWith("xdstp:")) {
+      if ((edsServiceName == null) && clusterName.toLowerCase(Locale.ROOT).startsWith("xdstp:")) {
         return StructOrError.fromError(
             "EDS service_name must be set when Cluster resource has an xdstp name");
       }

diff --git a/xds/src/main/java/io/grpc/xds/internal/Matchers.java b/xds/src/main/java/io/grpc/xds/internal/Matchers.java
@@ -22,6 +22,7 @@
 import com.google.re2j.Pattern;
 import java.math.BigInteger;
 import java.net.InetAddress;
+import java.util.Locale;
 import javax.annotation.Nullable;
 
 /**
@@ -273,11 +274,11 @@ public boolean matches(String args) {
             : exact().equals(args);
       } else if (prefix() != null) {
         return ignoreCase()
-            ? args.toLowerCase().startsWith(prefix().toLowerCase())
+            ? args.toLowerCase(Locale.ROOT).startsWith(prefix().toLowerCase(Locale.ROOT))
             : args.startsWith(prefix());
       } else if (suffix() != null) {
         return ignoreCase()
-            ? args.toLowerCase().endsWith(suffix().toLowerCase())
+            ? args.toLowerCase(Locale.ROOT).endsWith(suffix().toLowerCase(Locale.ROOT))
             : args.endsWith(suffix());
       } else if (contains() != null) {
         return args.contains(contains());

diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java
@@ -30,6 +30,7 @@
 import java.security.cert.X509Certificate;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
 import javax.annotation.Nullable;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
@@ -97,7 +98,8 @@ private static boolean verifyDnsNamePrefix(
       return false;
     }
     return ignoreCase
-        ? altNameFromCert.toLowerCase().startsWith(sanToVerifyPrefix.toLowerCase())
+        ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith(
+            sanToVerifyPrefix.toLowerCase(Locale.ROOT))
         : altNameFromCert.startsWith(sanToVerifyPrefix);
   }
 
@@ -107,7 +109,8 @@ private static boolean verifyDnsNameSuffix(
       return false;
     }
     return ignoreCase
-            ? altNameFromCert.toLowerCase().endsWith(sanToVerifySuffix.toLowerCase())
+            ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith(
+                sanToVerifySuffix.toLowerCase(Locale.ROOT))
             : altNameFromCert.endsWith(sanToVerifySuffix);
   }
 
@@ -117,7 +120,8 @@ private static boolean verifyDnsNameContains(
       return false;
     }
     return ignoreCase
-            ? altNameFromCert.toLowerCase().contains(sanToVerifySubstring.toLowerCase())
+            ? altNameFromCert.toLowerCase(Locale.ROOT).contains(
+                sanToVerifySubstring.toLowerCase(Locale.ROOT))
             : altNameFromCert.contains(sanToVerifySubstring);
   }