[GitHub] [rocketmq] vongosling commented on a change in pull request #2893: fix the problem of potential NPE in ACL plain

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

vongosling commented on a change in pull request #2893:
URL: https://github.com/apache/rocketmq/pull/2893#discussion_r634902460



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -50,9 +50,9 @@
 
     private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
 
-    private  Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();

Review comment:
       Do you check your style correct, just like here.?https://github.com/apache/rocketmq/pull/2899

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -73,14 +73,14 @@ public void load() {
         JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
             JSONObject.class);
         if (plainAclConfData == null || plainAclConfData.isEmpty()) {
-            throw new AclException(String.format("%s file is not data", fileHome + File.separator + fileName));
+            throw new AclException(String.format("%s file  is not data", fileHome + File.separator + fileName));

Review comment:
       the middle of file and is has two blanks, right?

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -133,7 +133,9 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
 
         Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
             Map.class);
-
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("%s file  is not data", fileHome + File.separator + fileName));

Review comment:
       what is file is not data, is it right sematic?




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