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 223fbee ]

Change-Id: I20856eb51ff66fe4b7145f796a540a832c3aa4d8

glance/common/client.py

@@ -506,12 +506,17 @@ def _chunkbody(connection, iter):
             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:

glance/tests/stubs.py

@@ -59,7 +59,7 @@ def connect(self):
         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 @@ class FakeGlanceConnection(object):
 
         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 close(self):
 
         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,15 +130,18 @@ def putheader(self, key, value):
             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
             # data is of the form %x\r\n%s\r\n. Strip off the %x and
             # 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 fake_image_iter(self):
         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 connect(self):
         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:

glance/tests/unit/test_clients.py

@@ -26,7 +26,7 @@
 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.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 @@ def test_add_image_with_image_data_as_string(self):
         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 @@ def test_add_image_with_image_data_as_file(self):
                    '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 @@ def test_add_image_with_image_data_as_file(self):
         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,