Add Vary: headers for CORS responses
From the (non-normative) Implementation Considerations section of https://www.w3.org/TR/cors/#resource-implementation : > Resources that wish to enable themselves to be shared with multiple > Origins but do not respond uniformly with "*" must in practice > generate the Access-Control-Allow-Origin header dynamically in > response to every request they wish to allow. As a consequence, > authors of such resources should send a Vary: Origin HTTP header or > provide other appropriate control directives to prevent caching of > such responses, which may be inaccurate if re-used across-origins. We do the first part (dynamic Access-Control-Allow-Origin: generation based on the incoming Origin: header), but not the second (send a Vary: Origin header). Consider this scenario: 1. Swift user Alice has some static content that should be available from some (but not all) other domains. She creates a new container with an appropriate X-Container-Meta-Access-Control-Allow-Origin like "http://foo.example.com http://bar.example.com". 2. End user Bob pulls up a browser and visits http://foo.example.com, which references a cross-origin resource. Seeing this, the browser issues a preflight request and gets back a response that includes headers like: Access-Control-Allow-Origin: http://foo.example.com Access-Control-Allow-Methods: HEAD, GET, PUT, POST, COPY, OPTIONS, DELETE Since the preflight succeeded, the browser follows through on the cross-origin request and everything loads properly. 3. Now Bob visits http://bar.example.com, which references the same resource. Ordinarily, the exact same thing would happen, but with http://bar.example.com in the headers. However, if the browser cached the preflight response (because it didn't want to make two requests everytime it needed a resource), it would assume the server would only allow resource-sharing with http://foo.example.com and not load the resource. Similar issues arise from the dynamically-generated Access-Control-Allow-Headers header. For more information on the Vary: header, see http://tools.ietf.org/html/rfc7231#section-7.1.4 Change-Id: I9950e593312f654ee596b7f43f7ab9e5b684d8e5
This commit is contained in:
Tim Burke
parent
29d13b7161
commit
3e46079546
@ -1853,28 +1853,37 @@ class Controller(object):
|
||||
resp.status = HTTP_UNAUTHORIZED
|
||||
return resp
|
||||
|
||||
# Allow all headers requested in the request. The CORS
|
||||
# specification does leave the door open for this, as mentioned in
|
||||
# http://www.w3.org/TR/cors/#resource-preflight-requests
|
||||
# Note: Since the list of headers can be unbounded
|
||||
# simply returning headers can be enough.
|
||||
allow_headers = set()
|
||||
if req.headers.get('Access-Control-Request-Headers'):
|
||||
allow_headers.update(
|
||||
list_from_csv(req.headers['Access-Control-Request-Headers']))
|
||||
|
||||
# Populate the response with the CORS preflight headers
|
||||
if cors.get('allow_origin') and \
|
||||
cors.get('allow_origin').strip() == '*':
|
||||
headers['access-control-allow-origin'] = '*'
|
||||
else:
|
||||
headers['access-control-allow-origin'] = req_origin_value
|
||||
if 'vary' in headers:
|
||||
headers['vary'] += ', Origin'
|
||||
else:
|
||||
headers['vary'] = 'Origin'
|
||||
|
||||
if cors.get('max_age') is not None:
|
||||
headers['access-control-max-age'] = cors.get('max_age')
|
||||
|
||||
headers['access-control-allow-methods'] = \
|
||||
', '.join(self.allowed_methods)
|
||||
|
||||
# Allow all headers requested in the request. The CORS
|
||||
# specification does leave the door open for this, as mentioned in
|
||||
# http://www.w3.org/TR/cors/#resource-preflight-requests
|
||||
# Note: Since the list of headers can be unbounded
|
||||
# simply returning headers can be enough.
|
||||
allow_headers = set(
|
||||
list_from_csv(req.headers.get('Access-Control-Request-Headers')))
|
||||
if allow_headers:
|
||||
headers['access-control-allow-headers'] = ', '.join(allow_headers)
|
||||
if 'vary' in headers:
|
||||
headers['vary'] += ', Access-Control-Request-Headers'
|
||||
else:
|
||||
headers['vary'] = 'Access-Control-Request-Headers'
|
||||
|
||||
resp.headers = headers
|
||||
|
||||
return resp
|
||||
|
||||
@ -4971,6 +4971,8 @@ class TestObjectController(unittest.TestCase):
|
||||
'Access-Control-Request-Method': 'GET'})
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(401, resp.status_int)
|
||||
self.assertNotIn('Access-Control-Allow-Origin', resp.headers)
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
|
||||
def my_empty_origin_container_info(*args):
|
||||
return {'cors': {'allow_origin': None}}
|
||||
@ -4982,6 +4984,8 @@ class TestObjectController(unittest.TestCase):
|
||||
'Access-Control-Request-Method': 'GET'})
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(401, resp.status_int)
|
||||
self.assertNotIn('Access-Control-Allow-Origin', resp.headers)
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
|
||||
def my_container_info(*args):
|
||||
return {
|
||||
@ -5002,13 +5006,13 @@ class TestObjectController(unittest.TestCase):
|
||||
self.assertEqual(
|
||||
'https://foo.bar',
|
||||
resp.headers['access-control-allow-origin'])
|
||||
for verb in 'OPTIONS GET POST PUT DELETE HEAD'.split():
|
||||
self.assertIn(verb,
|
||||
resp.headers['access-control-allow-methods'])
|
||||
self.assertEqual('Origin', resp.headers.get('vary'))
|
||||
self.assertEqual(
|
||||
len(resp.headers['access-control-allow-methods'].split(', ')),
|
||||
6)
|
||||
sorted(resp.headers['access-control-allow-methods']
|
||||
.split(', ')),
|
||||
sorted('OPTIONS GET POST PUT DELETE HEAD'.split()))
|
||||
self.assertEqual('999', resp.headers['access-control-max-age'])
|
||||
|
||||
req = Request.blank(
|
||||
'/v1/a/c/o.jpg',
|
||||
{'REQUEST_METHOD': 'OPTIONS'},
|
||||
@ -5016,19 +5020,28 @@ class TestObjectController(unittest.TestCase):
|
||||
req.content_length = 0
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(401, resp.status_int)
|
||||
self.assertNotIn('Access-Control-Allow-Origin', resp.headers)
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
|
||||
req = Request.blank('/v1/a/c/o.jpg', {'REQUEST_METHOD': 'OPTIONS'})
|
||||
req.content_length = 0
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(200, resp.status_int)
|
||||
for verb in 'OPTIONS GET POST PUT DELETE HEAD'.split():
|
||||
self.assertIn(verb, resp.headers['Allow'])
|
||||
self.assertEqual(len(resp.headers['Allow'].split(', ')), 6)
|
||||
self.assertEqual(
|
||||
sorted(resp.headers['Allow'].split(', ')),
|
||||
sorted('OPTIONS GET POST PUT DELETE HEAD'.split()))
|
||||
self.assertNotIn('Access-Control-Allow-Origin', resp.headers)
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
|
||||
req = Request.blank(
|
||||
'/v1/a/c/o.jpg',
|
||||
{'REQUEST_METHOD': 'OPTIONS'},
|
||||
headers={'Origin': 'http://foo.com'})
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(401, resp.status_int)
|
||||
self.assertNotIn('Access-Control-Allow-Origin', resp.headers)
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
|
||||
req = Request.blank(
|
||||
'/v1/a/c/o.jpg',
|
||||
{'REQUEST_METHOD': 'OPTIONS'},
|
||||
@ -5037,6 +5050,7 @@ class TestObjectController(unittest.TestCase):
|
||||
controller.app.cors_allow_origin = ['http://foo.bar', ]
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(200, resp.status_int)
|
||||
self.assertEqual('Origin', resp.headers.get('vary'))
|
||||
|
||||
def my_container_info_wildcard(*args):
|
||||
return {
|
||||
@ -5055,12 +5069,11 @@ class TestObjectController(unittest.TestCase):
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(200, resp.status_int)
|
||||
self.assertEqual('*', resp.headers['access-control-allow-origin'])
|
||||
for verb in 'OPTIONS GET POST PUT DELETE HEAD'.split():
|
||||
self.assertIn(verb,
|
||||
resp.headers['access-control-allow-methods'])
|
||||
self.assertNotIn('Vary', resp.headers)
|
||||
self.assertEqual(
|
||||
len(resp.headers['access-control-allow-methods'].split(', ')),
|
||||
6)
|
||||
sorted(resp.headers['access-control-allow-methods']
|
||||
.split(', ')),
|
||||
sorted('OPTIONS GET POST PUT DELETE HEAD'.split()))
|
||||
self.assertEqual('999', resp.headers['access-control-max-age'])
|
||||
|
||||
def _get_CORS_response(self, container_cors, strict_mode, object_get=None):
|
||||
@ -7114,11 +7127,13 @@ class TestContainerController(unittest.TestCase):
|
||||
'/v1/a/c/o.jpg',
|
||||
{'REQUEST_METHOD': 'OPTIONS'},
|
||||
headers={'Origin': 'https://bar.baz',
|
||||
'Access-Control-Request-Headers': ' , ,,',
|
||||
'Access-Control-Request-Method': 'GET'})
|
||||
req.content_length = 0
|
||||
resp = controller.OPTIONS(req)
|
||||
self.assertEqual(200, resp.status_int)
|
||||
self.assertEqual('*', resp.headers['access-control-allow-origin'])
|
||||
self.assertNotIn('access-control-allow-headers', resp.headers)
|
||||
for verb in 'OPTIONS GET POST PUT DELETE HEAD'.split():
|
||||
self.assertIn(verb,
|
||||
resp.headers['access-control-allow-methods'])
|
||||
@ -7132,7 +7147,7 @@ class TestContainerController(unittest.TestCase):
|
||||
{'REQUEST_METHOD': 'OPTIONS'},
|
||||
headers={'Origin': 'https://bar.baz',
|
||||
'Access-Control-Request-Headers':
|
||||
'x-foo, x-bar, x-auth-token',
|
||||
'x-foo, x-bar, , x-auth-token',
|
||||
'Access-Control-Request-Method': 'GET'}
|
||||
)
|
||||
req.content_length = 0
|
||||
@ -7141,6 +7156,8 @@ class TestContainerController(unittest.TestCase):
|
||||
self.assertEqual(
|
||||
sortHeaderNames('x-foo, x-bar, x-auth-token'),
|
||||
sortHeaderNames(resp.headers['access-control-allow-headers']))
|
||||
self.assertEqual('Access-Control-Request-Headers',
|
||||
resp.headers.get('vary'))
|
||||
|
||||
def test_CORS_valid(self):
|
||||
with save_globals():
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user