From 6c320b2908e5821e2f398e0dae8a67774e110603 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Thu, 13 Apr 2017 11:16:54 +0100 Subject: [PATCH] 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 Closes-Bug: #1680731 --- swift/proxy/controllers/base.py | 5 ++- swift/proxy/controllers/obj.py | 4 +- test/functional/tests.py | 56 +++++++++++++++++++++++++ test/unit/proxy/controllers/test_obj.py | 9 ++-- 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index feedce445f..0908c9db06 100644 --- a/swift/proxy/controllers/base.py +++ b/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', - 'connection', 'x-put-timestamp', 'x-delete-after'): + elif name.lower() not in ( + 'date', 'content-length', 'content-type', + 'connection', 'x-put-timestamp', 'x-delete-after'): response.headers[name] = value diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index 1af00490b4..7949d275b9 100644 --- a/swift/proxy/controllers/obj.py +++ b/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: diff --git a/test/functional/tests.py b/test/functional/tests.py index 15d09378d4..4af65418dd 100644 --- a/test/functional/tests.py +++ b/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' diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index 7738440980..31faa5e274 100644 --- a/test/unit/proxy/controllers/test_obj.py +++ b/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 - with set_http_connect(*get_resp): + get_statuses = [200] * self.policy.ec_ndata + 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