search

containers / ocidir-rs

Low level Rust library for working with OCI (opencontainers) directories
Apache License 2.0
6 stars 3 forks source link

Fix the behavior of complete_verified_as function #18

Closed ariel-miculas closed 1 month ago

ariel-miculas commented 1 month ago

complete_verified_as finishes writing the blob, verifying its digest and size against the expected descriptor, and then writing the contents to a file with a completely different sha256 digest. The reason is that both complete_verified_as and complete call self.hash.finish(), but this function can only be called once, because after the first call it transitions into the Finalized state [1]. The second time it gets called it realizes it's in the Finalized state and then it calls self.init, resetting the hasher. This is a bad API design.

Fix this by only calling self.hash.finish() once and then passing the result to a new internal function complete_as, which does the same thing as complete but avoids calling self.hash.finish() again.

Add a test to ensure complete_verified_as behaves as expected.

[1] https://docs.rs/openssl/0.10.66/src/openssl/hash.rs.html#295-297

ariel-miculas commented 1 month ago

I think finish should just take self as the parameter and consume tha hasher, that way you can't misuse the API.

cgwalters commented 1 month ago

I think finish should just take self as the parameter and consume tha hasher, that way you can't misuse the API.

Yeah, I think it's just messy to do since we'd have to make the hash member an Option or so? Probably a simpler change on the openssl side would be to argue to add a .finish_once() API that panic'd instead of doing a silent reset.

Also related to this PR I did https://github.com/containers/ocidir-rs/pull/19