From 47f401b2ef0023f57b1fce83b10f9c6a3aa5c757 Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Tue, 12 Nov 2013 15:26:10 -0500 Subject: [PATCH] fix metadata overwrite during a post request During a post request, the object-server is ovewriting the existing object metadata. This fix prevents the overwrite of the system metadata while it allows for the user to add/remove user metadata Change-Id: Ic62cd064589b625ee425a9934be8766650622c13 Signed-off-by: Thiago da Silva Reviewed-on: http://review.gluster.org/6254 Reviewed-by: Luis Pabon Tested-by: Luis Pabon Signed-off-by: Thiago da Silva Reviewed-on: http://review.gluster.org/6315 --- .../middleware/gswauth/swauth/middleware.py | 46 +++----- gluster/swift/obj/diskfile.py | 37 ++++++- test/unit/obj/test_diskfile.py | 100 +++++++++++++++++- 3 files changed, 140 insertions(+), 43 deletions(-) diff --git a/gluster/swift/common/middleware/gswauth/swauth/middleware.py b/gluster/swift/common/middleware/gswauth/swauth/middleware.py index 6388aa2..996228d 100644 --- a/gluster/swift/common/middleware/gswauth/swauth/middleware.py +++ b/gluster/swift/common/middleware/gswauth/swauth/middleware.py @@ -40,13 +40,9 @@ from swift.common.utils import cache_from_env, get_logger, get_remote_client, \ import swift.common.wsgi -from gluster.swift.common.middleware.gswauth.swauth import swift_version from gluster.swift.common.middleware.gswauth.swauth import authtypes -MEMCACHE_TIME = swift_version.newer_than('1.7.7-dev') - - class Swauth(object): """ Scalable authentication and authorization system that uses Swift as its @@ -349,14 +345,9 @@ class Swauth(object): expires_from_now = float(resp.getheader('x-auth-ttl')) groups = resp.getheader('x-auth-groups') if memcache_client: - if MEMCACHE_TIME: - memcache_client.set( - memcache_key, (time() + expires_from_now, groups), - time=expires_from_now) - else: - memcache_client.set( - memcache_key, (time() + expires_from_now, groups), - timeout=expires_from_now) + memcache_client.set( + memcache_key, (time() + expires_from_now, groups), + timeout=expires_from_now) else: path = quote('/v1/%s/.token_%s/%s' % (self.auth_account, token[-1], token)) @@ -375,16 +366,10 @@ class Swauth(object): groups.append(detail['account_id']) groups = ','.join(groups) if memcache_client: - if MEMCACHE_TIME: - memcache_client.set( - memcache_key, - (detail['expires'], groups), - time=float(detail['expires'] - time())) - else: - memcache_client.set( - memcache_key, - (detail['expires'], groups), - timeout=float(detail['expires'] - time())) + memcache_client.set( + memcache_key, + (detail['expires'], groups), + timeout=float(detail['expires'] - time())) return groups def authorize(self, req): @@ -1369,18 +1354,11 @@ class Swauth(object): if not memcache_client: raise Exception( 'No memcache set up; required for Swauth middleware') - if MEMCACHE_TIME: - memcache_client.set( - memcache_key, - (self.itoken_expires, - '.auth,.reseller_admin,%s.auth' % self.reseller_prefix), - time=self.token_life) - else: - memcache_client.set( - memcache_key, - (self.itoken_expires, - '.auth,.reseller_admin,%s.auth' % self.reseller_prefix), - timeout=self.token_life) + memcache_client.set( + memcache_key, + (self.itoken_expires, + '.auth,.reseller_admin,%s.auth' % self.reseller_prefix), + timeout=self.token_life) return self.itoken def get_admin_detail(self, req): diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index 6e6856d..cccd5e6 100644 --- a/gluster/swift/obj/diskfile.py +++ b/gluster/swift/obj/diskfile.py @@ -44,7 +44,8 @@ from gluster.swift.common.utils import read_metadata, write_metadata, \ get_object_metadata from gluster.swift.common.utils import X_CONTENT_TYPE, \ X_TIMESTAMP, X_TYPE, X_OBJECT_TYPE, FILE, OBJECT, DIR_TYPE, \ - FILE_TYPE, DEFAULT_UID, DEFAULT_GID, DIR_NON_OBJECT, DIR_OBJECT + FILE_TYPE, DEFAULT_UID, DEFAULT_GID, DIR_NON_OBJECT, DIR_OBJECT, \ + X_ETAG, X_CONTENT_LENGTH from ConfigParser import ConfigParser, NoSectionError, NoOptionError # FIXME: Hopefully we'll be able to move to Python 2.7+ where O_CLOEXEC will @@ -260,7 +261,8 @@ def _adjust_metadata(metadata): # Fix up the metadata to ensure it has a proper value for the # Content-Type metadata, as well as an X_TYPE and X_OBJECT_TYPE # metadata values. - content_type = metadata[X_CONTENT_TYPE] + content_type = metadata.get(X_CONTENT_TYPE, '') + if not content_type: # FIXME: How can this be that our caller supplied us with metadata # that has a content type that evaluates to False? @@ -983,12 +985,39 @@ class DiskFile(object): :raises DiskFileError: this implementation will raise the same errors as the `create()` method. """ - # FIXME: we need to validate system metadata is preserved - metadata = _adjust_metadata(metadata) + metadata = self._keep_sys_metadata(metadata) data_file = os.path.join(self._put_datadir, self._obj) self._threadpool.run_in_thread( write_metadata, data_file, metadata) + def _keep_sys_metadata(self, metadata): + """ + Make sure system metadata is not lost when writing new user metadata + + This method will read the existing metadata and check for system + metadata. If there are any, it should be appended to the metadata obj + the user is trying to write. + """ + orig_metadata = self.read_metadata() + + sys_keys = [X_CONTENT_TYPE, X_ETAG, 'name', X_CONTENT_LENGTH, + X_OBJECT_TYPE, X_TYPE] + + for key in sys_keys: + if key in orig_metadata: + metadata[key] = orig_metadata[key] + + if X_OBJECT_TYPE not in orig_metadata: + if metadata[X_CONTENT_TYPE].lower() == DIR_TYPE: + metadata[X_OBJECT_TYPE] = DIR_OBJECT + else: + metadata[X_OBJECT_TYPE] = FILE + + if X_TYPE not in orig_metadata: + metadata[X_TYPE] = OBJECT + + return metadata + def _unlinkold(self): if self._is_dir: # Marker, or object, directory. diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index 8270e05..340087f 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -425,11 +425,101 @@ class TestDiskFile(unittest.TestCase): gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z") md = {'Content-Type': 'application/octet-stream', 'a': 'b'} gdf.write_metadata(md.copy()) - on_disk_md = _metadata[_mapit(the_dir)] - del on_disk_md['X-Type'] - del on_disk_md['X-Object-Type'] - assert on_disk_md == md, "on_disk_md = %r, md = %r" % ( - on_disk_md, md) + self.assertIsNone(gdf._metadata) + fmd = _metadata[_mapit(the_dir)] + md.update({'X-Object-Type': 'file', 'X-Type': 'Object'}) + self.assertTrue(fmd['a'], md['a']) + self.assertTrue(fmd['Content-Type'], md['Content-Type']) + + def test_add_metadata_to_existing_file(self): + the_path = os.path.join(self.td, "vol0", "bar") + the_file = os.path.join(the_path, "z") + os.makedirs(the_path) + with open(the_file, "wb") as fd: + fd.write("1234") + ini_md = { + 'X-Type': 'Object', + 'X-Object-Type': 'file', + 'Content-Length': 4, + 'ETag': 'etag', + 'X-Timestamp': 'ts', + 'Content-Type': 'application/loctet-stream'} + _metadata[_mapit(the_file)] = ini_md + gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z") + md = {'Content-Type': 'application/octet-stream', 'a': 'b'} + gdf.write_metadata(md.copy()) + self.assertTrue(_metadata[_mapit(the_file)]['a'], 'b') + newmd = {'X-Object-Meta-test':'1234'} + gdf.write_metadata(newmd.copy()) + on_disk_md = _metadata[_mapit(the_file)] + self.assertTrue(on_disk_md['Content-Length'], 4) + self.assertTrue(on_disk_md['X-Object-Meta-test'], '1234') + self.assertTrue(on_disk_md['X-Type'], 'Object') + self.assertTrue(on_disk_md['X-Object-Type'], 'file') + self.assertTrue(on_disk_md['ETag'], 'etag') + self.assertFalse('a' in on_disk_md) + + def test_add_md_to_existing_file_with_md_in_gdf(self): + the_path = os.path.join(self.td, "vol0", "bar") + the_file = os.path.join(the_path, "z") + os.makedirs(the_path) + with open(the_file, "wb") as fd: + fd.write("1234") + ini_md = { + 'X-Type': 'Object', + 'X-Object-Type': 'file', + 'Content-Length': 4, + 'name': 'z', + 'ETag': 'etag', + 'X-Timestamp': 'ts'} + _metadata[_mapit(the_file)] = ini_md + gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z") + + # make sure gdf has the _metadata + gdf.open() + md = {'a': 'b'} + gdf.write_metadata(md.copy()) + self.assertTrue(_metadata[_mapit(the_file)]['a'], 'b') + newmd = {'X-Object-Meta-test':'1234'} + gdf.write_metadata(newmd.copy()) + on_disk_md = _metadata[_mapit(the_file)] + self.assertTrue(on_disk_md['Content-Length'], 4) + self.assertTrue(on_disk_md['X-Object-Meta-test'], '1234') + self.assertFalse('a' in on_disk_md) + + def test_add_metadata_to_existing_dir(self): + the_cont = os.path.join(self.td, "vol0", "bar") + the_dir = os.path.join(the_cont, "dir") + os.makedirs(the_dir) + gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "dir") + self.assertEquals(gdf._metadata, None) + init_md = { + 'X-Type': 'Object', + 'Content-Length': 0, + 'ETag': 'etag', + 'X-Timestamp': 'ts', + 'X-Object-Meta-test':'test', + 'Content-Type': 'application/directory'} + _metadata[_mapit(the_dir)] = init_md + + md = {'X-Object-Meta-test':'test'} + gdf.write_metadata(md.copy()) + self.assertEqual(_metadata[_mapit(the_dir)]['X-Object-Meta-test'], + 'test') + self.assertEqual(_metadata[_mapit(the_dir)]['Content-Type'].lower(), + 'application/directory') + + # set new metadata + newmd = {'X-Object-Meta-test2':'1234'} + gdf.write_metadata(newmd.copy()) + self.assertEqual(_metadata[_mapit(the_dir)]['Content-Type'].lower(), + 'application/directory') + self.assertEqual(_metadata[_mapit(the_dir)]["X-Object-Meta-test2"], + '1234') + self.assertEqual(_metadata[_mapit(the_dir)]['X-Object-Type'], + DIR_OBJECT) + self.assertFalse('X-Object-Meta-test' in _metadata[_mapit(the_dir)]) + def test_write_metadata_w_meta_file(self): the_path = os.path.join(self.td, "vol0", "bar")