Skip to content
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.

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

rgw: fix opslog can't record referrer when using curl as client #16863

Merged
merged 1 commit into from Aug 29, 2017

Conversation

mikulely
Copy link
Contributor

@mikulely mikulely commented Aug 7, 2017

@mikulely mikulely changed the title rgw opslog cant not record referrer when using curl as client rgw: fix opslog cant not record referrer when using curl as client Aug 7, 2017
@mikulely mikulely force-pushed the fix-curl-referer branch 3 times, most recently from 975e5dd to dda244d Compare August 7, 2017 10:00
@mikulely
Copy link
Contributor Author

mikulely commented Aug 8, 2017

@joscollin mind to take a look? Thx.

@joscollin joscollin self-requested a review August 8, 2017 06:07
@joscollin joscollin changed the title rgw: fix opslog cant not record referrer when using curl as client rgw: fix opslog can't record referrer when using curl as client Aug 8, 2017
@@ -346,7 +346,11 @@ int rgw_log_op(RGWRados *store, RGWREST* const rest, struct req_state *s,
else
set_param_str(s, "REMOTE_ADDR", entry.remote_addr);
set_param_str(s, "HTTP_USER_AGENT", entry.user_agent);
set_param_str(s, "HTTP_REFERRER", entry.referrer);
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be fixed as HTTP_REFERER for now and do the entire misspelling clean up as a different PR. Because after we fix the misspelling, the below logic won't be needed at all. What do you say ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As AWS S3 are using referrer (see http://docs.aws.amazon.com/AmazonS3/latest/dev/LogFormat.html Referrer Section), rgw are correctly following this convention, but curl -e option are still using misspelling referer . I think it's a problem of legacy client, this purpose of this pr is to improve the rgw compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

makes sense

Fixes: http://tracker.ceph.com/issues/20935

Reported-by: Zhang Shaowen <zhangshaowen@cmss.chinamobile.com>
Signed-off-by: Jiaying Ren <jiaying.ren@umcloud.com>
@mikulely
Copy link
Contributor Author

Jenkins, retest this please.

1 similar comment
@mikulely
Copy link
Contributor Author

Jenkins, retest this please.

Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

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

LGTM

@joscollin
Copy link
Member

Jenkins retest this please

@yehudasa yehudasa merged commit 9bfd96e into ceph:master Aug 29, 2017
@mikulely mikulely deleted the fix-curl-referer branch August 30, 2017 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants