Merge "Add Vary: headers for CORS responses"

swift/proxy/controllers/base.py

@@ -1886,28 +1886,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

test/unit/proxy/test_server.py

@@ -5081,6 +5081,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}}
@@ -5092,6 +5094,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 {
@@ -5112,13 +5116,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.assertEqual('Origin', resp.headers.get('vary'))
-                self.assertIn(verb,
-                              resp.headers['access-control-allow-methods'])
             self.assertEqual(
-                len(resp.headers['access-control-allow-methods'].split(', ')),
+                sorted(resp.headers['access-control-allow-methods']
-                6)
+                       .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'},
@@ -5126,19 +5130,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.assertEqual(
-                self.assertIn(verb, resp.headers['Allow'])
+                sorted(resp.headers['Allow'].split(', ')),
-            self.assertEqual(len(resp.headers['Allow'].split(', ')), 6)
+                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'},
@@ -5147,6 +5160,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 {
@@ -5165,12 +5179,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.assertNotIn('Vary', resp.headers)
-                self.assertIn(verb,
-                              resp.headers['access-control-allow-methods'])
             self.assertEqual(
-                len(resp.headers['access-control-allow-methods'].split(', ')),
+                sorted(resp.headers['access-control-allow-methods']
-                6)
+                       .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):
@@ -7485,11 +7498,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'])
@@ -7503,7 +7518,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
@@ -7512,6 +7527,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():