ai-yang opened a new issue, #757:
URL: https://github.com/apache/rocketmq-dashboard/issues/757

   ## Affected branch
   
   `rocketmq-studio` at `bbf1b7e0cf25a5065ba049b5450cc8155569f710`
   
   ## Problem
   
   `POST /api/acl/rules/update` creates a new ACL rule when the supplied ID 
does not exist.
   
   `AclService.updateRule` only checks that the ID is non-blank, assigns 
`createdAt` when it is absent, and calls `AclRepository.saveRule`. 
`InMemoryAclRepository.saveRule` uses `ConcurrentHashMap.put`, so the update 
path has upsert semantics instead of update semantics.
   
   This is inconsistent with other Studio update endpoints, including ACL user 
updates, alert rule updates, and data source updates, which return `404` for 
unknown IDs.
   
   ## Deterministic reproduction
   
   1. Create an empty `InMemoryAclRepository` and an `AclService` backed by it.
   2. Call `updateRule` with `id = "missing-rule"` and otherwise valid rule 
fields.
   3. Query all rules.
   
   Expected:
   
   - `updateRule` throws `BusinessException(404, "ACL rule not found: 
missing-rule")`.
   - The repository remains empty.
   
   Actual:
   
   - The call succeeds.
   - A new rule with ID `missing-rule` appears in the repository.
   
   A regression test using this sequence failed 5/5 on the unmodified branch 
under Java 21, without sleeps or randomized concurrency:
   
   ```text
   AclServiceTest#updateRuleShouldRejectUnknownIdInsteadOfCreatingRule
   Expecting code to raise a throwable.
   ```
   
   ## Impact
   
   A stale client, typo, or retried edit can silently create a new ACL rule 
instead of reporting that the original rule no longer exists. It also bypasses 
the intended create endpoint and its generated-ID semantics.
   
   ## Suggested fix
   
   - Give the repository an atomic replace operation 
(`ConcurrentHashMap.replace` or `computeIfPresent`) rather than implementing 
update as `put`.
   - Return `404` when no entry was replaced.
   - Preserve the original creation timestamp and keep create/update validation 
consistent.
   - Add service and controller regression coverage for unknown IDs.
   
   ## Related work checked
   
   - #516 introduced the ACL update endpoint.
   - #636 validates ACL create/delete requests but does not reject unknown 
update IDs.
   - #622 and #631 only harden frontend/mock copy behavior.
   
   Searches across open and closed issues and pull requests found no existing 
fix for the unknown-ID upsert behavior.
   


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