From 7a9e3a7dc53a20971dbd653f2061337efec136a2 Mon Sep 17 00:00:00 2001 From: Mike Lundy Date: Fri, 13 Apr 2012 19:53:16 -0700 Subject: [PATCH] Omit Content-Length on chunked transfer Content-Length and Transfer-Encoding conflict according to the HTTP spec. This fixes bug 981332. This also adds the ability to test both the sendfile-present and sendfile-absent codepaths; the sendfile-present test will be skipped on sendfile-absent platforms. [ This is backported from 223fbee49a55691504623fa691bbb3e48048d5f3 ] Change-Id: I20856eb51ff66fe4b7145f796a540a832c3aa4d8 --- glance/common/client.py | 9 +++++++-- glance/tests/stubs.py | 29 +++++++++++++++++++++++------ glance/tests/unit/test_clients.py | 15 +++++++++++++-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/glance/common/client.py b/glance/common/client.py index 7c6ae55..2e1f35c 100644 --- a/glance/common/client.py +++ b/glance/common/client.py @@ -506,12 +506,17 @@ class BaseClient(object): elif _filelike(body) or self._iterable(body): c.putrequest(method, path) + use_sendfile = self._sendable(body) + + # According to HTTP/1.1, Content-Length and Transfer-Encoding + # conflict. for header, value in headers.items(): - c.putheader(header, value) + if use_sendfile or header.lower() != 'content-length': + c.putheader(header, value) iter = self.image_iterator(c, headers, body) - if self._sendable(body): + if use_sendfile: # send actual file without copying into userspace _sendbody(c, iter) else: diff --git a/glance/tests/stubs.py b/glance/tests/stubs.py index dba4d7f..776da40 100644 --- a/glance/tests/stubs.py +++ b/glance/tests/stubs.py @@ -59,7 +59,7 @@ def stub_out_registry_and_store_server(stubs, base_dir): def close(self): return True - def request(self, method, url, body=None, headers={}): + def request(self, method, url, body=None, headers=None): self.req = webob.Request.blank("/" + url.lstrip("/")) self.req.method = method if headers: @@ -110,7 +110,8 @@ def stub_out_registry_and_store_server(stubs, base_dir): def __init__(self, *args, **kwargs): self.sock = FakeSocket() - pass + self.stub_force_sendfile = kwargs.get('stub_force_sendfile', + SENDFILE_SUPPORTED) def connect(self): return True @@ -120,7 +121,7 @@ def stub_out_registry_and_store_server(stubs, base_dir): def putrequest(self, method, url): self.req = webob.Request.blank("/" + url.lstrip("/")) - if SENDFILE_SUPPORTED: + if self.stub_force_sendfile: fake_sendfile = FakeSendFile(self.req) stubs.Set(sendfile, 'sendfile', fake_sendfile.sendfile) self.req.method = method @@ -129,7 +130,10 @@ def stub_out_registry_and_store_server(stubs, base_dir): self.req.headers[key] = value def endheaders(self): - pass + hl = [i.lower() for i in self.req.headers.keys()] + assert not ('content-length' in hl and + 'transfer-encoding' in hl), \ + 'Content-Length and Transfer-Encoding are mutually exclusive' def send(self, data): # send() is called during chunked-transfer encoding, and @@ -137,7 +141,7 @@ def stub_out_registry_and_store_server(stubs, base_dir): # only write the actual data in tests. self.req.body += data.split("\r\n")[1] - def request(self, method, url, body=None, headers={}): + def request(self, method, url, body=None, headers=None): self.req = webob.Request.blank("/" + url.lstrip("/")) self.req.method = method if headers: @@ -187,8 +191,21 @@ def stub_out_registry_and_store_server(stubs, base_dir): for i in self.source.app_iter: yield i + def fake_sendable(self, body): + force = getattr(self, 'stub_force_sendfile', None) + if force is None: + return self._stub_orig_sendable(body) + else: + if force: + assert glance.common.client.SENDFILE_SUPPORTED + return force + stubs.Set(glance.common.client.BaseClient, 'get_connection_type', fake_get_connection_type) + setattr(glance.common.client.BaseClient, '_stub_orig_sendable', + glance.common.client.BaseClient._sendable) + stubs.Set(glance.common.client.BaseClient, '_sendable', + fake_sendable) stubs.Set(glance.common.client.ImageBodyIterator, '__iter__', fake_image_iter) @@ -211,7 +228,7 @@ def stub_out_registry_server(stubs, **kwargs): def close(self): return True - def request(self, method, url, body=None, headers={}): + def request(self, method, url, body=None, headers=None): self.req = webob.Request.blank("/" + url.lstrip("/")) self.req.method = method if headers: diff --git a/glance/tests/unit/test_clients.py b/glance/tests/unit/test_clients.py index e8e71de..1548f48 100644 --- a/glance/tests/unit/test_clients.py +++ b/glance/tests/unit/test_clients.py @@ -26,7 +26,7 @@ import stubout import webob from glance import client -from glance.common import context +from glance.common import client as base_client from glance.common import exception from glance.common import utils from glance.registry.db import api as db_api @@ -36,6 +36,7 @@ from glance.registry import context as rcontext from glance.tests import stubs from glance.tests import utils as test_utils from glance.tests.unit import base +from glance.tests import utils as test_utils CONF = {'sql_connection': 'sqlite://'} @@ -1842,7 +1843,7 @@ class TestClient(base.IsolatedUnitTest): for k, v in fixture.items(): self.assertEquals(v, new_meta[k]) - def test_add_image_with_image_data_as_file(self): + def add_image_with_image_data_as_file(self, sendfile): """Tests can add image by passing image data as file""" fixture = {'name': 'fake public image', 'is_public': True, @@ -1852,6 +1853,8 @@ class TestClient(base.IsolatedUnitTest): 'properties': {'distro': 'Ubuntu 10.04 LTS'}, } + self.client.stub_force_sendfile = sendfile + image_data_fixture = r"chunk00000remainder" tmp_image_filepath = '/tmp/rubbish-image' @@ -1879,6 +1882,14 @@ class TestClient(base.IsolatedUnitTest): for k, v in fixture.items(): self.assertEquals(v, new_meta[k]) + @test_utils.skip_if(not base_client.SENDFILE_SUPPORTED, + 'sendfile not supported') + def test_add_image_with_image_data_as_file_with_sendfile(self): + self.add_image_with_image_data_as_file(sendfile=True) + + def test_add_image_with_image_data_as_file_without_sendfile(self): + self.add_image_with_image_data_as_file(sendfile=False) + def _add_image_as_iterable(self): fixture = {'name': 'fake public image', 'is_public': True,