Enable Error Prone check DefaultCharset

This is focused on charsets, so it won't catch locale-related problems like `String#toLowerCase` with default locale. But it will catch `String` to `byte[]` (and vice versa) conversion with default charset, in addition to the IO stream related issues fixed in this commit. The existing cases were fixed to explicitly use `defaultCharset()` where the data being read comes from (or is written to a file on) the local system, otherwise they are fixed to use `UTF_8` - unless there are special considerations to use a different charset.
trinodb · May 12, 2022 · 08bcca6 · 08bcca6
1 parent bff82d0
commit 08bcca6
Show file tree

Hide file tree

Showing 12 changed files with 25 additions and 12 deletions.
diff --git a/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopProcessFactory.java b/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopProcessFactory.java
@@ -38,6 +38,7 @@
 import static io.trino.plugin.atop.AtopErrorCode.ATOP_CANNOT_START_PROCESS_ERROR;
 import static io.trino.plugin.atop.AtopErrorCode.ATOP_READ_TIMEOUT;
 import static java.lang.String.format;
+import static java.nio.charset.Charset.defaultCharset;
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.Executors.newFixedThreadPool;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -97,7 +98,7 @@ private static final class AtopProcess
         private AtopProcess(Process process, Duration readTimeout, ExecutorService executor)
         {
             this.process = requireNonNull(process, "process is null");
-            underlyingReader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+            underlyingReader = new BufferedReader(new InputStreamReader(process.getInputStream(), defaultCharset()));
             TimeLimiter limiter = SimpleTimeLimiter.create(executor);
             this.reader = limiter.newProxy(underlyingReader::readLine, LineReader.class, readTimeout.toMillis(), MILLISECONDS);
             try {

diff --git a/plugin/trino-atop/src/test/java/io/trino/plugin/atop/TestingAtopFactory.java b/plugin/trino-atop/src/test/java/io/trino/plugin/atop/TestingAtopFactory.java
@@ -28,6 +28,7 @@
 import java.util.NoSuchElementException;
 
 import static java.lang.String.format;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 
 public class TestingAtopFactory
@@ -51,7 +52,7 @@ private static final class TestingAtop
         private TestingAtop(InputStream dataStream, ZonedDateTime date)
         {
             this.date = date;
-            this.reader = new BufferedReader(new InputStreamReader(dataStream));
+            this.reader = new BufferedReader(new InputStreamReader(dataStream, UTF_8));
             try {
                 line = reader.readLine();
             }

diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java
@@ -42,6 +42,7 @@
 import static io.trino.tpch.TpchTable.PART;
 import static io.trino.tpch.TpchTable.REGION;
 import static java.lang.String.format;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 public class DruidQueryRunner
@@ -112,7 +113,7 @@ private static void writeDataAsTsv(MaterializedResult rows, String dataFile)
             throws IOException
     {
         File file = new File(dataFile);
-        try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
+        try (BufferedWriter bw = new BufferedWriter(new FileWriter(file, UTF_8))) {
             for (MaterializedRow row : rows.getMaterializedRows()) {
                 bw.write(convertToTSV(row.getFields()));
                 bw.newLine();

diff --git a/plugin/trino-local-file/src/main/java/io/trino/plugin/localfile/LocalFileRecordCursor.java b/plugin/trino-local-file/src/main/java/io/trino/plugin/localfile/LocalFileRecordCursor.java
@@ -59,6 +59,7 @@
 import static io.trino.spi.type.VarcharType.createUnboundedVarcharType;
 import static java.lang.Math.toIntExact;
 import static java.lang.String.format;
+import static java.nio.charset.Charset.defaultCharset;
 import static java.time.format.DateTimeFormatter.ISO_OFFSET_DATE_TIME;
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -309,7 +310,7 @@ private BufferedReader createNextReader()
             FileInputStream fileInputStream = new FileInputStream(file);
 
             InputStream in = isGZipped(file) ? new GZIPInputStream(fileInputStream) : fileInputStream;
-            return new BufferedReader(new InputStreamReader(in));
+            return new BufferedReader(new InputStreamReader(in, defaultCharset()));
         }
 
         public static boolean isGZipped(File file)

diff --git a/plugin/trino-ml/src/main/java/io/trino/plugin/ml/SvmClassifier.java b/plugin/trino-ml/src/main/java/io/trino/plugin/ml/SvmClassifier.java
@@ -25,6 +25,7 @@
 import java.io.UncheckedIOException;
 
 import static io.trino.plugin.ml.type.ClassifierType.BIGINT_CLASSIFIER;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 
 public class SvmClassifier
@@ -50,7 +51,8 @@ public static SvmClassifier deserialize(byte[] modelData)
     {
         // TODO do something with the hyperparameters
         try {
-            svm_model model = svm.svm_load_model(new BufferedReader(new InputStreamReader(new ByteArrayInputStream(modelData))));
+            // UTF-8 should work, though the only thing we can say about the charset is that it's guaranteed to be 8-bits
+            svm_model model = svm.svm_load_model(new BufferedReader(new InputStreamReader(new ByteArrayInputStream(modelData), UTF_8)));
             return new SvmClassifier(model);
         }
         catch (IOException e) {

diff --git a/plugin/trino-ml/src/main/java/io/trino/plugin/ml/SvmRegressor.java b/plugin/trino-ml/src/main/java/io/trino/plugin/ml/SvmRegressor.java
@@ -25,6 +25,7 @@
 import java.io.UncheckedIOException;
 
 import static io.trino.plugin.ml.type.RegressorType.REGRESSOR;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 
 public class SvmRegressor
@@ -50,7 +51,8 @@ public static SvmRegressor deserialize(byte[] modelData)
     {
         // TODO do something with the hyperparameters
         try {
-            svm_model model = svm.svm_load_model(new BufferedReader(new InputStreamReader(new ByteArrayInputStream(modelData))));
+            // UTF-8 should work, though the only thing we can say about the charset is that it's guaranteed to be 8-bits
+            svm_model model = svm.svm_load_model(new BufferedReader(new InputStreamReader(new ByteArrayInputStream(modelData), UTF_8)));
             return new SvmRegressor(model);
         }
         catch (IOException e) {

diff --git a/...o-tpcds/src/main/java/io/trino/plugin/tpcds/statistics/TableStatisticsDataRepository.java b/...o-tpcds/src/main/java/io/trino/plugin/tpcds/statistics/TableStatisticsDataRepository.java
@@ -27,6 +27,7 @@
 import java.util.Optional;
 
 import static java.lang.String.format;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 public class TableStatisticsDataRepository
 {
@@ -58,7 +59,7 @@ private void writeStatistics(Path path, TableStatisticsData tableStatisticsData)
             objectMapper
                     .writerWithDefaultPrettyPrinter()
                     .writeValue(file, tableStatisticsData);
-            try (FileWriter fileWriter = new FileWriter(file, true)) {
+            try (FileWriter fileWriter = new FileWriter(file, UTF_8, true)) {
                 fileWriter.append('\n');
             }
         }

diff --git a/...ino-tpch/src/main/java/io/trino/plugin/tpch/statistics/TableStatisticsDataRepository.java b/...ino-tpch/src/main/java/io/trino/plugin/tpch/statistics/TableStatisticsDataRepository.java
@@ -28,6 +28,7 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static io.trino.plugin.tpch.util.Optionals.withBoth;
 import static java.lang.String.format;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 public class TableStatisticsDataRepository
 {
@@ -58,7 +59,7 @@ private void writeStatistics(Path path, TableStatisticsData tableStatisticsData)
             objectMapper
                     .writerWithDefaultPrettyPrinter()
                     .writeValue(file, tableStatisticsData);
-            try (FileWriter fileWriter = new FileWriter(file, true)) {
+            try (FileWriter fileWriter = new FileWriter(file, UTF_8, true)) {
                 fileWriter.append('\n');
             }
         }

diff --git a/pom.xml b/pom.xml
@@ -1893,6 +1893,7 @@
                                     -Xep:BoxedPrimitiveConstructor:ERROR \
                                     -Xep:ClassCanBeStatic:ERROR \
                                     -Xep:CompareToZero:ERROR \
+                                    -Xep:DefaultCharset:ERROR \
                                     -Xep:EqualsGetClass:OFF <!-- we would rather want the opposite check --> \
                                     -Xep:EqualsIncompatibleType:ERROR \
                                     -Xep:FallThrough:ERROR \

diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/JsonEventClient.java b/service/trino-verifier/src/main/java/io/trino/verifier/JsonEventClient.java
@@ -27,6 +27,7 @@
 import java.io.PrintStream;
 import java.io.UncheckedIOException;
 
+import static java.nio.charset.Charset.defaultCharset;
 import static java.util.Objects.requireNonNull;
 
 public class JsonEventClient
@@ -52,7 +53,7 @@ public <T> void postEvent(T event)
             ByteArrayOutputStream buffer = new ByteArrayOutputStream();
             JsonGenerator generator = factory.createGenerator(buffer, JsonEncoding.UTF8);
             serializer.serialize(event, generator);
-            out.println(buffer.toString().trim());
+            out.println(buffer.toString(defaultCharset()).trim());
         }
         catch (IOException e) {
             throw new UncheckedIOException(e);

diff --git a/...ing/trino-benchmark/src/main/java/io/trino/benchmark/SimpleLineBenchmarkResultWriter.java b/...ing/trino-benchmark/src/main/java/io/trino/benchmark/SimpleLineBenchmarkResultWriter.java
@@ -22,6 +22,7 @@
 import java.io.Writer;
 import java.util.Map;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 
 public class SimpleLineBenchmarkResultWriter
@@ -31,7 +32,7 @@ public class SimpleLineBenchmarkResultWriter
 
     public SimpleLineBenchmarkResultWriter(OutputStream outputStream)
     {
-        writer = new OutputStreamWriter(requireNonNull(outputStream, "outputStream is null"));
+        writer = new OutputStreamWriter(requireNonNull(outputStream, "outputStream is null"), UTF_8);
     }
 
     @Override

diff --git a/testing/trino-plugin-reader/src/main/java/io/trino/server/PluginReader.java b/testing/trino-plugin-reader/src/main/java/io/trino/server/PluginReader.java
@@ -143,7 +143,7 @@ private static Map<String, String> mapModulesToPlugins(File rootPom)
 
     private static List<String> readTrinoPlugins(File rootPom)
     {
-        try (FileReader fileReader = new FileReader(rootPom)) {
+        try (FileReader fileReader = new FileReader(rootPom, UTF_8)) {
             MavenXpp3Reader reader = new MavenXpp3Reader();
             Model model = reader.read(fileReader);
             return model.getModules().stream()
@@ -161,7 +161,7 @@ private static List<String> readTrinoPlugins(File rootPom)
     private static boolean isTrinoPlugin(String module)
     {
         String modulePom = module + "/pom.xml";
-        try (FileReader fileReader = new FileReader(modulePom)) {
+        try (FileReader fileReader = new FileReader(modulePom, UTF_8)) {
             MavenXpp3Reader reader = new MavenXpp3Reader();
             Model model = reader.read(fileReader);
             return model.getPackaging().equals("trino-plugin");