Merge pull request #53678 from twangboy/fix_lock_1688_lgpo

Fix LGPO when string object is None

salt/modules/win_lgpo.py

@@ -5728,6 +5728,18 @@ def _checkValueItemParent(policy_element, policy_name, policy_key,
     return False
 
 
+def _encode_string(value):
+    encoded_null = chr(0).encode('utf-16-le')
+    if value is None:
+        return encoded_null
+    else:
+        # Should we raise an error here, or attempt to cast to a string
+        if not isinstance(value, six.string_types):
+            raise TypeError('Value {0} is not a string type\n'
+                            'Type: {1}'.format(repr(value), type(value)))
+        return b''.join([value.encode('utf-16-le'), encoded_null])
+
+
 def _buildKnownDataSearchString(reg_key, reg_valueName, reg_vtype, reg_data,
                                 check_deleted=False):
     '''
@@ -5749,8 +5761,7 @@ def _buildKnownDataSearchString(reg_key, reg_valueName, reg_vtype, reg_data,
         elif reg_vtype == "REG_QWORD":
             this_element_value = struct.pack(b'Q', int(reg_data))
         elif reg_vtype == 'REG_SZ':
-            this_element_value = b''.join([reg_data.encode('utf-16-le'),
-                                           encoded_null])
+            this_element_value = _encode_string(reg_data)
     if check_deleted:
         reg_vtype = 'REG_SZ'
         expected_string = b''.join(['['.encode('utf-16-le'),
@@ -5845,8 +5856,7 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
             return None
     elif etree.QName(element).localname == 'string':
         this_vtype = 'REG_SZ'
-        this_element_value = b''.join([element.text.encode('utf-16-le'),
-                                       encoded_null])
+        this_element_value = _encode_string(element.text)
     elif etree.QName(parent_element).localname == 'elements':
         standard_element_expected_string = True
         if etree.QName(element).localname == 'boolean':
@@ -5889,9 +5899,7 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
             if 'expandable' in element.attrib:
                 if element.attrib['expandable'].lower() == 'true':
                     this_vtype = 'REG_EXPAND_SZ'
-            if this_element_value is not None:
-                this_element_value = b''.join([this_element_value.encode('utf-16-le'),
-                                               encoded_null])
+            this_element_value = _encode_string(this_element_value)
         elif etree.QName(element).localname == 'multiText':
             this_vtype = 'REG_MULTI_SZ' if not check_deleted else 'REG_SZ'
             if this_element_value is not None:
@@ -5959,8 +5967,7 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
                                                                       six.unichr(len('{0}{1}'.format(element_values[i],
                                                                                  chr(0)).encode('utf-16-le'))).encode('utf-32-le'),
                                                                       encoded_semicolon,
-                                                                      b''.join([element_values[i].encode('utf-16-le'),
-                                                                               encoded_null]),
+                                                                      _encode_string(element_values[i]),
                                                                       ']'.encode('utf-16-le')])
                 else:
                     expected_string = del_keys + b''.join(['['.encode('utf-16-le'),

tests/unit/modules/test_win_lgpo.py

@@ -0,0 +1,55 @@
+# -*- coding: utf-8 -*-
+'''
+:codeauthor: Shane Lee <slee@saltstack.com>
+'''
+
+# Import Python Libs
+from __future__ import absolute_import, unicode_literals, print_function
+
+# Import Salt Testing Libs
+from tests.support.unit import TestCase
+
+# Import Salt Libs
+import salt.modules.win_lgpo as win_lgpo
+
+
+class WinSystemTestCase(TestCase):
+    '''
+    Test cases for salt.modules.win_lgpo
+    '''
+    encoded_null = chr(0).encode('utf-16-le')
+
+    def test__encode_string(self):
+        '''
+        ``_encode_string`` should return a null terminated ``utf-16-le`` encoded
+        string when a string value is passed
+        '''
+        encoded_value = b''.join(['Salt is awesome'.encode('utf-16-le'),
+                                  self.encoded_null])
+        value = win_lgpo._encode_string('Salt is awesome')
+        self.assertEqual(value, encoded_value)
+
+    def test__encode_string_empty_string(self):
+        '''
+        ``_encode_string`` should return an encoded null when an empty string
+        value is passed
+        '''
+        value = win_lgpo._encode_string('')
+        self.assertEqual(value, self.encoded_null)
+
+    def test__encode_string_error(self):
+        '''
+        ``_encode_string`` should raise an error if a non-string value is passed
+        '''
+        self.assertRaises(TypeError, win_lgpo._encode_string, [1])
+        test_list = ['item1', 'item2']
+        self.assertRaises(TypeError, win_lgpo._encode_string, [test_list])
+        test_dict = {'key1': 'value1', 'key2': 'value2'}
+        self.assertRaises(TypeError, win_lgpo._encode_string, [test_dict])
+
+    def test__encode_string_none(self):
+        '''
+        ``_encode_string`` should return an encoded null when ``None`` is passed
+        '''
+        value = win_lgpo._encode_string(None)
+        self.assertEqual(value, self.encoded_null)