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: make sending Content-Length in 204 and 304 responses controllable #10156

Merged
merged 1 commit into from Feb 17, 2017

Conversation

rzarzynski
Copy link
Contributor

No description provided.

@wido
Copy link
Member

wido commented Jul 6, 2016

Always nasty to violate RFCs, but understandable. In which situation is this required?

Code looks good to me.

@rzarzynski
Copy link
Contributor Author

@wido: thanks for the review. We need the configurable to pass RefStack due to the following test case:

tempest.api.object_storage.test_object_services.ObjectTest.test_delete_object

You might also want to take a look on the PR #9188. Anyway, I'm putting the DNM tag. There is a new hope for fixing the issue in Tempest.

@rzarzynski rzarzynski changed the title rgw: make sending Content-Length in 204 and 304 responses controllable [DNM] rgw: make sending Content-Length in 204 and 304 responses controllable Jul 6, 2016
@wido
Copy link
Member

wido commented Jul 6, 2016

Ok, great! I wouldn't want to encourage people to violate RFCs, they are there for a reason.

Don't mind if this gets merged though.

@rzarzynski rzarzynski changed the title [DNM] rgw: make sending Content-Length in 204 and 304 responses controllable rgw: make sending Content-Length in 204 and 304 responses controllable Jul 19, 2016
@yehudasa
Copy link
Member

yehudasa commented Oct 4, 2016

@rzarzynski do we still need this?

@rzarzynski
Copy link
Contributor Author

@yehudasa: fortunately no :-) as our fix in Tempest has been merged. I will push 93629c0 in a separated pull request. Closing this one.

@rzarzynski rzarzynski closed this Oct 5, 2016
@mattbenjamin mattbenjamin reopened this Feb 16, 2017
This commit introduces a new configurable "rgw print prohibited
content length" to let operator decide whether RadosGW complies
to RFC 7230 (a part of the HTTP specification) or violates it
but follows the Swift's behavior.

Fixes: http://tracker.ceph.com/issues/16602
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
@rzarzynski rzarzynski changed the title rgw: make sending Content-Length in 204 and 304 responses controllable [DNM] rgw: make sending Content-Length in 204 and 304 responses controllable Feb 17, 2017
@rzarzynski
Copy link
Contributor Author

@mattbenjamin: I've just pushed a version ported to current master. DNM till testing on the morning.

@rzarzynski
Copy link
Contributor Author

@mattbenjamin:

Manual testing:

  • For rgw_print_prohibited_content_length = false (default):
$ curl -i "${publicURL}/cont/obj10" -X DELETE -H "X-Auth-Token: ${token}" 
HTTP/1.1 204 No Content
X-Trans-Id: tx00000000000000000000b-0058a6eb39-1018-default
Content-Type: text/plain; charset=utf-8
Date: Fri, 17 Feb 2017 12:23:21 GMT

  • For rgw_print_prohibited_content_length = true:
$ curl -i "${publicURL}/cont/obj10" -X DELETE -H "X-Auth-Token: ${token}" 
HTTP/1.1 204 No Content
X-Trans-Id: tx000000000000000000010-0058a6ea8c-1015-default
Content-Type: text/plain; charset=utf-8
Content-Length: 0
Date: Fri, 17 Feb 2017 12:20:28 GMT

However, Tempest has found a regression in tempest.api.object_storage.test_container_services.ContainerTest.test_list_container_contents_with_end_marker. However, most likely it is unrelated to the change. I'm working on this.

@mattbenjamin mattbenjamin self-assigned this Feb 17, 2017
@mattbenjamin mattbenjamin changed the title [DNM] rgw: make sending Content-Length in 204 and 304 responses controllable rgw: make sending Content-Length in 204 and 304 responses controllable Feb 17, 2017
@rzarzynski
Copy link
Contributor Author

rzarzynski commented Feb 17, 2017

@mattbenjamin: definitely the regression in Tempest is unrelated to the change.

@rzarzynski
Copy link
Contributor Author

Created a ticket to track the issue: http://tracker.ceph.com/issues/18977.

Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

the change appears logically and trivially correct; it's 100/200 subset rgw suite run has failures which are clearly unrelated, merging to avoid schedule impact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants