From d69e013519201af9af7683b1b6dfdf1efa226c7c Mon Sep 17 00:00:00 2001 From: Kiyoung Jung Date: Thu, 7 Nov 2013 04:45:27 +0000 Subject: [PATCH] change the last-modified header value with valid one the Last-Modified header in Response didn't have a suitable value - an integer part of object's timestamp. This leads that the the if-[un]modified-since header with the value from last-modified is always earlier than timestamp and results the content is always newer than value of these conditional headers. Patched code returns math.ceil() of object's timestamp in Last-Modified header so the later conditional header works correctly Closes-Bug: #1248818 Change-Id: I1ece7d008551bf989da74d23f0ed6307c45c5436 --- swift/obj/server.py | 7 ++-- swift/proxy/controllers/obj.py | 3 +- test/functional/swift_test_client.py | 15 ++++++++- test/functional/tests.py | 22 ++++++++++++ test/unit/obj/test_server.py | 50 ++++++++++++++++++++++++++-- test/unit/proxy/test_server.py | 50 ++++++++++++++++++++++++++++ 6 files changed, 140 insertions(+), 7 deletions(-) diff --git a/swift/obj/server.py b/swift/obj/server.py index 6756fef6be..fbdca19bf6 100644 --- a/swift/obj/server.py +++ b/swift/obj/server.py @@ -21,6 +21,7 @@ import os import time import traceback import socket +import math from datetime import datetime from swift import gettext_ as _ from hashlib import md5 @@ -483,7 +484,7 @@ class ObjectController(object): except (OverflowError, ValueError): # catches timestamps before the epoch return HTTPPreconditionFailed(request=request) - if if_modified_since and file_x_ts_utc < if_modified_since: + if if_modified_since and file_x_ts_utc <= if_modified_since: return HTTPNotModified(request=request) keep_cache = (self.keep_cache_private or ('X-Auth-Token' not in request.headers and @@ -499,7 +500,7 @@ class ObjectController(object): key.lower() in self.allowed_headers: response.headers[key] = value response.etag = metadata['ETag'] - response.last_modified = file_x_ts_flt + response.last_modified = math.ceil(file_x_ts_flt) response.content_length = obj_size try: response.content_encoding = metadata[ @@ -541,7 +542,7 @@ class ObjectController(object): response.headers[key] = value response.etag = metadata['ETag'] ts = metadata['X-Timestamp'] - response.last_modified = float(ts) + response.last_modified = math.ceil(float(ts)) # Needed for container sync feature response.headers['X-Timestamp'] = ts response.content_length = int(metadata['Content-Length']) diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index e54d388ded..03fc745b83 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -28,6 +28,7 @@ import itertools import mimetypes import re import time +import math from datetime import datetime from swift import gettext_ as _ from urllib import unquote, quote @@ -1169,7 +1170,7 @@ class ObjectController(Controller): resp.headers['X-Copied-From-Last-Modified'] = \ source_resp.headers['last-modified'] copy_headers_into(req, resp) - resp.last_modified = float(req.headers['X-Timestamp']) + resp.last_modified = math.ceil(float(req.headers['X-Timestamp'])) return resp @public diff --git a/test/functional/swift_test_client.py b/test/functional/swift_test_client.py index f5e35c7a25..24a8f034c9 100644 --- a/test/functional/swift_test_client.py +++ b/test/functional/swift_test_client.py @@ -697,7 +697,8 @@ class File(Base): else: raise RuntimeError - def write(self, data='', hdrs={}, parms={}, callback=None, cfg={}): + def write(self, data='', hdrs={}, parms={}, callback=None, cfg={}, + return_resp=False): block_size = 2 ** 20 if isinstance(data, file): @@ -736,6 +737,9 @@ class File(Base): self.md5 = self.compute_md5sum(data) + if return_resp: + return self.conn.response + return True def write_random(self, size=None, hdrs={}, parms={}, cfg={}): @@ -744,3 +748,12 @@ class File(Base): raise ResponseError(self.conn.response) self.md5 = self.compute_md5sum(StringIO.StringIO(data)) return data + + def write_random_return_resp(self, size=None, hdrs={}, parms={}, cfg={}): + data = self.random_data(size) + resp = self.write(data, hdrs=hdrs, parms=parms, cfg=cfg, + return_resp=True) + if not resp: + raise ResponseError(self.conn.response) + self.md5 = self.compute_md5sum(StringIO.StringIO(data)) + return resp diff --git a/test/functional/tests.py b/test/functional/tests.py index d34489c418..c33e1bd926 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -1620,6 +1620,28 @@ class TestFileComparison(Base): self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(412) + def testLastModified(self): + file_name = Utils.create_name() + content_type = Utils.create_name() + + file = self.env.container.file(file_name) + file.content_type = content_type + resp = file.write_random_return_resp(self.env.file_size) + put_last_modified = resp.getheader('last-modified') + + file = self.env.container.file(file_name) + info = file.info() + self.assert_('last_modified' in info) + last_modified = info['last_modified'] + self.assertEqual(put_last_modified, info['last_modified']) + + hdrs = {'If-Modified-Since': last_modified} + self.assertRaises(ResponseError, file.read, hdrs=hdrs) + self.assert_status(304) + + hdrs = {'If-Unmodified-Since': last_modified} + self.assert_(file.read(hdrs=hdrs)) + class TestFileComparisonUTF8(Base2, TestFileComparison): set_up = False diff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py index bc8b491245..48873ae054 100755 --- a/test/unit/obj/test_server.py +++ b/test/unit/obj/test_server.py @@ -21,6 +21,7 @@ import operator import os import mock import unittest +import math from shutil import rmtree from StringIO import StringIO from time import gmtime, strftime, time @@ -672,7 +673,8 @@ class TestObjectController(unittest.TestCase): self.assertEquals(resp.headers['content-type'], 'application/x-test') self.assertEquals( resp.headers['last-modified'], - strftime('%a, %d %b %Y %H:%M:%S GMT', gmtime(float(timestamp)))) + strftime('%a, %d %b %Y %H:%M:%S GMT', + gmtime(math.ceil(float(timestamp))))) self.assertEquals(resp.headers['etag'], '"0b4c12d7e0a73840c1c4f148fda3b037"') self.assertEquals(resp.headers['x-object-meta-1'], 'One') @@ -774,7 +776,8 @@ class TestObjectController(unittest.TestCase): self.assertEquals(resp.headers['content-type'], 'application/x-test') self.assertEquals( resp.headers['last-modified'], - strftime('%a, %d %b %Y %H:%M:%S GMT', gmtime(float(timestamp)))) + strftime('%a, %d %b %Y %H:%M:%S GMT', + gmtime(math.ceil(float(timestamp))))) self.assertEquals(resp.headers['etag'], '"0b4c12d7e0a73840c1c4f148fda3b037"') self.assertEquals(resp.headers['x-object-meta-1'], 'One') @@ -976,6 +979,37 @@ class TestObjectController(unittest.TestCase): resp = req.get_response(self.object_controller) self.assertEquals(resp.status_int, 304) + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.object_controller) + since = resp.headers['Last-Modified'] + self.assertEquals(since, strftime('%a, %d %b %Y %H:%M:%S GMT', + gmtime(math.ceil(float(timestamp))))) + + req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Modified-Since': since}) + resp = self.object_controller.GET(req) + self.assertEquals(resp.status_int, 304) + + timestamp = normalize_timestamp(int(time())) + req = Request.blank('/sda1/p/a/c/o2', + environ={'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Timestamp': timestamp, + 'Content-Type': 'application/octet-stream', + 'Content-Length': '4'}) + req.body = 'test' + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 201) + + since = strftime('%a, %d %b %Y %H:%M:%S GMT', + gmtime(float(timestamp))) + req = Request.blank('/sda1/p/a/c/o2', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Modified-Since': since}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 304) + def test_GET_if_unmodified_since(self): timestamp = normalize_timestamp(time()) req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, @@ -1012,6 +1046,18 @@ class TestObjectController(unittest.TestCase): resp = req.get_response(self.object_controller) self.assertEquals(resp.status_int, 200) + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.object_controller) + since = resp.headers['Last-Modified'] + self.assertEquals(since, strftime('%a, %d %b %Y %H:%M:%S GMT', + gmtime(math.ceil(float(timestamp))))) + + req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Unmodified-Since': since}) + resp = self.object_controller.GET(req) + self.assertEquals(resp.status_int, 200) + def test_GET_quarantine(self): # Test swift.obj.server.ObjectController.GET timestamp = normalize_timestamp(time()) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index a16bf219e5..f5cb411ea5 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -998,6 +998,56 @@ class TestObjectController(unittest.TestCase): finally: swift.proxy.controllers.obj.MAX_FILE_SIZE = MAX_FILE_SIZE + def test_PUT_last_modified(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.last_modified HTTP/1.1\r\n' + 'Host: localhost\r\nConnection: close\r\n' + 'X-Storage-Token: t\r\nContent-Length: 0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + lm_hdr = 'Last-Modified: ' + self.assertEqual(headers[:len(exp)], exp) + + last_modified_put = [line for line in headers.split('\r\n') + if lm_hdr in line][0][len(lm_hdr):] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('HEAD /v1/a/c/o.last_modified HTTP/1.1\r\n' + 'Host: localhost\r\nConnection: close\r\n' + 'X-Storage-Token: t\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 200' + self.assertEqual(headers[:len(exp)], exp) + last_modified_head = [line for line in headers.split('\r\n') + if lm_hdr in line][0][len(lm_hdr):] + self.assertEqual(last_modified_put, last_modified_head) + + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n' + 'Host: localhost\r\nConnection: close\r\n' + 'If-Modified-Since: %s\r\n' + 'X-Storage-Token: t\r\n\r\n' % last_modified_put) + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 304' + self.assertEqual(headers[:len(exp)], exp) + + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n' + 'Host: localhost\r\nConnection: close\r\n' + 'If-Unmodified-Since: %s\r\n' + 'X-Storage-Token: t\r\n\r\n' % last_modified_put) + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 200' + self.assertEqual(headers[:len(exp)], exp) + def test_expirer_DELETE_on_versioned_object(self): test_errors = []