Skip to content

fix the problem of potential NPE in ACL plain #2893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 23, 2021
Merged

fix the problem of potential NPE in ACL plain #2893

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b219c16
fix the problem of potential NPE in ACL plain
May 14, 2021
7b7e45b
fix the problem of potential NPE in ACL plain
May 14, 2021
4f48fb7
gerge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 20, 2021
85168cd
fix the problem of potential NPE in ACL plain
May 14, 2021
7af4307
Merge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 20, 2021
7804c74
Merge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 20, 2021
cd183b8
Merge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 20, 2021
99b69ab
Merge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 20, 2021
e320775
Merge branch 'develop' of https://github.com/shendongsd/rocketmq into…
May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you check your style correct, just like here.?#2899

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you check your style correct, just like here.?#2899

yes, i checked


private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();

private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();

Expand Down Expand Up @@ -80,7 +80,7 @@ public void load() {
if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
for (int i = 0; i < globalWhiteRemoteAddressesList.size(); i++) {
globalWhiteRemoteAddressStrategy.add(remoteAddressStrategyFactory.
getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(i)));
getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(i)));
}
}

Expand All @@ -89,7 +89,7 @@ public void load() {
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);
}
}

Expand Down Expand Up @@ -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("the %s file is not found or empty", 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) {
Expand Down Expand Up @@ -164,8 +166,9 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
return false;
}

private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existedAccountMap, PlainAccessConfig plainAccessConfig) {

private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existedAccountMap,
PlainAccessConfig plainAccessConfig) {

Map<String, Object> newAccountsMap = null;
if (existedAccountMap == null) {
newAccountsMap = new LinkedHashMap<String, Object>();
Expand All @@ -176,8 +179,8 @@ private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existed
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());

Expand Down Expand Up @@ -218,8 +221,10 @@ public boolean deleteAccessConfig(String accesskey) {
}

Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
Map.class);

Map.class);
if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
throw new AclException(String.format("the %s file is not found or empty", 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();
Expand Down Expand Up @@ -251,15 +256,17 @@ public boolean updateGlobalWhiteAddrsConfig(List<String> globalWhiteAddrsList) {

Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
Map.class);

if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
throw new AclException(String.format("the %s file is not found or empty", fileHome + File.separator + fileName));
}
List<String> globalWhiteRemoteAddrList = (List<String>) aclAccessConfigMap.get(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS);

if (globalWhiteRemoteAddrList != null) {
globalWhiteRemoteAddrList.clear();
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;
}
Expand All @@ -275,7 +282,7 @@ public AclConfig getAllAclConfig() {
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));
}
Expand Down Expand Up @@ -359,7 +366,7 @@ public PlainAccessResource buildPlainAccessResource(PlainAccessConfig plainAcces
|| 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());
Expand All @@ -375,7 +382,7 @@ public PlainAccessResource buildPlainAccessResource(PlainAccessConfig plainAcces
Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());

plainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
getRemoteAddressStrategy(plainAccessResource.getWhiteRemoteAddress()));
getRemoteAddressStrategy(plainAccessResource.getWhiteRemoteAddress()));

return plainAccessResource;
}
Expand Down