You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/11/19 18:56:07 UTC

[GitHub] [accumulo] milleruntime commented on a change in pull request #1795: Read/Write config files consistently

milleruntime commented on a change in pull request #1795:
URL: https://github.com/apache/accumulo/pull/1795#discussion_r527123471



##########
File path: start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
##########
@@ -87,7 +89,7 @@ public static String getAccumuloProperty(String propertyName, String defaultValu
     }
     try {
       var config = new PropertiesConfiguration();
-      try (var reader = new FileReader(accumuloConfigUrl.getFile())) {
+      try (var reader = new InputStreamReader(accumuloConfigUrl.openStream(), UTF_8)) {

Review comment:
       Looks like this changed got dinged by Sec bugs.  Should triage this before committing. 

##########
File path: minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShellOptions.java
##########
@@ -69,8 +70,8 @@ public RemoteShellOptions() {
       if (f.exists() && f.isFile() && f.canRead()) {
         FileReader reader = null;
         try {
-          reader = new FileReader(f);
-        } catch (FileNotFoundException e) {
+          reader = new FileReader(f, UTF_8);
+        } catch (IOException e) {

Review comment:
       It's odd that this constructor throws a more general exception, I would think it would have been the other way around.  




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org