Skip to content

Commit

Permalink
Specify an explicit locale for all to{Lower,Upper}Case calls
Browse files Browse the repository at this point in the history
Java's `String#to{Lower,Upper}Case()` is locale-dependent, which can
lead to unexpected results in locales with special case mappings in the
ASCII range (e.g. in a Turkish locale, a capital ASCII `I` lowercases to
a non-ASCII variant of `i`).

This is prevented by specifying a local without such case mappings. This
commit uses `Locale.ROOT` as the canonical choice with the same case
mapping behavior as other common locales such as `Locale.ENGLISH` or
`Locale.US`.

Follow-up changes could use Guava's `Ascii.to{Lower,Upper}Case` instead,
but whether this is safe may depend on the context, which makes this
replacement unsuitable to perform across the repo.

Fixes bazelbuild#17541
  • Loading branch information
fmeum committed Mar 8, 2023
1 parent 486a964 commit a1d3a17
Show file tree
Hide file tree
Showing 35 changed files with 87 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.EnumMap;
import java.util.Locale;
import java.util.Map;

/** TestIntegration represents an external link that is integrated with the test results. */
Expand Down Expand Up @@ -66,7 +67,7 @@ public enum ExternalLinkAttribute {

/** Gets the string representation of the current enum. */
public String getXmlAttributeName() {
return name().toLowerCase();
return name().toLowerCase(Locale.ROOT);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.docgen.DocgenConsts.RuleType;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
Expand Down Expand Up @@ -157,7 +158,7 @@ String getFamilySummary() {
*/
@VisibleForTesting
static String normalize(String s) {
return s.toLowerCase()
return s.toLowerCase(Locale.ROOT)
.replace("+", "p")
.replaceAll("[()]", "")
.replaceAll("[\\s/]", "-")
Expand Down Expand Up @@ -298,12 +299,12 @@ public String getAttributeSignature() {
if (attributeDoc.isCommonType()) {
sb.append(String.format("<a href=\"%s#%s.%s\">%s</a>",
COMMON_DEFINITIONS_PAGE,
attributeDoc.getGeneratedInRule(ruleName).toLowerCase(),
attributeDoc.getGeneratedInRule(ruleName).toLowerCase(Locale.ROOT),
attrName,
attrName));
} else {
sb.append(String.format("<a href=\"#%s.%s\">%s</a>",
attributeDoc.getGeneratedInRule(ruleName).toLowerCase(),
attributeDoc.getGeneratedInRule(ruleName).toLowerCase(Locale.ROOT),
attrName,
attrName));
}
Expand All @@ -322,7 +323,7 @@ private String expandBuiltInVariables(String key, String value) {
switch (key) {
case DocgenConsts.VAR_IMPLICIT_OUTPUTS:
return String.format("<h4 id=\"%s_implicit_outputs\">Implicit output targets</h4>\n%s",
ruleName.toLowerCase(), value);
ruleName.toLowerCase(Locale.ROOT), value);
default:
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.docgen.DocgenConsts.RuleType;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import java.util.List;
import java.util.Locale;

/**
* Helper class for representing a rule family in the rule summary table template.
Expand Down Expand Up @@ -71,7 +72,7 @@ public class RuleFamily {
* "c-cpp".
*/
static String normalize(String s) {
return FAMILY_NAME_ESCAPER.escape(s.toLowerCase()).replaceAll("[-]+", "-");
return FAMILY_NAME_ESCAPER.escape(s.toLowerCase(Locale.ROOT)).replaceAll("[-]+", "-");
}

public int size() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public String getDescription() {
}

public String getTemplateIdentifier() {
return name().toLowerCase().replace("_", "-");
return name().toLowerCase(Locale.ROOT).replace("_", "-");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static SymlinkBehavior parse(String value) throws IllegalArgumentExceptio

@Override
public String toString() {
return super.toString().toLowerCase();
return super.toString().toLowerCase(Locale.ROOT);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
Expand Down Expand Up @@ -514,7 +515,8 @@ private ImmutableList<String> generateBzlConfigFor(String version, CToolchain to
tp ->
String.format(
" %s_path = '%s'",
tp.getName().toLowerCase().replaceAll("-", "_"), tp.getPath()))
tp.getName().toLowerCase(Locale.ROOT).replaceAll("-", "_"),
tp.getPath()))
.collect(ImmutableList.toImmutableList()));
return bigConditional.add("").build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import javax.annotation.Nullable;

Expand All @@ -36,9 +37,9 @@ private enum RuntimeType {
private final String fileExtension;

RuntimeType(String fileExtension) {
this.name = name().toLowerCase();
this.name = name().toLowerCase(Locale.ROOT);
this.fileExtension = fileExtension;
}
}
}

private final Map<String, String> fileGroupNameToFileGlobs = new LinkedHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand Down Expand Up @@ -130,7 +131,7 @@ private static String querySdkRoot(
String developerDir, String sdkVersion, String appleSdkPlatform)
throws IOException, InterruptedException {
try {
String sdkString = appleSdkPlatform.toLowerCase() + sdkVersion;
String sdkString = appleSdkPlatform.toLowerCase(Locale.ROOT) + sdkVersion;
Map<String, String> env =
Strings.isNullOrEmpty(developerDir)
? ImmutableMap.<String, String>of()
Expand Down Expand Up @@ -193,7 +194,7 @@ private static String getSdkRoot(String developerDir, String sdkVersion, String
throws IOException, InterruptedException {
try {
return sdkRootCache.computeIfAbsent(
developerDir + ":" + appleSdkPlatform.toLowerCase() + ":" + sdkVersion,
developerDir + ":" + appleSdkPlatform.toLowerCase(Locale.ROOT) + ":" + sdkVersion,
(key) -> {
try {
String sdkRoot = querySdkRoot(developerDir, sdkVersion, appleSdkPlatform);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -120,7 +121,7 @@ private enum Type {
final String findFilter;

private Rule(String type, String pattern, String findRoot, String findFilter) {
this.type = Type.valueOf(type.trim().toUpperCase());
this.type = Type.valueOf(type.trim().toUpperCase(Locale.ROOT));
this.pattern = Pattern.compile("^" + pattern + "$");
this.findRoot = findRoot.replace('\\', '$');
this.findFilter = findFilter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.LinkedHashSet;
import java.util.Locale;
import java.util.Set;

/**
Expand Down Expand Up @@ -65,7 +66,7 @@ public Set<E> convert(String input) throws OptionsParsingException {
EnumSet<E> excludedSet = EnumSet.noneOf(typeClass);
for (String value : input.split(",", -1)) {
boolean excludeFlag = value.startsWith("-");
String s = (excludeFlag ? value.substring(1) : value).toUpperCase();
String s = (excludeFlag ? value.substring(1) : value).toUpperCase(Locale.ROOT);
if (!allowedValues.contains(s)) {
throw new OptionsParsingException("Invalid " + prettyEnumName + " filter '" + value +
"' in the input '" + input + "'");
Expand All @@ -91,6 +92,6 @@ public Set<E> convert(String input) throws OptionsParsingException {
@Override
public final String getTypeDescription() {
return "comma-separated list of values: "
+ StringUtil.joinEnglishList(allowedValues).toLowerCase();
+ StringUtil.joinEnglishList(allowedValues).toLowerCase(Locale.ROOT);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ public boolean isSpecified() {
@Override
public String toString() {
if (exceptions.isEmpty()) {
return licenseTypes.toString().toLowerCase();
return licenseTypes.toString().toLowerCase(Locale.ROOT);
} else {
return licenseTypes.toString().toLowerCase() + " with exceptions " + exceptions;
return licenseTypes.toString().toLowerCase(Locale.ROOT) + " with exceptions " + exceptions;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableMap;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Locale;
import java.util.Set;
import javax.annotation.Nullable;

Expand All @@ -41,7 +42,7 @@ public enum TestSize {
static {
ImmutableMap.Builder<String, TestSize> builder = ImmutableMap.builder();
for (TestSize size : TestSize.values()) {
builder.put(size.name().toLowerCase(), size);
builder.put(size.name().toLowerCase(Locale.ROOT), size);
}
CANONICAL_LOWER_CASE_NAME_TABLE = builder.buildOrThrow();
}
Expand Down Expand Up @@ -94,7 +95,7 @@ public static TestSize getTestSize(TestTimeout timeout) {
*/
@Nullable
public static TestSize getTestSize(String attr) {
if (!attr.equals(attr.toLowerCase())) {
if (!attr.equals(attr.toLowerCase(Locale.ROOT))) {
return null;
}
try {
Expand All @@ -107,7 +108,7 @@ public static TestSize getTestSize(String attr) {
/** Normal practice is to always use size tags as lower case strings. */
@Override
public String toString() {
return super.toString().toLowerCase();
return super.toString().toLowerCase(Locale.ROOT);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public enum TestTimeout {
*/
@Nullable
public static TestTimeout getTestTimeout(String attr) {
if (!attr.equals(attr.toLowerCase())) {
if (!attr.equals(attr.toLowerCase(Locale.ROOT))) {
return null;
}
try {
Expand All @@ -138,7 +138,7 @@ public static TestTimeout getTestTimeout(String attr) {
@Nullable
public static TestTimeout getTestTimeout(Rule testTarget) {
String attr = NonconfigurableAttributeMapper.of(testTarget).get("timeout", Type.STRING);
if (!attr.equals(attr.toLowerCase())) {
if (!attr.equals(attr.toLowerCase(Locale.ROOT))) {
return null; // attribute values must be lowercase
}
try {
Expand All @@ -150,14 +150,14 @@ public static TestTimeout getTestTimeout(Rule testTarget) {

@Override
public String toString() {
return super.toString().toLowerCase();
return super.toString().toLowerCase(Locale.ROOT);
}

/**
* We print to upper case to make the test timeout warnings more readable.
*/
public String prettyPrint() {
return super.toString().toUpperCase();
return super.toString().toUpperCase(Locale.ROOT);
}

@Deprecated // use getTimeout instead
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
import java.net.URISyntaxException;
import java.nio.channels.ClosedChannelException;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -236,7 +237,7 @@ private static ServerCapabilities getAndVerifyServerCapabilities(
|| status == HttpResponseStatus.SERVICE_UNAVAILABLE.code()
|| status == HttpResponseStatus.GATEWAY_TIMEOUT.code();
} else if (e instanceof IOException) {
String msg = e.getMessage().toLowerCase();
String msg = e.getMessage().toLowerCase(Locale.ROOT);
if (msg.contains("connection reset by peer")) {
retry = true;
} else if (msg.contains("operation timed out")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionMetadataTag;
import java.util.List;
import java.util.Locale;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkValue;

Expand Down Expand Up @@ -165,9 +166,9 @@ public enum AndroidManifestMerger {

public static List<String> getAttributeValues() {
return ImmutableList.of(
LEGACY.name().toLowerCase(),
ANDROID.name().toLowerCase(),
FORCE_ANDROID.name().toLowerCase(),
LEGACY.name().toLowerCase(Locale.ROOT),
ANDROID.name().toLowerCase(Locale.ROOT),
FORCE_ANDROID.name().toLowerCase(Locale.ROOT),
getRuleAttributeDefault());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.List;
import java.util.Locale;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkInt;

Expand Down Expand Up @@ -920,7 +921,7 @@ public enum MultidexMode {

/** Returns the attribute value that specifies this mode. */
public String getAttributeValue() {
return toString().toLowerCase();
return toString().toLowerCase(Locale.ROOT);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -153,7 +154,7 @@ public void writeOutputFile(OutputStream out) throws IOException {
ps.printf("--verbosity=%s\n", incrementalInstallVerbosity);
}

ps.printf("--start=%s\n", start.name().toLowerCase());
ps.printf("--start=%s\n", start.name().toLowerCase(Locale.ROOT));

if (userHomeDirectory != null) {
ps.printf("--user_home_dir=%s\n", userHomeDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.Arrays;
import java.util.EnumMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
Expand Down Expand Up @@ -412,7 +413,7 @@ public Label getXcodeConfigLabel() {
public String getOutputDirectoryName() {
List<String> components = new ArrayList<>();
if (!appleCpus.appleSplitCpu().isEmpty()) {
components.add(applePlatformType.toString().toLowerCase());
components.add(applePlatformType.toString().toLowerCase(Locale.ROOT));
components.add(appleCpus.appleSplitCpu());

if (options.getMinimumOsVersion() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public boolean isImmutable() {

@Override
public String toString() {
return name().toLowerCase();
return name().toLowerCase(Locale.ROOT);
}

/**
Expand Down
Loading

0 comments on commit a1d3a17

Please sign in to comment.