From fd5579a154694418c66075d97f43e5d9d741058b Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Mon, 18 Jan 2016 11:38:44 -0800
Subject: Check responses when retrying bodies

Previously, if a Range request came back 200 OK (rather than 206 Partial
Content), we would mangle the response body. This could happen if there
was a middleware that would silently drop Range headers, for example.

Now, if the response does not include a Content-Range header, we will
log a warning and seek to our previous position in the stream. If the
Content-Range header has an unexpected value, we will raise an exception.

Change-Id: I94d4536cc1489968d45a2b6ba7edd70c85800275
---
 swiftclient/client.py          | 17 ++++++++++
 tests/unit/test_swiftclient.py | 70 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/swiftclient/client.py b/swiftclient/client.py
index a5c4dc3..6990ea3 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -249,6 +249,23 @@ class _RetryBody(_ObjectBody):
                                           response_dict=self.response_dict,
                                           headers=self.headers,
                                           attempts=self.conn.attempts)
+            expected_range = 'bytes %d-%d/%d' % (
+                self.bytes_read,
+                self.expected_length - 1,
+                self.expected_length)
+            if 'content-range' not in hdrs:
+                # Server didn't respond with partial content; manually seek
+                logger.warning('Received 200 while retrying %s/%s; seeking...',
+                               self.container, self.obj)
+                to_read = self.bytes_read
+                while to_read > 0:
+                    buf = body.resp.read(min(to_read, self.chunk_size))
+                    to_read -= len(buf)
+            elif hdrs['content-range'] != expected_range:
+                msg = ('Expected range "%s" while retrying %s/%s '
+                       'but got "%s"' % (expected_range, self.container,
+                                         self.obj, hdrs['content-range']))
+                raise ClientException(msg)
             self.resp = body.resp
             buf = self.read(length)
         return buf
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index b2453b0..0969f41 100644
--- a/tests/unit/test_swiftclient.py
+++ b/tests/unit/test_swiftclient.py
@@ -855,9 +855,11 @@ class TestGetObject(MockHttpTest):
                 StubResponse(200, 'abcdef', {'etag': 'some etag',
                                              'content-length': '6'}),
                 StubResponse(206, 'cdef', {'etag': 'some etag',
-                                           'content-length': '4'}),
+                                           'content-length': '4',
+                                           'content-range': 'bytes 2-5/6'}),
                 StubResponse(206, 'ef', {'etag': 'some etag',
-                                         'content-length': '2'}),
+                                         'content-length': '2',
+                                         'content-range': 'bytes 4-5/6'}),
             )
             __, resp = conn.get_object('asdf', 'asdf', resp_chunk_size=2)
             self.assertEqual(next(resp), 'ab')
@@ -887,6 +889,70 @@ class TestGetObject(MockHttpTest):
             }),
         ])
 
+    def test_chunk_size_iter_retry_no_range_support(self):
+        conn = c.Connection('http://auth.url/', 'some_user', 'some_key')
+        with mock.patch('swiftclient.client.get_auth_1_0') as mock_get_auth:
+            mock_get_auth.return_value = ('http://auth.url', 'tToken')
+            c.http_connection = self.fake_http_connection(*[
+                StubResponse(200, 'abcdef', {'etag': 'some etag',
+                                             'content-length': '6'})
+            ] * 3)
+            __, resp = conn.get_object('asdf', 'asdf', resp_chunk_size=2)
+            self.assertEqual(next(resp), 'ab')
+            self.assertEqual(1, conn.attempts)
+            # simulate a dropped connection
+            resp.resp.read()
+            self.assertEqual(next(resp), 'cd')
+            self.assertEqual(2, conn.attempts)
+            # simulate a dropped connection
+            resp.resp.read()
+            self.assertEqual(next(resp), 'ef')
+            self.assertEqual(3, conn.attempts)
+            self.assertRaises(StopIteration, next, resp)
+        self.assertRequests([
+            ('GET', '/asdf/asdf', '', {
+                'x-auth-token': 'tToken',
+            }),
+            ('GET', '/asdf/asdf', '', {
+                'range': 'bytes=2-',
+                'if-match': 'some etag',
+                'x-auth-token': 'tToken',
+            }),
+            ('GET', '/asdf/asdf', '', {
+                'range': 'bytes=4-',
+                'if-match': 'some etag',
+                'x-auth-token': 'tToken',
+            }),
+        ])
+
+    def test_chunk_size_iter_retry_bad_range_response(self):
+        conn = c.Connection('http://auth.url/', 'some_user', 'some_key')
+        with mock.patch('swiftclient.client.get_auth_1_0') as mock_get_auth:
+            mock_get_auth.return_value = ('http://auth.url', 'tToken')
+            c.http_connection = self.fake_http_connection(
+                StubResponse(200, 'abcdef', {'etag': 'some etag',
+                                             'content-length': '6'}),
+                StubResponse(206, 'abcdef', {'etag': 'some etag',
+                                             'content-length': '6',
+                                             'content-range': 'chunk 1-2/3'})
+            )
+            __, resp = conn.get_object('asdf', 'asdf', resp_chunk_size=2)
+            self.assertEqual(next(resp), 'ab')
+            self.assertEqual(1, conn.attempts)
+            # simulate a dropped connection
+            resp.resp.read()
+            self.assertRaises(c.ClientException, next, resp)
+        self.assertRequests([
+            ('GET', '/asdf/asdf', '', {
+                'x-auth-token': 'tToken',
+            }),
+            ('GET', '/asdf/asdf', '', {
+                'range': 'bytes=2-',
+                'if-match': 'some etag',
+                'x-auth-token': 'tToken',
+            }),
+        ])
+
     def test_get_object_with_resp_chunk_size_zero(self):
         def get_connection(self):
             def get_auth():
-- 
cgit v1.2.1