rgw: fix opslog cannot record remote_addr #16860
Conversation
|
Jenkins, retest this please. |
src/rgw/rgw_civetweb.cc
Outdated
| @@ -120,6 +120,10 @@ void RGWCivetWeb::init_env(CephContext *cct) | |||
| env.set("REMOTE_USER", info->remote_user); | |||
| } | |||
|
|
|||
| if (info->remote_addr) { | |||
There was a problem hiding this comment.
Would it be better to check the string length too ?
strlen(info->remote_addr) != 0
or
if(info->remote_addr[0] != '\0')
There was a problem hiding this comment.
Even if remote_add is empty, it's still fine(ops log entry default value is empty string), I'm fine with adding length check here, the concern is if we've introduced the length check, should we also do this for info->remote_user?
So in order to keep consistency with surrounding code, I'd prefer we don't add length check.
There was a problem hiding this comment.
@mikulely Agreed with your point. But I don't think there is a possibility of if (info->remote_addr) would take a false branch. Check struct mg_request_info{};.
There was a problem hiding this comment.
@joscollin Agree, dropped.
joscollin
changed the title
|
Jenkins, retest this please. |
|
Jenkins, retest this please. |
Fixes: http://tracker.ceph.com/issues/20931 Reported-by: Zhang Shaowen <zhangshaowen@cmss.chinamobile.com> Signed-off-by: Jiaying Ren <jiaying.ren@umcloud.com>
|
Jenkins, retest this please. |
|
Jenkins, retest this please. |
1 similar comment
|
Jenkins, retest this please. |
|
Jenkins retest this please |
Fixes: http://tracker.ceph.com/issues/20931
Signed-off-by: Jiaying Ren jiaying.ren@umcloud.com