From 7b7e45bca15500e3f36940180c87f80ceab291eb Mon Sep 17 00:00:00 2001
From: shendong <shendong@cmss.chinamobile.com>
Date: Fri, 14 May 2021 14:25:44 +0800
Subject: [PATCH] fix the problem of potential NPE in ACL plain

---
 .../acl/plain/PlainPermissionManager.java     | 43 +++++++++++--------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
index c182d7eb..97bd6bc8 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
@@ -50,9 +50,9 @@ public class PlainPermissionManager {
 
     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<>();
 
-    private  List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
+    private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
@@ -80,7 +80,7 @@ public class PlainPermissionManager {
         if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
             for (int i = 0; i < globalWhiteRemoteAddressesList.size(); i++) {
                 globalWhiteRemoteAddressStrategy.add(remoteAddressStrategyFactory.
-                        getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(i)));
+                    getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(i)));
             }
         }
 
@@ -89,7 +89,7 @@ public class PlainPermissionManager {
             List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
             for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
                 PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
-                plainAccessResourceMap.put(plainAccessResource.getAccessKey(),plainAccessResource);
+                plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
             }
         }
 
@@ -133,7 +133,9 @@ public class PlainPermissionManager {
 
         Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
             Map.class);
-
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("%s file not found or isEmpty", fileHome + File.separator + fileName));
+        }
         List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
         Map<String, Object> updateAccountMap = null;
         if (accounts != null) {
@@ -164,20 +166,21 @@ public class PlainPermissionManager {
         return false;
     }
 
-    private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existedAccountMap, PlainAccessConfig plainAccessConfig) {
-        
+    private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existedAccoutMap,
+        PlainAccessConfig plainAccessConfig) {
+
         Map<String, Object> newAccountsMap = null;
-        if (existedAccountMap == null) {
+        if (existedAccoutMap == null) {
             newAccountsMap = new LinkedHashMap<String, Object>();
         } else {
-            newAccountsMap = existedAccountMap;
+            newAccountsMap = existedAccoutMap;
         }
 
         if (StringUtils.isEmpty(plainAccessConfig.getAccessKey()) ||
             plainAccessConfig.getAccessKey().length() <= AclConstants.ACCESS_KEY_MIN_LENGTH) {
             throw new AclException(String.format(
-                    "The accessKey=%s cannot be null and length should longer than 6",
-                    plainAccessConfig.getAccessKey()));
+                "The accessKey=%s cannot be null and length should longer than 6",
+                plainAccessConfig.getAccessKey()));
         }
         newAccountsMap.put(AclConstants.CONFIG_ACCESS_KEY, plainAccessConfig.getAccessKey());
 
@@ -218,8 +221,10 @@ public class PlainPermissionManager {
         }
 
         Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
-                    Map.class);
-
+            Map.class);
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("%s file not found or isEmpty", fileHome + File.separator + fileName));
+        }
         List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get("accounts");
         if (accounts != null) {
             Iterator<Map<String, Object>> itemIterator = accounts.iterator();
@@ -251,7 +256,9 @@ public class PlainPermissionManager {
 
         Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
             Map.class);
-
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("%s file not found or isEmpty", fileHome + File.separator + fileName));
+        }
         List<String> globalWhiteRemoteAddrList = (List<String>) aclAccessConfigMap.get(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS);
 
         if (globalWhiteRemoteAddrList != null) {
@@ -259,7 +266,7 @@ public class PlainPermissionManager {
             globalWhiteRemoteAddrList.addAll(globalWhiteAddrsList);
 
             // Update globalWhiteRemoteAddr element in memeory map firstly
-            aclAccessConfigMap.put(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS,globalWhiteRemoteAddrList);
+            aclAccessConfigMap.put(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS, globalWhiteRemoteAddrList);
             if (AclUtils.writeDataObject(fileHome + File.separator + fileName, updateAclConfigFileVersion(aclAccessConfigMap))) {
                 return true;
             }
@@ -275,7 +282,7 @@ public class PlainPermissionManager {
         List<PlainAccessConfig> configs = new ArrayList<>();
         List<String> whiteAddrs = new ArrayList<>();
         JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
-                JSONObject.class);
+            JSONObject.class);
         if (plainAclConfData == null || plainAclConfData.isEmpty()) {
             throw new AclException(String.format("%s file is not data", fileHome + File.separator + fileName));
         }
@@ -359,7 +366,7 @@ public class PlainPermissionManager {
             || plainAccessConfig.getSecretKey().length() <= AclConstants.SECRET_KEY_MIN_LENGTH) {
             throw new AclException(String.format(
                 "The accessKey=%s and secretKey=%s cannot be null and length should longer than 6",
-                    plainAccessConfig.getAccessKey(), plainAccessConfig.getSecretKey()));
+                plainAccessConfig.getAccessKey(), plainAccessConfig.getSecretKey()));
         }
         PlainAccessResource plainAccessResource = new PlainAccessResource();
         plainAccessResource.setAccessKey(plainAccessConfig.getAccessKey());
@@ -375,7 +382,7 @@ public class PlainPermissionManager {
         Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());
 
         plainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
-                getRemoteAddressStrategy(plainAccessResource.getWhiteRemoteAddress()));
+            getRemoteAddressStrategy(plainAccessResource.getWhiteRemoteAddress()));
 
         return plainAccessResource;
     }
-- 
GitLab