From 54d96284dfad89d7e3d50786de3dadb7d48c222f Mon Sep 17 00:00:00 2001
From: Alexander Grund <alexander.grund@tu-dresden.de>
Date: Fri, 6 Jan 2023 11:23:59 +0100
Subject: [PATCH] Fix the checksum type check

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.
---
 easybuild/framework/easyconfig/types.py | 24 +++++++++++++++++++-----
 test/framework/type_checking.py         | 18 ++++++++++++++++--
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/easybuild/framework/easyconfig/types.py b/easybuild/framework/easyconfig/types.py
index 622479f480..9e0b705964 100644
--- a/easybuild/framework/easyconfig/types.py
+++ b/easybuild/framework/easyconfig/types.py
@@ -608,19 +608,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, SANITY_CHECK_PATHS_ENTRY, STRING_DICT, STRING_OR_TUPLE_LIST,
                    STRING_OR_TUPLE_DICT, 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 = {
diff --git a/test/framework/type_checking.py b/test/framework/type_checking.py
index 83f1ef41ac..9acb727f0e 100644
--- a/test/framework/type_checking.py
+++ b/test/framework/type_checking.py
@@ -221,10 +221,24 @@ def test_check_type_of_param_value_checksums(self):
                 {'foo.txt': sha256_checksum1, 'bar.txt': sha256_checksum2},
                 # 3 alternative checksums for a single file, one match is sufficient
                 (sha256_checksum1, sha256_checksum2, sha256_checksum3),
-            ]
+                # two alternative checksums for a single file (not to be confused by checksum-type & -value tuple)
+                (sha256_checksum1, md5_checksum),
+                # three alternative checksums for a single file of different types
+                (sha256_checksum1, ('md5', md5_checksum), {'foo.txt': sha256_checksum1}),
+                # alternative checksums in dicts are also allowed
+                {'foo.txt': (sha256_checksum2, sha256_checksum3), 'bar.txt': (sha256_checksum1, md5_checksum)},
+                # Same but with lists -> all must match for each file
+                {'foo.txt': [sha256_checksum2, sha256_checksum3], 'bar.txt': [sha256_checksum1, md5_checksum]},
+            ],
+            # None is allowed, meaning skip the checksum
+            [
+                None,
+                # Also in mappings
+                {'foo.txt': sha256_checksum1, 'bar.txt': None},
+            ],
         ]
         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."""