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 https://github.com/aboutcode-org/commoncode/blob/878be6140deac30e2b95fb0fad9eb8feca015fc8/src/commoncode/hash.py#L38 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

scancode-toolkit/src/formattedcode/output_spdx.py

Line 255 in 498467c

if file_data.get('file_type') == 'empty':

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 aboutcode-org/commoncode@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

[AyanSinhaMahapatra]

@stefan6419846 This returns None allright now, as we reverted the changes which was wrong, see:

• Fix empty file checksum commoncode#82
• 70157d2

[AyanSinhaMahapatra]

There is also some docstrings for multi_checksums which is the main API function which is also what we use on the SCTK side.

We need some docs too, but this should probably on the SCTK docs, so moving this there.

[armijnhemel]

Isn't this easy to fix using a lookup table that maps "empty" to a hash?

So if the file is empty you would return SHA256 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 and so on.

[stefan6419846]

This is what SCTK is already doing when doing the SPDX export. In my case, I am mostly using lower-level public APIs like from commoncode directly to calculate hashes as the automated scans from SCTK require further reviews and changes before being suitable. For this reason, it is unexpected to call commoncode.hash.sha1 and suddenly get 0 or None for a hash which should be a hexadecimal string of a fixed length. I have added a workaround in my own code as well, but think that this is something which should be documented clearly.

[AyanSinhaMahapatra]

@stefan6419846 Thanks, this is correct.
Btw, you should use commoncode.hash.multi_checksums instead of commoncode.hash.sha1 since this does chunking to avoid exhausting memory for larger files, and incase you want multiple checksums this avoids reading the file many times. Also this function has a documented behaviour since this is API. But we should also document the individual functions here just in case.

[stefan6419846]

since this does chunking to avoid exhausting memory for larger files, and incase you want multiple checksums this avoids reading the file many times

While I understand the part with multiple checksums, the other statement is incorrect. The single checksums are using checksum, which calls binary_chunks to yield chunks and checksum_from_chunks to calculate the checksum in a chunk-based manner. This is identical to what multi_checksums does internally, except caring about different hashers.

[AyanSinhaMahapatra]

The single checksums are using checksum, which calls binary_chunks to yield chunks and checksum_from_chunks to calculate the checksum in a chunk-based manner.

That's actually correct now that I see it, I forgot the individual functions also come under the chunking refactor, my bad.

[chinyeungli]

@AyanSinhaMahapatra / @pombredanne
I am not sure what need to be done here in terms of documentation in RTD.
I don't see any 'None' value in the scan output (json format)