rgw: set Access-Control-Allow-Origin to an asterisk if allowed in a rule #8528
Conversation
wido
force-pushed
the
hammer-issue-15348
branch
from
ce4f254
to
b41a170
Compare
|
There were some failures on #8441 with the tests. Fixed the same things in this PR. |
|
@wido: Please describe the conflicts in the commit message. |
|
@wido There are some useful pointers on describing conflicts in http://tracker.ceph.com/projects/ceph-releases/wiki/HOWTO_backport_commits |
wido
force-pushed
the
hammer-issue-15348
branch
2 times, most recently
from
7f39d70
to
b474ab3
Compare
|
@smithfarm Done! I updated the commit and force pushed |
|
Changelog:
|
|
@wido Would you care to stage the jewel backport as well? The tracker URL is http://tracker.ceph.com/issues/16112 |
…sterisk if allowed in a rule Reviewed-by: Nathan Cutler <ncutler@suse.com>
|
@smithfarm Done, please see #9453 It merged cleanly into the Jewel branch on my local testing |
|
@wido Thanks, http://tracker.ceph.com/issues/16112 assigned to you ;-) |
|
@wido, can you add the cherry-picked commit to the commit message? |
|
@oritwas Yes! Just did |
|
@wido , I noticed this is very different from the upstream pr #8441 which may cause crashes http://tracker.ceph.com/issues/16323. |
oritwas
changed the title
|
@oritwas Yes, I must have pushed a old branch. I just did a force push for Hammer. Could you review it again? |
|
@wido Sorry if this is tiresome, but could you re-do the patch using "git cherry-pick -x" ? |
|
Just did @smithfarm :) |
|
@wido Which version of git are you using? When I do Whatever you add to the original commit message should come after that line and should be limited to a description of the conflicts. |
|
@smithfarm I have git 1.9. I think this is because I manually edited the commit message with a copy-paste of the message I already had. So you want to conflicts first? I thought this message was OK. |
Before this patch the RGW would respond with the Origin send by the client in the request if a wildcard/asterisk was specified as a valid Origin. This patch makes sure we respond with a header like this: Access-Control-Allow-Origin: * This way a resource can be used on different Origins by the same browser and that browser will use the content as the asterisk. We also keep in mind that when Authorization is send by the client different rules apply. In the case of Authorization we may not respond with an Asterisk, but we do have to add the Vary header with 'Origin' as a value to let the browser know that for different Origins it has to perform a new request. More information: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS Fixes: ceph#15348 Signed-off-by: Wido den Hollander <wido@42on.com> (cherry picked from commit 0021e22) Conflicts: src/rgw/rgw_rest.cc hammer still uses s->cio->print() where master uses STREAM_IO(s)->print()
|
Ah, yes! My bad @smithfarm , thanks for the pointer. I was somehow cherry-picking the wrong commit. |
|
@wido Thanks, looks good now. |
wido
changed the title
|
@smithfarm How far are we with this one? I would like to see it in 0.94.8 :) |
…sterisk if allowed in a rule Reviewed-by: Nathan Cutler <ncutler@suse.com>
…sterisk if allowed in a rule Reviewed-by: Nathan Cutler <ncutler@suse.com>
|
@oritwas This PR passed an rgw suite with some valgrind-related failures that appear harmless - see http://tracker.ceph.com/issues/15895#note-19 Do you think this PR is ready to merge? |
|
lgtm |
smithfarm
changed the title
http://tracker.ceph.com/issues/15839
Before this patch the RGW would respond with the Origin send by the client in the request
if a wildcard/asterisk was specified as a valid Origin.
This patch makes sure we respond with a header like this:
Access-Control-Allow-Origin: *
This way a resource can be used on different Origins by the same browser and that browser
will use the content as the asterisk.
We also keep in mind that when Authorization is send by the client different rules apply.
In the case of Authorization we may not respond with an Asterisk, but we do have to
add the Vary header with 'Origin' as a value to let the browser know that for different
Origins it has to perform a new request.
More information: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS
Fixes: #15348
Signed-off-by: Wido den Hollander wido@42on.com
Conflicts:
src/rgw/rgw_rest.cc
This is the backport fix for #8441