From 337570a03a57b2bceb615c4fe99ccaa18e0220c9 Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Fri, 29 Jan 2016 16:50:15 -0800
Subject: [PATCH] Don't trust X-Object-Meta-Mtime

Still use it if we can, but stop throwing ValueErrors if it's garbage.

Change-Id: I2cf25b535ad62cfacb7561954a92a4a73d91000a
---
 swiftclient/service.py     | 12 +++--
 tests/unit/test_service.py | 91 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/swiftclient/service.py b/swiftclient/service.py
index 492055dd..20c023b4 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -1162,11 +1162,15 @@ class SwiftService(object):
                 if fp is not None:
                     fp.close()
                     if 'x-object-meta-mtime' in headers and not no_file:
-                        mtime = float(headers['x-object-meta-mtime'])
-                        if options['out_file']:
-                            utime(options['out_file'], (mtime, mtime))
+                        try:
+                            mtime = float(headers['x-object-meta-mtime'])
+                        except ValueError:
+                            pass  # no real harm; couldn't trust it anyway
                         else:
-                            utime(path, (mtime, mtime))
+                            if options['out_file']:
+                                utime(options['out_file'], (mtime, mtime))
+                            else:
+                                utime(path, (mtime, mtime))
 
             res = {
                 'action': 'download_object',
diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py
index 7522e657..51baa1f2 100644
--- a/tests/unit/test_service.py
+++ b/tests/unit/test_service.py
@@ -1535,6 +1535,97 @@ class TestServiceDownload(_TestServiceBase):
         )
         self._assertDictEqual(expected_r, actual_r)
 
+    def test_download_object_job_with_mtime(self):
+        mock_conn = self._get_mock_connection()
+        objcontent = six.BytesIO(b'objcontent')
+        mock_conn.get_object.side_effect = [
+            ({'content-type': 'text/plain',
+              'etag': '2cbbfe139a744d6abbe695e17f3c1991',
+              'x-object-meta-mtime': '1454113727.682512'},
+             objcontent)
+        ]
+        expected_r = self._get_expected({
+            'success': True,
+            'start_time': 1,
+            'finish_time': 2,
+            'headers_receipt': 3,
+            'auth_end_time': 4,
+            'read_length': len(b'objcontent'),
+        })
+
+        with mock.patch.object(builtins, 'open') as mock_open, \
+                mock.patch('swiftclient.service.utime') as mock_utime:
+            written_content = Mock()
+            mock_open.return_value = written_content
+            s = SwiftService()
+            _opts = self.opts.copy()
+            _opts['no_download'] = False
+            actual_r = s._download_object_job(
+                mock_conn, 'test_c', 'test_o', _opts)
+            actual_r = dict(  # Need to override the times we got from the call
+                actual_r,
+                **{
+                    'start_time': 1,
+                    'finish_time': 2,
+                    'headers_receipt': 3
+                }
+            )
+            mock_open.assert_called_once_with('test_o', 'wb')
+            mock_utime.assert_called_once_with(
+                'test_o', (1454113727.682512, 1454113727.682512))
+            written_content.write.assert_called_once_with(b'objcontent')
+
+        mock_conn.get_object.assert_called_once_with(
+            'test_c', 'test_o', resp_chunk_size=65536, headers={},
+            response_dict={}
+        )
+        self._assertDictEqual(expected_r, actual_r)
+
+    def test_download_object_job_bad_mtime(self):
+        mock_conn = self._get_mock_connection()
+        objcontent = six.BytesIO(b'objcontent')
+        mock_conn.get_object.side_effect = [
+            ({'content-type': 'text/plain',
+              'etag': '2cbbfe139a744d6abbe695e17f3c1991',
+              'x-object-meta-mtime': 'foo'},
+             objcontent)
+        ]
+        expected_r = self._get_expected({
+            'success': True,
+            'start_time': 1,
+            'finish_time': 2,
+            'headers_receipt': 3,
+            'auth_end_time': 4,
+            'read_length': len(b'objcontent'),
+        })
+
+        with mock.patch.object(builtins, 'open') as mock_open, \
+                mock.patch('swiftclient.service.utime') as mock_utime:
+            written_content = Mock()
+            mock_open.return_value = written_content
+            s = SwiftService()
+            _opts = self.opts.copy()
+            _opts['no_download'] = False
+            actual_r = s._download_object_job(
+                mock_conn, 'test_c', 'test_o', _opts)
+            actual_r = dict(  # Need to override the times we got from the call
+                actual_r,
+                **{
+                    'start_time': 1,
+                    'finish_time': 2,
+                    'headers_receipt': 3
+                }
+            )
+            mock_open.assert_called_once_with('test_o', 'wb')
+            self.assertEqual(0, len(mock_utime.mock_calls))
+            written_content.write.assert_called_once_with(b'objcontent')
+
+        mock_conn.get_object.assert_called_once_with(
+            'test_c', 'test_o', resp_chunk_size=65536, headers={},
+            response_dict={}
+        )
+        self._assertDictEqual(expected_r, actual_r)
+
     def test_download_object_job_exception(self):
         mock_conn = self._get_mock_connection()
         mock_conn.get_object = Mock(side_effect=self.exc)