Fix 499 client disconnected on COPY EC object

Currently, a COPY request for an EC object might go to fail as 499 Client
disconnected because of the difference between destination request content
length and actual transferred bytes.

That is because the conditional response status and content length for
an EC object range GET is handled at calling the response instance on
proxy server. Therefore the calling response instance (resp()) will change
the conditional status from 200 (HTTP_OK) to 206 (PartialContent) and will
change the content length for the range GET.

In EC case, sometimes Swift needs whole stored contents to decode a segment.
It will make 200 HTTP OK response from object-server and proxy-server
will unfortunately set whole content length to the destination content
length and it makes the bug 1467677.

This patch introduces a new method "fix_conditional_response" for
swift.common.swob.Response that calling _response_iter() and cached the
iter in the Response instance. By calling it, Swift can set correct condtional
response any time after setting whole content_length to the response
instance like EC case.

Change-Id: If85826243f955d2f03c6ad395215c73daab509b1
Closes-Bug: #1467677

swift/common/swob.py

@@ -1127,6 +1127,7 @@ class Response(object):
         self.request = request
         self.body = body
         self.app_iter = app_iter
+        self.response_iter = None
         self.status = status
         self.boundary = "%.32x" % random.randint(0, 256 ** 16)
         if request:
@@ -1322,6 +1323,17 @@ class Response(object):
                 return [body]
         return ['']
 
+    def fix_conditional_response(self):
+        """
+        You may call this once you have set the content_length to the whole
+        object length and body or app_iter to reset the content_length
+        properties on the request.
+
+        It is ok to not call this method, the conditional resposne will be
+        maintained for you when you __call__ the response.
+        """
+        self.response_iter = self._response_iter(self.app_iter, self._body)
+
     def absolute_location(self):
         """
         Attempt to construct an absolute location.
@@ -1372,12 +1384,15 @@ class Response(object):
         if not self.request:
             self.request = Request(env)
         self.environ = env
-        app_iter = self._response_iter(self.app_iter, self._body)
+
+        if not self.response_iter:
+            self.response_iter = self._response_iter(self.app_iter, self._body)
+
         if 'location' in self.headers and \
                 not env.get('swift.leave_relative_location'):
             self.location = self.absolute_location()
         start_response(self.status, self.headers.items())
-        return app_iter
+        return self.response_iter
 
 
 class HTTPException(Response, Exception):

swift/proxy/controllers/obj.py

@@ -2203,11 +2203,10 @@ class ECObjectController(BaseObjectController):
                 resp = self.best_response(
                     req, statuses, reasons, bodies, 'Object',
                     headers=headers)
-
-        self._fix_response_headers(resp)
+        self._fix_response(resp)
         return resp
 
-    def _fix_response_headers(self, resp):
+    def _fix_response(self, resp):
         # EC fragment archives each have different bytes, hence different
         # etags. However, they all have the original object's etag stored in
         # sysmeta, so we copy that here so the client gets it.
@@ -2215,6 +2214,7 @@ class ECObjectController(BaseObjectController):
             'X-Object-Sysmeta-Ec-Etag')
         resp.headers['Content-Length'] = resp.headers.get(
             'X-Object-Sysmeta-Ec-Content-Length')
+        resp.fix_conditional_response()
 
         return resp
 

test/functional/test_object.py

@@ -243,6 +243,23 @@ class TestObject(unittest.TestCase):
         self.assertEqual(resp.status, 200)
         self.assertEqual(dest_contents, source_contents)
 
+        # copy source to dest with COPY and range
+        def copy(url, token, parsed, conn):
+            conn.request('COPY', '%s/%s' % (parsed.path, source), '',
+                         {'X-Auth-Token': token,
+                          'Destination': dest,
+                          'Range': 'bytes=1-2'})
+            return check_response(conn)
+        resp = retry(copy)
+        resp.read()
+        self.assertEqual(resp.status, 201)
+
+        # contents of dest should be the same as source
+        resp = retry(get_dest)
+        dest_contents = resp.read()
+        self.assertEqual(resp.status, 200)
+        self.assertEqual(dest_contents, source_contents[1:3])
+
         # delete the copy
         resp = retry(delete)
         resp.read()

test/unit/proxy/controllers/test_obj.py

@@ -1458,6 +1458,40 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase):
         self.assertEquals(resp.status_int, 201)
         self.assertTrue(response_time < response_sleep)
 
+    def test_COPY_with_ranges(self):
+        req = swift.common.swob.Request.blank(
+            '/v1/a/c/o', method='COPY',
+            headers={'Destination': 'c1/o',
+                     'Range': 'bytes=5-10'})
+        # turn a real body into fragments
+        segment_size = self.policy.ec_segment_size
+        real_body = ('asdf' * segment_size)[:-10]
+
+        # split it up into chunks
+        chunks = [real_body[x:x + segment_size]
+                  for x in range(0, len(real_body), segment_size)]
+
+        # we need only first chunk to rebuild 5-10 range
+        fragments = self.policy.pyeclib_driver.encode(chunks[0])
+        fragment_payloads = []
+        fragment_payloads.append(fragments)
+
+        node_fragments = zip(*fragment_payloads)
+        self.assertEqual(len(node_fragments), self.replicas())  # sanity
+        headers = {'X-Object-Sysmeta-Ec-Content-Length': str(len(real_body))}
+        responses = [(200, ''.join(node_fragments[i]), headers)
+                     for i in range(POLICIES.default.ec_ndata)]
+        responses += [(201, '', {})] * self.obj_ring.replicas
+        status_codes, body_iter, headers = zip(*responses)
+        expect_headers = {
+            'X-Obj-Metadata-Footer': 'yes',
+            'X-Obj-Multiphase-Commit': 'yes'
+        }
+        with set_http_connect(*status_codes, body_iter=body_iter,
+                              headers=headers, expect_headers=expect_headers):
+            resp = req.get_response(self.app)
+        self.assertEquals(resp.status_int, 201)
+
 
 if __name__ == '__main__':
     unittest.main()