From 67dd1c18c7065ed81ca3293097b4ffc4fba7fb88 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 13 Feb 2020 15:17:09 -0700 Subject: [PATCH 1/2] Fix issue with existing reg_dword entries When passing an invalid reg_dword type in the state file, such as '000001', the registry code casts it to the proper type. In the case of '000001' it would be case to an integer `11. This conversion was happening after the comparison to see if changes were to be made, thus comparing '000001' to the existing integer `1`. This PR moves the casting of the state file data before the comparison so that it is comparing proper types. Also fixes some issues with the comments being returned by the state file where it was returning incorrect duplicate strings. ` added to `. Now it returns ` name added to ` Added tests for the integer comparison. Fixed tests for the new comments --- salt/states/reg.py | 12 +++---- tests/unit/states/test_reg.py | 56 ++++++++++++++++++++++++++++++-- tests/unit/utils/test_win_reg.py | 4 +++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/salt/states/reg.py b/salt/states/reg.py index f776a342301c..af3510aabc92 100644 --- a/salt/states/reg.py +++ b/salt/states/reg.py @@ -396,10 +396,13 @@ def present(name, vname=vname, use_32bit_registry=use_32bit_registry) + # Cast the vdata according to the vtype + vdata_decoded = __utils__['reg.cast_vdata'](vdata=vdata, vtype=vtype) + # Check if the key already exists # If so, check perms # We check `vdata` and `success` because `vdata` can be None - if vdata == reg_current['vdata'] and reg_current['success']: + if vdata_decoded == reg_current['vdata'] and reg_current['success']: ret['comment'] = '{0} in {1} is already present' \ ''.format(salt.utils.stringutils.to_unicode(vname, 'utf-8') if vname else '(Default)', salt.utils.stringutils.to_unicode(name, 'utf-8')) @@ -413,9 +416,6 @@ def present(name, inheritance=win_inheritance, reset=win_perms_reset) - # Cast the vdata according to the vtype - vdata_decoded = __utils__['reg.cast_vdata'](vdata=vdata, vtype=vtype) - add_change = {'Key': r'{0}\{1}'.format(hive, key), 'Entry': '{0}'.format(salt.utils.stringutils.to_unicode(vname, 'utf-8') if vname else '(Default)'), 'Value': vdata_decoded, @@ -440,10 +440,10 @@ def present(name, if not ret['result']: ret['changes'] = {} - ret['comment'] = r'Failed to add {0} to {1}\{2}'.format(name, hive, key) + ret['comment'] = r'Failed to add {0} to {1}\{2}'.format(vname, hive, key) else: ret['changes'] = {'reg': {'Added': add_change}} - ret['comment'] = r'Added {0} to {1}\{2}'.format(name, hive, key) + ret['comment'] = r'Added {0} to {1}\{2}'.format(vname, hive, key) if ret['result']: ret = __utils__['dacl.check_perms']( diff --git a/tests/unit/states/test_reg.py b/tests/unit/states/test_reg.py index c65178cb96e1..e20df558485c 100644 --- a/tests/unit/states/test_reg.py +++ b/tests/unit/states/test_reg.py @@ -49,7 +49,7 @@ def test_present(self): Test to set a registry entry. ''' expected = { - 'comment': 'Added {0} to {0}'.format(self.name), + 'comment': 'Added {0} to {1}'.format(self.vname, self.name), 'pchanges': {}, 'changes': { 'reg': { @@ -58,15 +58,65 @@ def test_present(self): 'Perms': { 'Deny': None, 'Grant': None}, - 'Value': '0.15.3', + 'Value': self.vdata, 'Key': self.name, 'Owner': None, - 'Entry': 'version'}}}, + 'Entry': self.vname}}}, 'name': self.name, 'result': True} ret = reg.present(self.name, vname=self.vname, vdata=self.vdata) self.assertDictEqual(ret, expected) + @destructiveTest + def test_present_string_dword(self): + ''' + Test to set a registry entry. + ''' + vname = 'dword_data' + vdata = '00000001' + vtype = 'REG_DWORD' + expected_vdata = 1 + expected = { + 'comment': 'Added {0} to {1}'.format(vname, self.name), + 'pchanges': {}, + 'changes': { + 'reg': { + 'Added': { + 'Inheritance': True, + 'Perms': { + 'Deny': None, + 'Grant': None}, + 'Value': expected_vdata, + 'Key': self.name, + 'Owner': None, + 'Entry': vname}}}, + 'name': self.name, + 'result': True} + ret = reg.present( + self.name, vname=vname, vdata=vdata, vtype=vtype) + self.assertDictEqual(ret, expected) + + @destructiveTest + def test_present_string_dword_existing(self): + ''' + Test to set a registry entry. + ''' + vname = 'dword_data' + vdata = '0000001' + vtype = 'REG_DWORD' + # Set it first + reg.present( + self.name, vname=vname, vdata=vdata, vtype=vtype) + expected = { + 'comment': '{0} in {1} is already present'.format(vname, self.name), + 'pchanges': {}, + 'changes': {}, + 'name': self.name, + 'result': True} + ret = reg.present( + self.name, vname=vname, vdata=vdata, vtype=vtype) + self.assertDictEqual(ret, expected) + def test_present_test_true(self): expected = { 'comment': '', diff --git a/tests/unit/utils/test_win_reg.py b/tests/unit/utils/test_win_reg.py index 728800ad0dcc..d43cdfac0548 100644 --- a/tests/unit/utils/test_win_reg.py +++ b/tests/unit/utils/test_win_reg.py @@ -637,6 +637,10 @@ def test_cast_vdata_reg_dword(self): result = win_reg.cast_vdata(vdata=vdata, vtype='REG_DWORD') self.assertTrue(isinstance(result, six.integer_types)) + vdata = '0000001' + result = win_reg.cast_vdata(vdata=vdata, vtype='REG_DWORD') + self.assertTrue(isinstance(result, six.integer_types)) + def test_cast_vdata_reg_expand_sz(self): ''' Test the cast_vdata function with REG_EXPAND_SZ From 80650c2a8cf21442d54ada887696893e506b4433 Mon Sep 17 00:00:00 2001 From: twangboy Date: Tue, 10 Mar 2020 17:57:37 -0600 Subject: [PATCH 2/2] Make test more explicit --- tests/unit/utils/test_win_reg.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/utils/test_win_reg.py b/tests/unit/utils/test_win_reg.py index d43cdfac0548..f9230886cdfb 100644 --- a/tests/unit/utils/test_win_reg.py +++ b/tests/unit/utils/test_win_reg.py @@ -630,16 +630,17 @@ def test_cast_vdata_reg_dword(self): Should always return integer ''' vdata = 1 + expected = 1 result = win_reg.cast_vdata(vdata=vdata, vtype='REG_DWORD') - self.assertTrue(isinstance(result, six.integer_types)) + self.assertEqual(result, expected) vdata = '1' result = win_reg.cast_vdata(vdata=vdata, vtype='REG_DWORD') - self.assertTrue(isinstance(result, six.integer_types)) + self.assertEqual(result, expected) vdata = '0000001' result = win_reg.cast_vdata(vdata=vdata, vtype='REG_DWORD') - self.assertTrue(isinstance(result, six.integer_types)) + self.assertEqual(result, expected) def test_cast_vdata_reg_expand_sz(self): '''