Skip to content

Hashlib paradigm inconsistent with Bagit spec, but also causing runtime errors #158

Open

Description

@ross-spencer

Bagit-python is currently using a generic pattern to import the list of checksums available to the tool to be used:

bagit-python/bagit.py

Lines 125 to 131 in bff20d2

try:
CHECKSUM_ALGOS = hashlib.algorithms_guaranteed
except AttributeError:
# FIXME: remove when we drop Python 2 (https://github.com/LibraryOfCongress/bagit-python/issues/102)
# Python 2.7.0-2.7.8
CHECKSUM_ALGOS = set(hashlib.algorithms)
DEFAULT_CHECKSUMS = ["sha256", "sha512"]

This creates a help-command with the following in Python 3.9.6.

--sha3_224 
--sha3_256 
--blake2s 
--sha3_384 
--sha384 
--md5 
--sha224 
--shake_256 
--sha256 
--shake_128 
--sha3_512 
--blake2b 
--sha512 
--sha1 

@pwinckles brings up something similar here: #156 I am not clear what is invalid about those names - that being said:

The spec states:

To avoid future ambiguity, the checksum algorithm SHOULD be registered in IANA's "Named Information Hash Algorithm Registry"

The most obvious example of algorithm's not in the registry at time of writing are those from the shake family. Further, there is ambiguity in the Blake algorithms presented by Python's hashlib.

Errors caused by shake

Additionally, the shake algorithms require a length argument not handled by the code: https://docs.python.org/3/library/hashlib.html#shake-variable-length-digests

Causing the following when selected.

Traceback (most recent call last):
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 241, in make_bag
    total_bytes, total_files = make_manifests(
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1254, in make_manifests
    checksums = [manifest_line_generator(i) for i in _walk(data_dir)]
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1254, in <listcomp>
    checksums = [manifest_line_generator(i) for i in _walk(data_dir)]
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1412, in generate_manifest_lines
    results = [
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1413, in <listcomp>
    (alg, hasher.hexdigest(), decoded_filename, total_bytes)
TypeError: hexdigest() missing required argument 'length' (pos 1)
2022-05-13 10:38:36,341 - ERROR - Failed to create bag in govdoc-gifs: hexdigest() missing required argument 'length' (pos 1)
Traceback (most recent call last):
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1599, in main
    make_bag(
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 241, in make_bag
    total_bytes, total_files = make_manifests(
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1254, in make_manifests
    checksums = [manifest_line_generator(i) for i in _walk(data_dir)]
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1254, in <listcomp>
    checksums = [manifest_line_generator(i) for i in _walk(data_dir)]
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1412, in generate_manifest_lines
    results = [
  File "C:\temp\testing\bags\venv\lib\site-packages\bagit.py", line 1413, in <listcomp>
    (alg, hasher.hexdigest(), decoded_filename, total_bytes)
TypeError: hexdigest() missing required argument 'length' (pos 1)

While it's nice there's some extensibility for free, perhaps, as this issue can come up again in future, the algorithms can be provided from a known list compatible with IANA today, and Python, and perhaps cross-referenced against, checksums_guaranteed instead?

Activity

  1. acdha commented on May 13, 2022

    @acdha
    Member

    If memory serves, that bit of code predates the work on the RFC which strengthened the guidance around things like this. If you'd like to send a pull-request I think it would definitely make sense to trim that down to be only the list of non-draft algorithms from https://www.iana.org/assignments/named-information/named-information.xhtml.

  2. ross-spencer commented on May 16, 2022

    @ross-spencer
    Author

    That makes sense @acdha. I'll have a look and see if I can submit something. Folks reading this list might want to avail themselves on whether they're going to be affected by any of the changes, i.e. checking the IANA link above for any checksums removed they may be using.

  3. kieranjol commented on Aug 7, 2025

    @kieranjol
    Contributor

    Seems like using the IANA list would remove algorithms like MD5?

  4. tehaef commented on Jul 23, 2026

    @tehaef

    @pwinckles brings up something similar here: #156 I am not clear what is invalid about those names - that being said:

    I think, these names are invalid because they contain underscores. For example, with the --sha3_224 parameter, bagit.py creates a file manifest-sha3_224.txt. However, as per the specification, only alphanumeric characters are allowed, thus this filename, strictly speaking, violates the specification, if i understand and interpret this all correctly. As for the hashes included on the recommended IANA list, the allowed hash names filenames should be:

    sha256
    sha256128
    sha256120
    sha25696
    sha25664
    sha25632
    sha384
    sha512
    sha3224
    sha3256
    sha3384
    sha3512
    blake2s256
    blake2b256
    blake2b512
    k12256
    k12512
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions