[GitHub] [ozone] adoroszlai commented on a change in pull request #1673: HDDS-4559. Avoid using hard coding uft-8 charset

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #1673:
URL: https://github.com/apache/ozone/pull/1673#discussion_r538083644



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -351,7 +352,7 @@ private OzoneConsts() {
   public static final String GDPR_FLAG = "gdprEnabled";
   public static final String GDPR_ALGORITHM_NAME = "AES";
   public static final int GDPR_DEFAULT_RANDOM_SECRET_LENGTH = 16;
-  public static final String GDPR_CHARSET = "UTF-8";
+  public static final String GDPR_CHARSET = StandardCharsets.UTF_8.name();

Review comment:
       Both usages of `GDPR_CHARSET` are in calls to methods where both `String` and `Charset` versions exist, so we can make this change:
   
   ```suggestion
     public static final Charset GDPR_CHARSET = StandardCharsets.UTF_8;
   ```
   
   Since it is only used in `GDPRSymmetricKey`, I would also consider inlining this constant.  I don't think we need to define it globally.

##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java
##########
@@ -75,7 +76,7 @@
   private Path dnMetaDirPath;
   private SecurityConfig omSecurityConfig;
   private SecurityConfig dnSecurityConfig;
-  private final static String UTF = "UTF-8";
+  private final static String UTF = StandardCharsets.UTF_8.name();

Review comment:
       This is used in two kinds of calls:
   
   1. `RandomStringUtils.random`: its `chars` parameter is not intended for charsets, rather the set of characters to use.  Eg. for `"UTF-8"` it will generate strings like `"--8F8T8U8T"`.  Since we don't need this special kind of random string, this usage can be simply removed.
   2. `IOUtils.toInputStream`: can accept `Charset`, too.
   
   After removing the first kind of usage, we can convert this constant to `Charset`.  (I would also inline it, ie. simply use `UTF_8`.)

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/AWSV4AuthValidator.java
##########
@@ -42,7 +42,7 @@
   private final static Logger LOG =
       LoggerFactory.getLogger(AWSV4AuthValidator.class);
   private static final String HMAC_SHA256_ALGORITHM = "HmacSHA256";
-  private static final Charset UTF_8 = Charset.forName("utf-8");
+  private static final Charset UTF_8 = StandardCharsets.UTF_8;

Review comment:
       Nit: this constant can be replaced with `import static java.nio.charset.StandardCharsets.UTF_8;`.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/scm/cli/SQLCLI.java
##########
@@ -72,7 +73,7 @@
 
   private Options options;
   private BasicParser parser;
-  private final Charset encoding = Charset.forName("UTF-8");
+  private final Charset encoding = StandardCharsets.UTF_8;

Review comment:
       `encoding` is unused, can be removed.

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignatureProcessor.java
##########
@@ -32,7 +33,7 @@
   String X_AMAZ_DATE = "X-Amz-Date";
   String CONTENT_MD5 = "content-md5";
   String AUTHORIZATION_HEADER = "Authorization";
-  Charset UTF_8 = Charset.forName("utf-8");
+  Charset UTF_8 = StandardCharsets.UTF_8;

Review comment:
       Nit: usages of this constant can be replaced with the one from `StandardCharsets`.




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org