change .data vrs .meta file metadata filtering in obj.diskfile

Add DATAFILE_SYSTEM_META to diskfile.py which is a set of
system-set metadata keys that cannot be changed with a POST.

Fixes: bug #1214607

Change-Id: I4bdfc1e4813a1d27fe726ba83481c6e7194aab7f

swift/obj/diskfile.py

@@ -46,6 +46,9 @@ PICKLE_PROTOCOL = 2
 ONE_WEEK = 604800
 HASH_FILE = 'hashes.pkl'
 METADATA_KEY = 'user.swift.metadata'
+# These are system-set metadata keys that cannot be changed with a POST.
+# They should be lowercase.
+DATAFILE_SYSTEM_META = set('content-length content-type deleted etag'.split())
 
 
 def read_metadata(fd):
@@ -350,8 +353,7 @@ class DiskFile(object):
     def __init__(self, path, device, partition, account, container, obj,
                  logger, keep_data_fp=False, disk_chunk_size=65536,
                  bytes_per_sync=(512 * 1024 * 1024), iter_hook=None,
-                 threadpool=None, obj_dir='objects', mount_check=False,
+                 threadpool=None, obj_dir='objects', mount_check=False):
-                 disallowed_metadata_keys=None):
         if mount_check and not check_mount(path, device):
             raise DiskFileDeviceUnavailable()
         self.disk_chunk_size = disk_chunk_size
@@ -364,7 +366,6 @@ class DiskFile(object):
         self.device_path = join(path, device)
         self.tmpdir = join(path, device, 'tmp')
         self.logger = logger
-        self.disallowed_metadata_keys = disallowed_metadata_keys or []
         self.metadata = {}
         self.data_file = None
         self.fp = None
@@ -394,15 +395,20 @@ class DiskFile(object):
         if not self.data_file:
             return
         self.fp = open(self.data_file, 'rb')
-        self.metadata = read_metadata(self.fp)
+        datafile_metadata = read_metadata(self.fp)
         if not keep_data_fp:
             self.close(verify_file=False)
+
         if meta_file:
             with open(meta_file) as mfp:
-                for key in self.metadata.keys():
+                self.metadata = read_metadata(mfp)
-                    if key.lower() not in self.disallowed_metadata_keys:
+            sys_metadata = dict(
-                        del self.metadata[key]
+                [(key, val) for key, val in datafile_metadata.iteritems()
-                self.metadata.update(read_metadata(mfp))
+                 if key.lower() in DATAFILE_SYSTEM_META])
+            self.metadata.update(sys_metadata)
+        else:
+            self.metadata = datafile_metadata
+
         if 'name' in self.metadata:
             if self.metadata['name'] != self.name:
                 self.logger.error(_('Client path %(client)s does not match '

swift/obj/server.py

@@ -45,14 +45,13 @@ from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPCreated, \
     HTTPClientDisconnect, HTTPMethodNotAllowed, Request, Response, UTC, \
     HTTPInsufficientStorage, HTTPForbidden, HTTPException, HeaderKeyDict, \
     HTTPConflict
-from swift.obj.diskfile import DiskFile, get_hashes
+from swift.obj.diskfile import DATAFILE_SYSTEM_META, DiskFile, \
+    get_hashes
 
 
 DATADIR = 'objects'
 ASYNCDIR = 'async_pending'
 MAX_OBJECT_NAME_LENGTH = 1024
-# keep these lower-case
-DISALLOWED_HEADERS = set('content-length content-type deleted etag'.split())
 
 
 def _parse_path(request, minsegs=5, maxsegs=5):
@@ -109,10 +108,15 @@ class ObjectController(object):
             x-object-manifest,
             x-static-large-object,
         '''
-        self.allowed_headers = set(
+        extra_allowed_headers = [
-            i.strip().lower() for i in
+            header.strip().lower() for header in conf.get(
-            conf.get('allowed_headers', default_allowed_headers).split(',')
+                'allowed_headers', default_allowed_headers).split(',')
-            if i.strip() and i.strip().lower() not in DISALLOWED_HEADERS)
+            if header.strip()
+        ]
+        self.allowed_headers = set()
+        for header in extra_allowed_headers:
+            if header not in DATAFILE_SYSTEM_META:
+                self.allowed_headers.add(header)
         self.expiring_objects_account = \
             (conf.get('auto_create_account_prefix') or '.') + \
             'expiring_objects'
@@ -126,7 +130,6 @@ class ObjectController(object):
         kwargs.setdefault('disk_chunk_size', self.disk_chunk_size)
         kwargs.setdefault('threadpool', self.threadpools[device])
         kwargs.setdefault('obj_dir', DATADIR)
-        kwargs.setdefault('disallowed_metadata_keys', DISALLOWED_HEADERS)
         return DiskFile(self.devices, device, partition, account,
                         container, obj, self.logger, **kwargs)
 

test/unit/obj/test_diskfile.py

@@ -343,10 +343,23 @@ class TestDiskFile(unittest.TestCase):
         return df
 
     def test_disk_file_default_disallowed_metadata(self):
-        keep_data_fp = True
+        # build an object with some meta (ts 41)
+        orig_metadata = {'X-Object-Meta-Key1': 'Value1',
+                         'Content-Type': 'text/garbage'}
+        df = self._get_disk_file(ts=41, extra_metadata=orig_metadata)
+        self.assertEquals('1024', df.metadata['Content-Length'])
+        # write some new metadata (fast POST, don't send orig meta, ts 42)
         df = diskfile.DiskFile(self.testdir, 'sda1', '0', 'a', 'c', 'o',
-                                    FakeLogger(), keep_data_fp=keep_data_fp)
+                                    FakeLogger())
-        self.assertEquals(df.disallowed_metadata_keys, [])
+        df.put_metadata({'X-Timestamp': '42', 'X-Object-Meta-Key2': 'Value2'})
+        df = diskfile.DiskFile(self.testdir, 'sda1', '0', 'a', 'c', 'o',
+                                    FakeLogger())
+        # non-fast-post updateable keys are preserved
+        self.assertEquals('text/garbage', df.metadata['Content-Type'])
+        # original fast-post updateable keys are removed
+        self.assert_('X-Object-Meta-Key1' not in df.metadata)
+        # new fast-post updateable keys are added
+        self.assertEquals('Value2', df.metadata['X-Object-Meta-Key2'])
 
     def test_disk_file_app_iter_corners(self):
         df = self._create_test_file('1234567890')
@@ -490,7 +503,8 @@ class TestDiskFile(unittest.TestCase):
 
     def _get_disk_file(self, invalid_type=None, obj_name='o',
                        fsize=1024, csize=8, mark_deleted=False, ts=None,
-                       iter_hook=None, mount_check=False):
+                       iter_hook=None, mount_check=False,
+                       extra_metadata=None):
         '''returns a DiskFile'''
         df = diskfile.DiskFile(self.testdir, 'sda1', '0', 'a', 'c',
                                     obj_name, FakeLogger())
@@ -509,6 +523,7 @@ class TestDiskFile(unittest.TestCase):
                 'X-Timestamp': timestamp,
                 'Content-Length': str(os.fstat(writer.fd).st_size),
             }
+            metadata.update(extra_metadata or {})
             writer.put(metadata)
             if invalid_type == 'ETag':
                 etag = md5()