Skip to content

refactor: ClientRequestProcessor#process_request #4033

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 1 commit into from
Sep 12, 2025
Merged

refactor: ClientRequestProcessor#process_request #4033

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 4 additions & 14 deletions rocketmq-namesrv/src/processor/client_request_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,17 @@ impl RequestProcessor for ClientRequestProcessor {
#[inline]
async fn process_request(
&mut self,
channel: Channel,
ctx: ConnectionHandlerContext,
_channel: Channel,
_ctx: ConnectionHandlerContext,
request: &mut RemotingCommand,
) -> rocketmq_error::RocketMQResult<Option<RemotingCommand>> {
let request_code = RequestCode::from(request.code());
info!(
"Name server ClientRequestProcessor Received request code: {:?}",
request_code
);
self.process_request_inner(channel, ctx, request_code, request)

self.get_route_info_by_topic(request)
}
Comment on lines +58 to 60
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don’t unconditionally route every request to get_route_info_by_topic — this breaks non-105 request handling.

As written, all request codes attempt to decode GetRouteInfoRequestHeader, which will error for other codes and can surface as protocol failures. Restore dispatch on request_code and return a proper “not supported” response for others.

Apply this diff:

-        self.get_route_info_by_topic(request)
+        match request_code {
+            RequestCode::GetRouteinfoByTopic => self.get_route_info_by_topic(request),
+            other => {
+                warn!("Unsupported request code: {:?}.", other);
+                Ok(Some(
+                    RemotingCommand::create_response_command_with_code(
+                        RemotingSysResponseCode::RequestCodeNotSupported,
+                    )
+                    .set_remark(format!("Unsupported request code: {:?}", other)),
+                ))
+            }
+        }

If RemotingSysResponseCode::RequestCodeNotSupported doesn’t exist in this codebase, fall back to RemotingSysResponseCode::SystemError with a clear remark.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self.get_route_info_by_topic(request)
}
match request_code {
RequestCode::GetRouteInfoByTopic => self.get_route_info_by_topic(request),
other => {
warn!("Unsupported request code: {:?}.", other);
Ok(Some(
RemotingCommand::create_response_command_with_code(
RemotingSysResponseCode::SystemError,
)
.set_remark(format!("Unsupported request code: {:?}", other)),
))
}
}
}
🤖 Prompt for AI Agents 
In rocketmq-namesrv/src/processor/client_request_processor.rs around lines 58 to
60, the current code unconditionally calls get_route_info_by_topic(request)
causing every request to be decoded as GetRouteInfoRequestHeader which fails for
non-105 request codes; restore proper dispatch by matching on
request.request_code (or equivalent) and only call get_route_info_by_topic for
the GetRouteInfo request code (105), and for all other codes return an immediate
RemotingCommand response with RemotingSysResponseCode::RequestCodeNotSupported
(or if that variant is absent, use RemotingSysResponseCode::SystemError) and set
a clear remark stating the request code is not supported to avoid protocol
decode errors.

}

Expand All @@ -68,17 +69,6 @@ impl ClientRequestProcessor {
}
}

#[inline]
fn process_request_inner(
&mut self,
_channel: Channel,
_ctx: ConnectionHandlerContext,
_request_code: RequestCode,
request: &mut RemotingCommand,
) -> rocketmq_error::RocketMQResult<Option<RemotingCommand>> {
self.get_route_info_by_topic(request)
}

fn get_route_info_by_topic(
&self,
request: &mut RemotingCommand,
Expand Down
Loading