Panic on size being smaller than default

[hummingly]

I was looking through the source code and found this for the Blake3Hasher.finalize() method.

rust-multihash/src/hasher_impl.rs

Line 124 in f1d20e9

let digest_out = &mut self.digest[..digest_bytes.len().max(S)];

This will always panic if S is less than 32 and the same line of code appears for other hasher implementations too. The fix would be to compute the minimum length and use that to create the byte slices.

let digest = self.state.finalize();
let len = digest.len().min(S);
let digest_bytes = digest.as_bytes()[..len];
let digest_out = &mut self.digest[..len];
digest_out.copy_from_slice(digest_bytes);
digest_out

[vmx]

I don't think it would panic, but I might miss something.

So let's say S is 16 and digest_bytes.len() is 32. Then digest_bytes.len().max(S) would be 32.

So Blake3Hasher.finalize() makes sure that the returned value has at least S bytes, but maybe more. Then later, when the hash is used (that mostly happens in the multihash-derive crate), the result goes through Multihash::wrap(), which will error (not panic) in case the has is too long:

rust-multihash/src/multihash.rs

Lines 102 to 104 in f1d20e9

	if input_digest.len() > S {
	return Err(Error::InvalidSize(input_digest.len() as _));
	}

[hummingly]

My code looks like that where DIGEST_SIZE is 32 right now.

pub struct ContentHasher(Blake3Hasher<DIGEST_SIZE>);

impl ContentHasher {
    pub fn new() -> ContentHasher {
        ContentHasher(Blake3Hasher::default())
    }

    pub fn write(&mut self, input: &[u8]) {
        self.0.update(input);
    }

    pub fn finish(&mut self) -> ContentHash {
        let hash = multihash::Code::Blake3_256.wrap(self.0.finalize()).unwrap();
        hash.resize::<DIGEST_SIZE>().unwrap()
    }

    pub fn reset(&mut self) {
        self.0.reset();
    }
}

However, if I were to set it to something smaller than the default digest size (32), it will panic. The reason is an out of bounds indexing error due the array digest in the Blake3Hasher struct as I explained in the previous comment.

rust-multihash/src/hasher_impl.rs

Lines 100 to 103 in f1d20e9

	pub struct Blake3Hasher<const S: usize> {
	hasher: ::blake3::Hasher,
	digest: [u8; S],
	}

So Blake3Hasher.finalize() makes sure that the returned value has at least S bytes, but maybe more. Then later, when the hash is used (that mostly happens in the multihash-derive crate), the result goes through Multihash::wrap(), which will error (not panic) in case the has is too long:

It is the other way around. The length will always be S or smaller because Blake3Hasher.finalize() returns a slice to the array digest in Blake3Hasher.

Do you consider it a user error to use an S smaller than the default size of the hash?

[mriise]

Blake3Hasher was originally intended to be flexible like this, but some implementation details had kept me away at the time but has since been fixed. see #130
The Code table being generic over Blake Digest sizes is still something to be solved, either through more proc macro magic or new const generic things.

[vmx]

Thanks @mriise for a fix and @hummingly for being patient and explaining things in even more depths. It's funny that I even opened an issue about it and didn't recall.

[mriise]

quick note for transparency, this was a bit more broken than this issue mentions, since the digest returned by blake3 was only 32 bytes, anything bigger would've been filled with 0s

...fun