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 13:16:11 UTC
[PR] [ISSUE #858] Validate null topic requests [rocketmq-dashboard]
Aias00 opened a new pull request, #859:
URL: https://github.com/apache/rocketmq-dashboard/pull/859
### Summary
- Reject null topic create/update/delete/send request bodies at the controller boundary
- Add metadata service null guards before forwarding topic create/update/send commands to the admin client
- Cover controller and service null-request paths with tests
### Tests
- `JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home mvn -Dtest=TopicControllerTest,MetadataServiceTest test`
Closes #858
--
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 #858] Validate null topic requests [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 closed pull request #859: [ISSUE #858] Validate null topic requests
URL: https://github.com/apache/rocketmq-dashboard/pull/859
--
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 #858] Validate null topic requests [rocketmq-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #859:
URL: https://github.com/apache/rocketmq-dashboard/pull/859#discussion_r3704338307
##########
server/src/main/java/org/apache/rocketmq/studio/instance/topic/MetadataService.java:
##########
@@ -121,4 +125,16 @@ public List<NamespaceVO> listNamespaces() {
private String normalizeFilter(String value) {
return value == null || value.isBlank() ? null : value.trim();
}
+
+ private void requireTopic(TopicVO topic) {
+ if (topic == null) {
+ throw new BusinessException(400, "Topic request is required");
+ }
+ }
+
+ private void requireSendMessageRequest(SendMessageDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "Topic send message request is required");
+ }
+ }
Review Comment:
As in `TopicController`, this introduces duplicated hard-coded status/message literals. Centralizing these constants (and using a named status/error-code constant instead of `400`) would make it easier to keep controller/service behavior consistent over time.
##########
server/src/main/java/org/apache/rocketmq/studio/instance/topic/TopicController.java:
##########
@@ -71,7 +75,26 @@ public Result<List<TopicConsumerVO>> getTopicConsumers(@PathVariable String name
}
@PostMapping("/send")
- public Result<SendMessageVO> sendMessage(@RequestBody SendMessageDTO request) {
+ public Result<SendMessageVO> sendMessage(@RequestBody(required = false) SendMessageDTO request) {
+ requireSendMessageRequest(request);
return Result.ok(metadataService.sendMessage(request));
}
+
+ private void requireTopicRequest(TopicVO topic) {
+ if (topic == null) {
+ throw new BusinessException(400, "Topic request is required");
+ }
+ }
+
+ private void requireDeleteTopicRequest(DeleteTopicDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "Topic delete request is required");
+ }
+ }
+
+ private void requireSendMessageRequest(SendMessageDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "Topic send message request is required");
+ }
+ }
Review Comment:
The status code and error messages are hard-coded literals here. To reduce duplication and prevent drift across controller/service/tests, consider using a shared constant/source for these messages and using a named status constant (e.g., `HttpStatus.BAD_REQUEST.value()` or a project-wide error code constant) instead of the magic number `400`.
##########
server/src/test/java/org/apache/rocketmq/studio/instance/topic/TopicControllerTest.java:
##########
@@ -108,6 +108,39 @@ void createTopicShouldReturnCreatedTopic() throws Exception {
.andExpect(jsonPath("$.data.writeQueues").value(16));
}
+ @Test
+ void topicWriteEndpointsShouldRejectNullRequestBody() throws Exception {
+ mockMvc.perform(post("/api/topics/create")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("Topic request is required"));
+
+ mockMvc.perform(post("/api/topics/update")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("Topic request is required"));
+
+ mockMvc.perform(post("/api/topics/delete")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("Topic delete request is required"));
+
+ mockMvc.perform(post("/api/topics/send")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("Topic send message request is required"));
Review Comment:
These tests cover the JSON literal `null`, but not other common 'missing body' forms (e.g., empty body with `Content-Type: application/json`, or omitting the body entirely). Since the controller now uses `@RequestBody(required = false)`, adding coverage for empty/missing request bodies would better validate the intended controller-boundary 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #858] Validate null topic requests [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #859:
URL: https://github.com/apache/rocketmq-dashboard/pull/859#issuecomment-5174205412
Superseded by consolidated PR #930, 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