Repository: microsoft/electionguard-python

Issue:
hashing doesn't correctly handle some corner cases
## Issue

**Description**
In experimenting with my "educational" version of ElectionGuard, where I accumulated the whole formula of every operation on ElementModP and Q, I discovered a few misfeatures in the hash function.

- If the value being hashed is the integer 0, that's treated by Python as "false" and is hashed as "null"
- There were some cases where lists weren't being hashed recursively, but were instead being converted to a string and then hashed as a string

None of these impact correctness, since the hash still properly depends on the value in question, but they do impact portability, in the sense that another programming language might properly implement the same hash function with identical behavior. 

The patch is pretty simple, only a few changes in `hash.py`

Discussion:

Patch:
diff --git a/src/electionguard/hash.py b/src/electionguard/hash.py
index f1fce1c..0473ebc 100644
--- a/src/electionguard/hash.py
+++ b/src/electionguard/hash.py
@@ -1,7 +1,14 @@
 from abc import abstractmethod
 from collections.abc import Sequence
 from hashlib import sha256
-from typing import Union, Protocol, runtime_checkable, Sequence as TypedSequence
+from typing import (
+    Iterable,
+    List,
+    Union,
+    Protocol,
+    runtime_checkable,
+    Sequence as TypedSequence,
+)
 
 from .group import (
     ElementModPOrQ,
@@ -69,25 +76,27 @@ def hash_elems(*a: CRYPTO_HASHABLE_ALL) -> ElementModQ:
         # that's a bit Python-specific, and we'd rather make it easier for other languages
         # to exactly match this hash function.
 
-        if not x:
-            # This case captures empty lists and None, nicely guaranteeing that we don't
-            # need to do a recursive call if the list is empty. So we need a string to
-            # feed in for both of these cases. "None" would be a Python-specific thing,
-            # so we'll go with the more JSON-ish "null".
-            hash_me = "null"
-
-        elif isinstance(x, (ElementModP, ElementModQ)):
+        if isinstance(x, (ElementModP, ElementModQ)):
             hash_me = x.to_hex()
         elif isinstance(x, CryptoHashable):
             hash_me = x.crypto_hash().to_hex()
         elif isinstance(x, str):
-            # strings are iterable, so it's important to handle them before the following check
+            # strings are iterable, so it's important to handle them before list-like types
             hash_me = x
-        elif isinstance(x, Sequence):
+        elif isinstance(x, int):
+            hash_me = str(x)
+        elif not x:
+            # This case captures empty lists and None, nicely guaranteeing that we don't
+            # need to do a recursive call if the list is empty. So we need a string to
+            # feed in for both of these cases. "None" would be a Python-specific thing,
+            # so we'll go with the more JSON-ish "null".
+            hash_me = "null"
+        elif isinstance(x, (Sequence, List, Iterable)):
             # The simplest way to deal with lists, tuples, and such are to crunch them recursively.
             hash_me = hash_elems(*x).to_hex()
         else:
             hash_me = str(x)
+
         h.update((hash_me + "|").encode("utf-8"))
 
     # We don't need the checked version of int_to_q, because the


Test patch:
diff --git a/tests/property/test_hash.py b/tests/property/test_hash.py
index 19f0df7..d42a8e0 100644
--- a/tests/property/test_hash.py
+++ b/tests/property/test_hash.py
@@ -28,8 +28,8 @@ class TestHash(unittest.TestCase):
         if ha != hb:
             self.assertNotEqual(a, b)
 
-    def test_hash_value_for_zero_string_different_from_zero_number(self):
-        self.assertNotEqual(hash_elems(0), hash_elems("0"))
+    def test_hash_for_zero_number_is_zero_string(self):
+        self.assertEqual(hash_elems(0), hash_elems("0"))
 
     def test_hash_for_non_zero_number_string_same_as_explicit_number(self):
         self.assertEqual(hash_elems(1), hash_elems("1"))
@@ -40,27 +40,24 @@ class TestHash(unittest.TestCase):
             hash_elems("welcome to electionguard"),
         )
 
-    def test_hash_for_empty_string_same_as_null_string(self):
-        self.assertEqual(hash_elems(""), hash_elems("null"))
-
     def test_hash_for_none_same_as_null_string(self):
         self.assertEqual(hash_elems(None), hash_elems("null"))
 
-    def test_hash_for_none_same_as_explicit_zero_number(self):
-        self.assertEqual(hash_elems(None), hash_elems(0))
-
     def test_hash_of_save_values_in_list_are_same_hash(self):
         self.assertEqual(hash_elems(["0", "0"]), hash_elems(["0", "0"]))
 
-    def test_hash_empty_list_same_as_hash_null_string(self):
+    def test_hash_null_equivalents(self):
         null_list: Optional[List[str]] = None
         empty_list: List[str] = []
 
         self.assertEqual(hash_elems(null_list), hash_elems(empty_list))
         self.assertEqual(hash_elems(empty_list), hash_elems(None))
-        self.assertEqual(hash_elems(empty_list), hash_elems(""))
         self.assertEqual(hash_elems(empty_list), hash_elems("null"))
 
+    def test_hash_not_null_equivalents(self):
+        self.assertNotEqual(hash_elems(None), hash_elems(""))
+        self.assertNotEqual(hash_elems(None), hash_elems(0))
+
     def test_hash_value_from_nested_list_and_result_of_hashed_list_by_taking_the_hex(
         self,
     ):