Skip to content

Commit

Permalink
Fix the checksum type check
Browse files Browse the repository at this point in the history
The `None` case was missed and due to the unrestricted `tuple` elem_type
it may return valid for actually invalid entries.
So restrict that beeing overly cautious so it may wrongly return invalid.
But in that case the conversion function will be called which can do more
elaborate verification.

Add test checking for None in checksums.
  • Loading branch information
Flamefire committed Jan 6, 2023
1 parent 8ba299f commit 011f80c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 15 deletions.
24 changes: 19 additions & 5 deletions easybuild/framework/easyconfig/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,19 +565,33 @@ def ensure_iterable_license_specs(specs):
}))
# checksums is a list of checksums, one entry per file (source/patch)
# each entry can be:
# None
# a single checksum value (string)
# a single checksum value of a specified type (2-tuple, 1st element is checksum type, 2nd element is checksum)
# a list of checksums (of different types, perhaps different formats), which should *all* be valid
# a dictionary with a mapping from filename to checksum value
CHECKSUM_LIST = (list, as_hashable({'elem_types': [str, tuple, STRING_DICT]}))
CHECKSUMS = (list, as_hashable({'elem_types': [str, tuple, STRING_DICT, CHECKSUM_LIST]}))
# a tuple of checksums (of different types, perhaps different formats), where one should be valid
# a dictionary with a mapping from filename to checksum (None, value, type&value, alternatives)

CHECKABLE_TYPES = [CHECKSUM_LIST, CHECKSUMS, DEPENDENCIES, DEPENDENCY_DICT, LIST_OF_STRINGS,
# Type & value, value may be an int for type "size"
# This is a bit too permissive as it allows the first element to be an int and doesn't restrict the number of elements
CHECKSUM_TUPLE = (tuple, as_hashable({'elem_types': [str, int]}))
CHECKSUM_DICT = (dict, as_hashable(
{
'elem_types': [type(None), str, CHECKSUM_TUPLE],
'key_types': [str],
}
))
CHECKSUM_LIST = (list, as_hashable({'elem_types': [str, CHECKSUM_TUPLE, CHECKSUM_DICT]}))

CHECKSUMS = (list, as_hashable({'elem_types': [type(None), str, CHECKSUM_LIST, CHECKSUM_TUPLE, CHECKSUM_DICT]}))

CHECKABLE_TYPES = [CHECKSUM_DICT, CHECKSUM_LIST, CHECKSUM_TUPLE, CHECKSUMS,
DEPENDENCIES, DEPENDENCY_DICT, LIST_OF_STRINGS,
SANITY_CHECK_PATHS_DICT, STRING_DICT, STRING_OR_TUPLE_LIST, STRING_OR_TUPLE_OR_DICT_LIST,
TOOLCHAIN_DICT, TUPLE_OF_STRINGS]

# easy types, that can be verified with isinstance
EASY_TYPES = [string_type, bool, dict, int, list, str, tuple]
EASY_TYPES = [string_type, bool, dict, int, list, str, tuple, type(None)]

# type checking is skipped for easyconfig parameters names not listed in PARAMETER_TYPES
PARAMETER_TYPES = {
Expand Down
27 changes: 17 additions & 10 deletions test/framework/type_checking.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,6 @@ def test_check_type_of_param_value_checksums(self):
[('md5', md5_checksum), ('sha256', sha256_checksum1)],
# checksum as dict (file to checksum mapping)
[{'foo.txt': sha256_checksum1, 'bar.txt': sha256_checksum2}],
# list of checksums for a single file
[[md5_checksum]],
[[sha256_checksum1, sha256_checksum2, sha256_checksum3]],
# in the mix (3 files, each a different kind of checksum spec)...
[
sha256_checksum1,
Expand All @@ -211,20 +208,30 @@ def test_check_type_of_param_value_checksums(self):
],
# each item can be a list of checksums for a single file, which can be of different types...
[
# two checksums for a single file, *both* should match
[sha256_checksum1, md5_checksum],
# three checksums for a single file, *all* should match
[sha256_checksum1, ('md5', md5_checksum), {'foo.txt': sha256_checksum1}],
# two alternative checksums for a single file
(sha256_checksum1, md5_checksum),
# three alternative checksums for a single file
(sha256_checksum1, ('md5', md5_checksum), {'foo.txt': sha256_checksum1}),
# single checksum for a single file
sha256_checksum1,
# filename-to-checksum mapping
{'foo.txt': sha256_checksum1, 'bar.txt': sha256_checksum2},
# 3 alternative checksums for a single file, one match is sufficient
# 3 alternative checksums for a single file
(sha256_checksum1, sha256_checksum2, sha256_checksum3),
]
],
[
# Normal checksum
sha256_checksum1,
# None is allowed to skip checksum checking
None,
# Also in mappings
{'foo.txt': sha256_checksum1, 'bar.txt': None},
# alternative checksums are also allowed
{'foo.txt': (sha256_checksum2, sha256_checksum3), 'bar.txt': (sha256_checksum1, md5_checksum)},
],
]
for inp in inputs:
self.assertEqual(check_type_of_param_value('checksums', inp), (True, inp))
self.assertTrue(check_type_of_param_value('checksums', inp), 'Failed for ' + str(inp))

def test_check_type_of_param_value_patches(self):
"""Test check_type_of_param_value function for patches."""
Expand Down

0 comments on commit 011f80c

Please sign in to comment.