Merge pull request #3176 from yuz10/develop3

[Issue #3175] check acl config before update. otherwise broker may fail to start
apache · Jul 31, 2021 · 9e8fea9 · 9e8fea9
2 parents 94abced + d21266d
commit 9e8fea9
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 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
@@ -130,6 +130,7 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
             log.error("Parameter value plainAccessConfig is null,Please check your parameter");
             throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
         }
+        checkPlainAccessConfig(plainAccessConfig);
 
         Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
         Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
@@ -357,15 +358,19 @@ void clearPermissionInfo() {
         this.globalWhiteRemoteAddressStrategy.clear();
     }
 
-    public PlainAccessResource buildPlainAccessResource(PlainAccessConfig plainAccessConfig) throws AclException {
+    public void checkPlainAccessConfig(PlainAccessConfig plainAccessConfig) throws AclException {
         if (plainAccessConfig.getAccessKey() == null
-            || plainAccessConfig.getSecretKey() == null
-            || plainAccessConfig.getAccessKey().length() <= AclConstants.ACCESS_KEY_MIN_LENGTH
-            || plainAccessConfig.getSecretKey().length() <= AclConstants.SECRET_KEY_MIN_LENGTH) {
+                || plainAccessConfig.getSecretKey() == null
+                || plainAccessConfig.getAccessKey().length() <= AclConstants.ACCESS_KEY_MIN_LENGTH
+                || 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()));
+                    "The accessKey=%s and secretKey=%s cannot be null and length should longer than 6",
+                    plainAccessConfig.getAccessKey(), plainAccessConfig.getSecretKey()));
         }
+    }
+
+    public PlainAccessResource buildPlainAccessResource(PlainAccessConfig plainAccessConfig) throws AclException {
+        checkPlainAccessConfig(plainAccessConfig);
         PlainAccessResource plainAccessResource = new PlainAccessResource();
         plainAccessResource.setAccessKey(plainAccessConfig.getAccessKey());
         plainAccessResource.setSecretKey(plainAccessConfig.getSecretKey());

diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
@@ -566,6 +566,16 @@ public void createAndUpdateAccessAclYamlConfigExceptionTest() {
         plainAccessValidator.updateAccessConfig(plainAccessConfig);
     }
 
+    @Test(expected = AclException.class)
+    public void createAndUpdateAccessAclNullSkExceptionTest() {
+        PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
+        plainAccessConfig.setAccessKey("RocketMQ33");
+        // secret key is null
+
+        PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
+        plainAccessValidator.updateAccessConfig(plainAccessConfig);
+    }
+
     @Test
     public void updateGlobalWhiteAddrsNormalTest() {
         System.setProperty("rocketmq.home.dir", "src/test/resources");