caigy opened a new issue, #4155:
URL: https://github.com/apache/rocketmq/issues/4155

   **BUG REPORT**
   
   1. Please describe the issue you observed:
   
   - What did you do (The steps to reproduce)?
   Check out `develop` branch and run `mvn test` under `acl` directory.
   
   - What is expected to see?
   All tests should be successful.
   
   - What did you see instead?
   Tests may fail with results below:
   ```
   Results :
   Failed tests: 
     
PlainAccessControlFlowTest.testOnlyAclFolderCase:170->testValidationAfterConfigFileChanged:228
 Message should not be consumed after account deleted
   ```
   
   2. Please tell us about your environment:
   - Java version: 1.8.0_292
   - maven: 3.6.3
   
   3. Other information (e.g. detailed explanation, logs, related issues, 
suggestions on how to fix, etc):
   Reproduction conditions:
   
   - Running tests with `mvn test` can reproduce the problem, but running test 
in GUI of Intelij IDE will not.
   - Running tests in rocketmq-acl together can reproduce the problem, but 
running all tests only in `PlainAccessControlFlowTest` will not.
   
   
   **Cause analysis**
   
   In 
`org.apache.rocketmq.acl.plain.PlainAccessControlFlowTest#testValidationAfterConfigFileChanged`,
  consumer access key (ak22222) is deleted in config file and then 
`org.apache.rocketmq.acl.plain.PlainPermissionManager#load(aclFilePath)` is 
called to load the content of config file previously updated.
   The loading logic is as follows:
   
   
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L238-L249
   
   After I printed the content of 
`org.apache.rocketmq.acl.plain.PlainPermissionManager#accessKeyTable`, I found 
something strange: the file path of `ak22222` is `...//conf/plain_acl.yml` (pay 
attention to the double slashes), but  
`PlainPermissionManager#load(aclFilePath)` was called with 
`/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/plain_acl.yml`
 as parameter. 
   ```
   
{ak22222=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf//conf/plain_acl.yml,
 
rocketmq2=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/acl/plain_acl.yml,
 
ak11111=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf//conf/plain_acl.yml,
 
RocketMQ=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/acl/plain_acl.yml}
   ```
   According to the loading logic above, access resource will not be updated.
   
   So where does the strange `//` come from? 
   In 
`org.apache.rocketmq.acl.plain.PlainAccessValidatorTest#deleteAccessAclToEmptyTest`,
 the system property `rocketmq.acl.plain.file` was set:
   
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java#L908-L917
   
   In `PlainPermissionManager`, `File.separator` was prepended to  the value of 
property `rocketmq.acl.plain.file`:
   
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L63
   
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L124-L127
   
   
   
   **Fix**
   
   - In real scenario,  `rocketmq.acl.plain.file` would not be set multiple 
times in one process, so I would fix this issue in a simple and direct way: 
removing redundant file separator in  `PlainAccessValidatorTest`.
   - Using file path as key in `PlainPermissionManager#load(aclFilePath)` is 
still risky, I would open another issue to normalize file paths used as key.
   


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to