Stop including Connection header in EC GET responses

Currently, EC GET responses from proxy to clients, unlike any other
response, include a "Connection: close" header. If the client has sent
a "Connection: keep-alive" header then eventlet.wsgi appends this to
the client response, so clients can receive a response with both
headers:

Connection: close
Connection: keep-alive

This patch fixes the proxy EC GET path to remove any Connection header
from it's response, but does not change the behaviour of eventlet.wsgi
with respect to returning any client provided 'Connection: keep-alive'
header.

Change-Id: I43cd27c978edb4a1a587f031dbbee26e9acdc920
Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
Closes-Bug:  #1680731

swift/proxy/controllers/base.py

@@ -79,8 +79,9 @@ def update_headers(response, headers):
     for name, value in headers:
         if name == 'etag':
             response.headers[name] = value.replace('"', '')
-        elif name not in ('date', 'content-length', 'content-type',
+        elif name.lower() not in (
-                          'connection', 'x-put-timestamp', 'x-delete-after'):
+                'date', 'content-length', 'content-type',
+                'connection', 'x-put-timestamp', 'x-delete-after'):
             response.headers[name] = value
 
 

swift/proxy/controllers/obj.py

@@ -65,7 +65,7 @@ from swift.common.http import (
 from swift.common.storage_policy import (POLICIES, REPL_POLICY, EC_POLICY,
                                          ECDriverError, PolicyError)
 from swift.proxy.controllers.base import Controller, delay_denial, \
-    cors_validation, ResumingGetter
+    cors_validation, ResumingGetter, update_headers
 from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
     HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \
     HTTPServerError, HTTPServiceUnavailable, HTTPClientDisconnect, \
@@ -2272,9 +2272,9 @@ class ECObjectController(BaseObjectController):
                 self.app.logger)
             resp = Response(
                 request=req,
-                headers=resp_headers,
                 conditional_response=True,
                 app_iter=app_iter)
+            update_headers(resp, resp_headers)
             try:
                 app_iter.kickoff(req, resp)
             except HTTPException as err_resp:

test/functional/tests.py

@@ -30,6 +30,8 @@ from unittest2 import SkipTest
 from swift.common.http import is_success, is_client_error
 from email.utils import parsedate
 
+import mock
+
 from test.functional import normalized_urls, load_constraint, cluster_info
 from test.functional import check_response, retry
 import test.functional as tf
@@ -1230,6 +1232,60 @@ class TestFileDevUTF8(Base2, TestFileDev):
 class TestFile(Base):
     env = TestFileEnv
 
+    def testGetResponseHeaders(self):
+        obj_data = 'test_body'
+
+        def do_test(put_hdrs, get_hdrs, expected_hdrs, unexpected_hdrs):
+            filename = Utils.create_name()
+            file_item = self.env.container.file(filename)
+            resp = file_item.write(
+                data=obj_data, hdrs=put_hdrs, return_resp=True)
+
+            # put then get an object
+            resp.read()
+            read_data = file_item.read(hdrs=get_hdrs)
+            self.assertEqual(obj_data, read_data)  # sanity check
+            resp_headers = file_item.conn.response.getheaders()
+
+            # check the *list* of all header (name, value) pairs rather than
+            # constructing a dict in case of repeated names in the list
+            errors = []
+            for k, v in resp_headers:
+                if k.lower() in unexpected_hdrs:
+                    errors.append('Found unexpected header %s: %s' % (k, v))
+            for k, v in expected_hdrs.items():
+                matches = [hdr for hdr in resp_headers if hdr[0] == k]
+                if not matches:
+                    errors.append('Missing expected header %s' % k)
+                for (got_k, got_v) in matches:
+                    if got_v != v:
+                        errors.append('Expected %s but got %s for %s' %
+                                      (v, got_v, k))
+            if errors:
+                self.fail(
+                    'Errors in response headers:\n  %s' % '\n  '.join(errors))
+
+        put_headers = {'X-Object-Meta-Fruit': 'Banana',
+                       'X-Delete-After': '10000',
+                       'Content-Type': 'application/test'}
+        expected_headers = {'content-length': str(len(obj_data)),
+                            'x-object-meta-fruit': 'Banana',
+                            'accept-ranges': 'bytes',
+                            'content-type': 'application/test',
+                            'etag': hashlib.md5(obj_data).hexdigest(),
+                            'last-modified': mock.ANY,
+                            'date': mock.ANY,
+                            'x-delete-at': mock.ANY,
+                            'x-trans-id': mock.ANY,
+                            'x-openstack-request-id': mock.ANY}
+        unexpected_headers = ['connection', 'x-delete-after']
+        do_test(put_headers, {}, expected_headers, unexpected_headers)
+
+        get_headers = {'Connection': 'keep-alive'}
+        expected_headers['connection'] = 'keep-alive'
+        unexpected_headers = ['x-delete-after']
+        do_test(put_headers, get_headers, expected_headers, unexpected_headers)
+
     def testCopy(self):
         # makes sure to test encoded characters
         source_filename = 'dealde%2Fl04 011e%204c8df/flash.png'

test/unit/proxy/controllers/test_obj.py

@@ -1110,10 +1110,11 @@ class TestReplicatedObjController(BaseObjectControllerMixin,
 
     def test_GET_simple(self):
         req = swift.common.swob.Request.blank('/v1/a/c/o')
-        with set_http_connect(200):
+        with set_http_connect(200, headers={'Connection': 'close'}):
             resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 200)
         self.assertIn('Accept-Ranges', resp.headers)
+        self.assertNotIn('Connection', resp.headers)
 
     def test_GET_transfer_encoding_chunked(self):
         req = swift.common.swob.Request.blank('/v1/a/c/o')
@@ -1484,11 +1485,13 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase):
 
     def test_GET_simple(self):
         req = swift.common.swob.Request.blank('/v1/a/c/o')
-        get_resp = [200] * self.policy.ec_ndata
+        get_statuses = [200] * self.policy.ec_ndata
-        with set_http_connect(*get_resp):
+        get_hdrs = [{'Connection': 'close'}] * self.policy.ec_ndata
+        with set_http_connect(*get_statuses, headers=get_hdrs):
             resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 200)
         self.assertIn('Accept-Ranges', resp.headers)
+        self.assertNotIn('Connection', resp.headers)
 
     def _test_if_match(self, method):
         num_responses = self.policy.ec_ndata if method == 'GET' else 1