Raise ValueError if empty value coming into encrypt_header_val

encrypt_header_val is used to translate a raw header value into an
encrypted value. Semantically, a header with an empty value won't be stored
and all callers seem to remove such a header before calling
encrypted_header_val.

So if no reason for returning ('', None), I think it's better to change it to
raise ValueError not to cause another error for users of the return value.
(e.g. dump_crypto_meta)

Plus this patch addes a few unit tests for those cases above.

Change-Id: Ic1237f4afb8c0e466be5ce59fe31b667c39242b0

swift/common/middleware/crypto/encrypter.py

@@ -39,9 +39,10 @@ def encrypt_header_val(crypto, value, key):
     :returns: a tuple of (encrypted value, crypto_meta) where crypto_meta is a
         dict of form returned by
         :py:func:`~swift.common.middleware.crypto.Crypto.get_crypto_meta`
+    :raises ValueError: if value is empty
     """
     if not value:
-        return '', None
+        raise ValueError('empty value is not acceptable')
 
     crypto_meta = crypto.create_crypto_meta()
     crypto_ctxt = crypto.create_encryption_ctxt(key, crypto_meta['iv'])

test/unit/common/middleware/crypto/test_encrypter.py

@@ -536,6 +536,7 @@ class TestEncrypter(unittest.TestCase):
         env = {'REQUEST_METHOD': 'POST',
                CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
         hdrs = {'x-object-meta-test': 'encrypt me',
+                'x-object-meta-test2': '',
                 'x-object-sysmeta-test': 'do not encrypt me'}
         req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs)
         key = fetch_crypto_keys()['object']
@@ -551,6 +552,8 @@ class TestEncrypter(unittest.TestCase):
 
         # user meta is encrypted
         self._verify_user_metadata(req_hdrs, 'Test', 'encrypt me', key)
+        # unless it had no value
+        self.assertEqual('', req_hdrs['X-Object-Meta-Test2'])
 
         # sysmeta is not encrypted
         self.assertEqual('do not encrypt me',
@@ -878,6 +881,36 @@ class TestEncrypter(unittest.TestCase):
             req.get_response(app)
         self.assertEqual(FakeAppThatExcepts.MESSAGE, catcher.exception.body)
 
+    def test_encrypt_header_val(self):
+        # Prepare key and Crypto instance
+        object_key = fetch_crypto_keys()['object']
+
+        # - Normal string can be crypted
+        encrypted = encrypter.encrypt_header_val(Crypto(), 'aaa', object_key)
+        # sanity: return value is 2 item tuple
+        self.assertEqual(2, len(encrypted))
+        crypted_val, crypt_info = encrypted
+        expected_crypt_val = base64.b64encode(
+            encrypt('aaa', object_key, FAKE_IV))
+        expected_crypt_info = {
+            'cipher': 'AES_CTR_256', 'iv': 'This is an IV123'}
+        self.assertEqual(expected_crypt_val, crypted_val)
+        self.assertEqual(expected_crypt_info, crypt_info)
+
+        # - Empty string raises a ValueError for safety
+        with self.assertRaises(ValueError) as cm:
+            encrypter.encrypt_header_val(Crypto(), '', object_key)
+
+        self.assertEqual('empty value is not acceptable',
+                         cm.exception.message)
+
+        # - None also raises a ValueError for safety
+        with self.assertRaises(ValueError) as cm:
+            encrypter.encrypt_header_val(Crypto(), None, object_key)
+
+        self.assertEqual('empty value is not acceptable',
+                         cm.exception.message)
+
 
 if __name__ == '__main__':
     unittest.main()