ubeddulla commented on PR #3598: URL: https://github.com/apache/brpc/pull/3598#issuecomment-6142262458
Thanks for the automated pass. These paths are all deliberately shaped to match the existing conventions, so I've kept them as-is: - ODR on `MyServerAuthenticator`: each `brpc_*_unittest.cpp` is built into its own executable (see the glob + per-file `add_executable` in test/CMakeLists.txt), so the redis and rtmp definitions never link into the same image. This is the same reason `MyAuthenticator` already lives at global scope in ~8 other test files. - Mongo verify hook: `VerifyMongoRequest` is a line-for-line mirror of `VerifyNsheadRequest`/`VerifyThriftRequest`, including the C-style `(Server*)msg_base->arg()` cast and the "does not support authentication" message. Mongo carries no credential, so there's nothing `IsAuthenticated()` could ever flip to true; rejecting unconditionally when `options().auth` is set is the intended fail-closed behavior, same as nshead/thrift. - Redis/rtmp log line: this matches the merged streaming check from #3481 (`Reject ... from unauthenticated " << *socket`). Happy to switch to `LOG_EVERY_SECOND` if you'd prefer, but I kept it identical to the sibling path for now. The point of the PR is consistency with the protocols that already fail closed, so I'd rather not diverge on these unless you want the whole family changed together. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
