You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/08/03 12:18:59 UTC
[PR] [ISSUE #850] Validate null nameserver requests [rocketmq-dashboard]
Aias00 opened a new pull request, #851:
URL: https://github.com/apache/rocketmq-dashboard/pull/851
### Summary
- Reject null NameServer write request bodies at the controller boundary
- Add service-level null guards before reading command fields
- Cover all NameServer write endpoints and direct service null-command calls
### Tests
- `JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home mvn -Dtest=NameServerControllerTest,ClusterServiceTest test`
Closes #850
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #850] Validate null nameserver requests [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #851:
URL: https://github.com/apache/rocketmq-dashboard/pull/851#issuecomment-5174171185
Superseded by consolidated PR #929, which contains this change and has passing checks. Closing this narrower PR to keep review focused.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #850] Validate null nameserver requests [rocketmq-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #851:
URL: https://github.com/apache/rocketmq-dashboard/pull/851#discussion_r3703890170
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameServerController.java:
##########
@@ -36,31 +37,45 @@ public class NameServerController {
private final ClusterService clusterService;
@PostMapping("/create")
- public Result<NameServerVO> createNameServer(@Valid @RequestBody CreateNameServerDTO command) {
+ public Result<NameServerVO> createNameServer(@Valid @RequestBody(required = false) CreateNameServerDTO command) {
+ requireCommand(command);
return Result.ok(clusterService.createNameServer(command));
}
@PostMapping("/update")
- public Result<Void> updateNameServer(@Valid @RequestBody UpdateNameServerDTO command) {
+ public Result<Void> updateNameServer(@Valid @RequestBody(required = false) UpdateNameServerDTO command) {
+ requireCommand(command);
clusterService.updateNameServer(command);
return Result.ok();
}
@PostMapping("/restart")
- public Result<Map<String, Boolean>> restartNameServer(@Valid @RequestBody RestartNameServerDTO command) {
+ public Result<Map<String, Boolean>> restartNameServer(
+ @Valid @RequestBody(required = false) RestartNameServerDTO command) {
+ requireCommand(command);
boolean success = clusterService.restartNameServer(command);
return Result.ok(Map.of("success", success));
}
@PostMapping("/upgrade")
- public Result<Map<String, Boolean>> upgradeNameServer(@Valid @RequestBody UpgradeNameServerDTO command) {
+ public Result<Map<String, Boolean>> upgradeNameServer(
+ @Valid @RequestBody(required = false) UpgradeNameServerDTO command) {
+ requireCommand(command);
boolean success = clusterService.upgradeNameServer(command);
return Result.ok(Map.of("success", success));
}
@PostMapping("/delete")
- public Result<Map<String, Boolean>> deleteNameServer(@Valid @RequestBody DeleteNameServerDTO command) {
+ public Result<Map<String, Boolean>> deleteNameServer(
+ @Valid @RequestBody(required = false) DeleteNameServerDTO command) {
+ requireCommand(command);
boolean success = clusterService.deleteNameServer(command);
return Result.ok(Map.of("success", success));
}
+
+ private void requireCommand(Object command) {
+ if (command == null) {
+ throw new BusinessException(400, "NameServer request is required");
+ }
+ }
Review Comment:
The HTTP code `400` and message string are hard-coded here and also duplicated in `ClusterService.requireNameServerCommand`, which risks future drift (e.g., one side changed but not the other). Consider extracting these into shared constants (or a small shared helper/factory for `BusinessException`) and using `HttpStatus.BAD_REQUEST.value()` (or an existing project-wide constant) instead of the magic number.
##########
server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java:
##########
@@ -71,6 +71,28 @@ void createNameServerShouldPassValidatedRequest() throws Exception {
verify(clusterService).createNameServer(any(CreateNameServerDTO.class));
}
+ @Test
+ void nameServerWriteEndpointsShouldRejectNullRequestBody() throws Exception {
+ String[] paths = {
+ "/api/nameservers/create",
+ "/api/nameservers/update",
+ "/api/nameservers/restart",
+ "/api/nameservers/upgrade",
+ "/api/nameservers/delete"
+ };
+
+ for (String path : paths) {
+ mockMvc.perform(post(path)
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("NameServer request is required"));
+ }
Review Comment:
This test covers the explicit JSON literal `null`, but the controller changes also set `@RequestBody(required = false)`, which affects the behavior when the body is completely missing. Add an additional assertion per endpoint for the 'no body sent' case (i.e., omit `.content(...)`) to ensure the new controller guard consistently returns the expected 400 error in that scenario too.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #850] Validate null nameserver requests [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 closed pull request #851: [ISSUE #850] Validate null nameserver requests
URL: https://github.com/apache/rocketmq-dashboard/pull/851
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org