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

civetweb: process unreadable char in header correct #28

Closed
wants to merge 1 commit into from
Closed

civetweb: process unreadable char in header correct #28

wants to merge 1 commit into from

Conversation

joke-lee
Copy link

in recent ceph/ceph master branch , we can not passed some test which contain unreadable cha in header, such as

test_object_create_bad_md5_unreadable
test_object_create_bad_expect_unreadable 
test_object_create_bad_contentlength_empty 
test_object_create_bad_contentlength_negative
test_object_create_bad_contentlength_none
test_object_create_bad_contentlength_unreadable
test_object_create_bad_contentlength_mismatch_above
test_object_create_bad_contenttype_invalid
test_object_create_bad_contenttype_empty
test_object_create_bad_contenttype_none
test_object_create_bad_authorization_empty
test_object_create_date_and_amz_date
test_object_create_amz_date_and_no_date
test_object_create_bad_authorization_none
test_bucket_create_contentlength_none
test_object_acl_create_contentlength_none
test_bucket_put_bad_canned_acl

but in jewel it is ok

static int get_request_len(const char *buf, int buflen)

Signed-off-by: yuliyang yuliyang@cmss.chinamobile.com

@joke-lee
Copy link
Author

fix: https://tracker.ceph.com/issues/25135

Signed-off-by: yuliyang <yuliyang@cmss.chinamobile.com>
@joke-lee
Copy link
Author

hi, @cbodley do you mind to help review this ? thanks

@joke-lee
Copy link
Author

joke-lee commented Aug 5, 2018

hi @theanalyst , do you mind to help review this ? thanks

@theanalyst
Copy link
Member

Thanks @joke-lee for identifying the issue
I'm not sure this pr fixes all these test cases which you posted. Wrt to the tests posted, this tests only seems to make the bad expect unreadable (and similar tests) where a binary string like \x07 is passed in the header value which violates rfc2616. Are there any particular use cases where we expect binary strings to be passed on as a header value and we want to ignore that? Or is there any other cases of headers we're trying to fix with this?

I also tried the current ceph master against both beast and civetweb frontends and the only test case that seems to fail is the content-length negative case where a 400 is returned though we don't return the correct reason.

@joke-lee
Copy link
Author

hi @theanalyst , we run in v4 signature

S3_USE_SIGV4=1  S3TEST_CONF=test.conf  ./virtualenv/bin/nosetests 

@joke-lee joke-lee closed this Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants