Hash is unexpectedly None for empty files

[stefan6419846]

Generating hashes for empty files will always return None, which is not documented and different from the usual hashing algorithms as well as contradicting the SPDX standard.

Example:

from commoncode.hash import sha1
from hashlib import sha1 as sha1_hashlib
from tempfile import NamedTemporaryFile

with NamedTemporaryFile() as temporary_file:
    temporary_file.write(b'')
    temporary_file.seek(0)
    print(sha1(location=temporary_file.name))
    print(sha1_hashlib(string=temporary_file.read(), used_for_security=False).hexdigest())

The reason seems to be that

commoncode/src/commoncode/hash.py

Line 38 in 878be61

self.h = msg and hmodule(msg).digest()[: self.digest_size] or None

does not use msg is not None, but basically bool(msg), which is False for empty inputs as well.

Replacing the line with

            self.h = msg is not None and hmodule(msg).digest()[:self.digest_size] or None

(as well as replacing the same pattern in sha1_git_hasher) seems to fix this issue.

[pombredanne]

Stefan: Thanks for the report!
This is intentional because there is a lot of emptiness out there! Returning None for empty files has been a design choice to avoid creating dummy checksums.

FWIW, this predates SPDX and this library is not SPDX specific .... With this said, this has not been documented correctly and should be documented. I will keep this open until we document this alright.

And for the SPDX compatibility, this is handled when creating SPDX documents in ScanCode Toolkit at https://github.com/aboutcode-org/scancode-toolkit/blob/498467c1e5f677edc20e92b12910f16d771ccad3/src/formattedcode/output_spdx.py#L255 where we effectively return a SHA1 for empty files there. (And in the future, other checksums, hardcoded in a similar fashion).

[stefan6419846]

Thanks for the explanation. Then this apparently is just a matter of missing/improper documentation.

[stefan6419846]

I just upgraded to version 32.2.1 and now I get 0 instead of None, although 6bc57ab#diff-2be6ecb882581a72793ebc5c76d40d20c07aa7047e5ebdc1bb1a85e6259b35a1R221-R293 now indicates that this should be None. Compare the SCTK test change as well: https://github.com/aboutcode-org/scancode-toolkit/pull/4149/files#r1983173751