RockteMQ-AI commented on code in PR #2547:
URL: 
https://github.com/apache/rocketmq-dashboard/pull/2547#discussion_r3842686042


##########
server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java:
##########
@@ -185,11 +185,12 @@ public AclUserVO updateUser(UpdateAclUserDTO user, String 
instanceId) {
         if (isTencentInstance(instanceId)) {
             return tencentAclService.updateUser(instanceId, 
user.toAclUserVO());
         }
-        if (user.getId() == null) {
+        if (!StringUtils.hasText(user.getId())) {
             throw new BusinessException(400, "ACL user id is required");
         }
-        log.info("Updating ACL user id={}, username={}", user.getId(), 
user.getUsername());
-        AclUserVO existing = aclRepository.findUserById(user.getId())
+        Long userId = EntityIds.parseId(user.getId());
+        log.info("Updating ACL user id={}, username={}", userId, 
user.getUsername());
+        AclUserVO existing = aclRepository.findUserById(userId)

Review Comment:
   **[Warning]** `EntityIds.parseId(user.getId())` could throw 
`NumberFormatException` if a non-numeric string ID is passed for a non-Tencent 
instance. Consider wrapping this in a try-catch or validating earlier that the 
ID is numeric for local ACL rows, to return a proper 400 error instead of a 500.



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