From e7ffda5d0b75bd85c3b886c3aad0c938c7d476d6 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Fri, 5 Jan 2018 17:41:21 +0000
Subject: [PATCH] Use _update_x_timestamp method in object controller DELETE
 method

The DELETE method repeats inline the same behaviour as provided by
_update_x_timestamp, so just call the method.

Also add unit tests for the behaviour of _update_x_timestamp.

Change-Id: I8b6cfdbfb54b6d43ac507f23d84309ab543374aa
---
 swift/proxy/controllers/obj.py          | 18 +++-------
 test/unit/proxy/controllers/test_obj.py | 48 ++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py
index f5c2dfb4f8..75b3c36807 100644
--- a/swift/proxy/controllers/obj.py
+++ b/swift/proxy/controllers/obj.py
@@ -456,7 +456,9 @@ class BaseObjectController(Controller):
                 req.headers.pop('x-detect-content-type')
 
     def _update_x_timestamp(self, req):
-        # Used by container sync feature
+        # The container sync feature includes an x-timestamp header with
+        # requests. If present this is checked and preserved, otherwise a fresh
+        # timestamp is added.
         if 'x-timestamp' in req.headers:
             try:
                 req_timestamp = Timestamp(req.headers['X-Timestamp'])
@@ -769,18 +771,8 @@ class BaseObjectController(Controller):
             return HTTPNotFound(request=req)
         partition, nodes = obj_ring.get_nodes(
             self.account_name, self.container_name, self.object_name)
-        # Used by container sync feature
-        if 'x-timestamp' in req.headers:
-            try:
-                req_timestamp = Timestamp(req.headers['X-Timestamp'])
-            except ValueError:
-                return HTTPBadRequest(
-                    request=req, content_type='text/plain',
-                    body='X-Timestamp should be a UNIX timestamp float value; '
-                         'was %r' % req.headers['x-timestamp'])
-            req.headers['X-Timestamp'] = req_timestamp.internal
-        else:
-            req.headers['X-Timestamp'] = Timestamp.now().internal
+
+        self._update_x_timestamp(req)
 
         # Include local handoff nodes if write-affinity is enabled.
         node_count = len(nodes)
diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py
index ef2ec6259c..d2f7da4ee4 100644
--- a/test/unit/proxy/controllers/test_obj.py
+++ b/test/unit/proxy/controllers/test_obj.py
@@ -45,7 +45,7 @@ from swift.common.storage_policy import POLICIES, ECDriverError, \
 from test.unit import FakeRing, FakeMemcache, fake_http_connect, \
     debug_logger, patch_policies, SlowBody, FakeStatus, \
     DEFAULT_TEST_EC_TYPE, encode_frag_archive_bodies, make_ec_object_stub, \
-    fake_ec_node_response, StubResponse
+    fake_ec_node_response, StubResponse, mocked_http_conn
 from test.unit.proxy.test_server import node_error_count
 
 
@@ -1580,6 +1580,52 @@ class TestReplicatedObjController(BaseObjectControllerMixin,
                 resp = req.get_response(self.app)
             self.assertEqual(resp.status_int, 202)
 
+    def test_x_timestamp_not_overridden(self):
+        def do_test(method, base_headers, resp_code):
+            # no given x-timestamp
+            req = swob.Request.blank(
+                '/v1/a/c/o', method=method, headers=base_headers)
+            codes = [resp_code] * self.replicas()
+            with mocked_http_conn(*codes) as fake_conn:
+                resp = req.get_response(self.app)
+            self.assertEqual(resp.status_int, resp_code)
+            self.assertEqual(self.replicas(), len(fake_conn.requests))
+            for req in fake_conn.requests:
+                self.assertIn('X-Timestamp', req['headers'])
+                # check value can be parsed as valid timestamp
+                Timestamp(req['headers']['X-Timestamp'])
+
+            # given x-timestamp is retained
+            def do_check(ts):
+                headers = dict(base_headers)
+                headers['X-Timestamp'] = ts.internal
+                req = swob.Request.blank(
+                    '/v1/a/c/o', method=method, headers=headers)
+                codes = [resp_code] * self.replicas()
+                with mocked_http_conn(*codes) as fake_conn:
+                    resp = req.get_response(self.app)
+                self.assertEqual(resp.status_int, resp_code)
+                self.assertEqual(self.replicas(), len(fake_conn.requests))
+                for req in fake_conn.requests:
+                    self.assertEqual(ts.internal,
+                                     req['headers']['X-Timestamp'])
+
+            do_check(Timestamp.now())
+            do_check(Timestamp.now(offset=123))
+
+            # given x-timestamp gets sanity checked
+            headers = dict(base_headers)
+            headers['X-Timestamp'] = 'bad timestamp'
+            req = swob.Request.blank(
+                '/v1/a/c/o', method=method, headers=headers)
+            with mocked_http_conn() as fake_conn:
+                resp = req.get_response(self.app)
+            self.assertEqual(resp.status_int, 400)
+            self.assertIn('X-Timestamp should be a UNIX timestamp ', resp.body)
+
+        do_test('PUT', {'Content-Length': 0}, 200)
+        do_test('DELETE', {}, 204)
+
 
 @patch_policies(
     [StoragePolicy(0, '1-replica', True),