diff --git a/rust-bindings/rust/src/checksum.rs b/rust-bindings/rust/src/checksum.rs index eee7e5c0..86087bc3 100644 --- a/rust-bindings/rust/src/checksum.rs +++ b/rust-bindings/rust/src/checksum.rs @@ -1,16 +1,17 @@ -use glib::translate::{from_glib_full, FromGlibPtrFull, Stash, ToGlibPtr}; +use glib::translate::{from_glib_full, FromGlibPtrFull}; use glib::GString; use glib_sys::{g_free, g_malloc, g_malloc0, gpointer}; use std::fmt; use std::ptr::copy_nonoverlapping; -const BYTES_BUF_SIZE: usize = ostree_sys::OSTREE_SHA256_DIGEST_LEN as usize; -const B64_BUF_SIZE: usize = 44; +const BYTES_LEN: usize = ostree_sys::OSTREE_SHA256_DIGEST_LEN as usize; +const HEX_LEN: usize = ostree_sys::OSTREE_SHA256_STRING_LEN as usize; +const B64_LEN: usize = 43; /// A binary SHA256 checksum. #[derive(Debug)] pub struct Checksum { - bytes: *mut [u8; BYTES_BUF_SIZE], + bytes: *mut [u8; BYTES_LEN], } impl Checksum { @@ -21,7 +22,7 @@ impl Checksum { /// `g_free` (this is e.g. the case if the memory was allocated with `g_malloc`). The value /// takes ownership of the memory, i.e. the memory is freed when the value is dropped. The /// memory must not be freed by other code. - unsafe fn new(bytes: *mut [u8; BYTES_BUF_SIZE]) -> Checksum { + unsafe fn new(bytes: *mut [u8; BYTES_LEN]) -> Checksum { assert!(!bytes.is_null()); Checksum { bytes } } @@ -30,26 +31,29 @@ impl Checksum { /// /// Unfortunately, the underlying libostree function has no way to report parsing errors. If the /// string is not a valid SHA256 string, the program will abort! - // TODO: implement by hand to avoid stupid assertions? pub fn from_hex(checksum: &str) -> Checksum { + // TODO: implement by hand to avoid stupid assertions? + assert!(checksum.len() == HEX_LEN); unsafe { + // We know checksum is at least as long as needed, trailing NUL is unnecessary. from_glib_full(ostree_sys::ostree_checksum_to_bytes( - checksum.to_glib_none().0, + checksum.as_ptr() as *const i8 )) } } /// Create a `Checksum` from a base64-encoded String. /// - /// Data that is too long or too short will be silently accepted. For example, if you pass in - /// the empty string, the resulting checksum value will be all 0's. - /// // TODO: implement by hand for better error reporting? + /// Invalid base64 characters will not be reported, but will cause unknown output instead, most + /// likely 0. pub fn from_base64(b64_checksum: &str) -> Checksum { - let b64_checksum: Stash<*mut libc::c_char, _> = b64_checksum.to_glib_none(); + // TODO: implement by hand for better error reporting? + assert!(b64_checksum.len() == B64_LEN); unsafe { - let buf = g_malloc0(BYTES_BUF_SIZE) as *mut [u8; BYTES_BUF_SIZE]; + let buf = g_malloc0(BYTES_LEN) as *mut [u8; BYTES_LEN]; + // We know b64_checksum is at least as long as needed, trailing NUL is unnecessary. ostree_sys::ostree_checksum_b64_inplace_to_bytes( - b64_checksum.0 as *const [i8; 32], + b64_checksum.as_ptr() as *const [i8; 32], buf as *mut u8, ); from_glib_full(buf) @@ -58,20 +62,20 @@ impl Checksum { /// Convert checksum to hex-encoded string. pub fn to_hex(&self) -> GString { + // This one returns a NUL-terminated string. unsafe { from_glib_full(ostree_sys::ostree_checksum_from_bytes(self.bytes)) } } /// Convert checksum to base64. pub fn to_base64(&self) -> String { - let mut buf: Vec = Vec::with_capacity(B64_BUF_SIZE); + let mut buf: Vec = Vec::with_capacity(B64_LEN + 1); unsafe { ostree_sys::ostree_checksum_b64_inplace_from_bytes( self.bytes, buf.as_mut_ptr() as *mut i8, ); - let len = libc::strlen(buf.as_mut_ptr() as *const i8); - // Assumption: (len + 1) valid bytes are in the buffer. - buf.set_len(len); + // Assumption: 43 valid bytes are in the buffer. + buf.set_len(B64_LEN); // Assumption: all characters are ASCII, ergo valid UTF-8. String::from_utf8_unchecked(buf) } @@ -89,9 +93,9 @@ impl Drop for Checksum { impl Clone for Checksum { fn clone(&self) -> Self { unsafe { - let cloned = g_malloc(BYTES_BUF_SIZE) as *mut [u8; BYTES_BUF_SIZE]; + let cloned = g_malloc(BYTES_LEN) as *mut [u8; BYTES_LEN]; // copy one array of 32 elements - copy_nonoverlapping::<[u8; BYTES_BUF_SIZE]>(self.bytes, cloned, 1); + copy_nonoverlapping::<[u8; BYTES_LEN]>(self.bytes, cloned, 1); Checksum::new(cloned) } } @@ -117,21 +121,21 @@ impl fmt::Display for Checksum { } } -impl FromGlibPtrFull<*mut [u8; BYTES_BUF_SIZE]> for Checksum { - unsafe fn from_glib_full(ptr: *mut [u8; BYTES_BUF_SIZE]) -> Self { +impl FromGlibPtrFull<*mut [u8; BYTES_LEN]> for Checksum { + unsafe fn from_glib_full(ptr: *mut [u8; BYTES_LEN]) -> Self { Checksum::new(ptr) } } -impl FromGlibPtrFull<*mut [*mut u8; BYTES_BUF_SIZE]> for Checksum { - unsafe fn from_glib_full(ptr: *mut [*mut u8; BYTES_BUF_SIZE]) -> Self { - Checksum::new(ptr as *mut u8 as *mut [u8; BYTES_BUF_SIZE]) +impl FromGlibPtrFull<*mut [*mut u8; BYTES_LEN]> for Checksum { + unsafe fn from_glib_full(ptr: *mut [*mut u8; BYTES_LEN]) -> Self { + Checksum::new(ptr as *mut u8 as *mut [u8; BYTES_LEN]) } } impl FromGlibPtrFull<*mut u8> for Checksum { unsafe fn from_glib_full(ptr: *mut u8) -> Self { - Checksum::new(ptr as *mut [u8; BYTES_BUF_SIZE]) + Checksum::new(ptr as *mut [u8; BYTES_LEN]) } } @@ -146,9 +150,9 @@ mod tests { #[test] fn should_create_checksum_from_bytes() { - let bytes = unsafe { g_malloc0(BYTES_BUF_SIZE) } as *mut u8; + let bytes = unsafe { g_malloc0(BYTES_LEN) } as *mut u8; let checksum: Checksum = unsafe { from_glib_full(bytes) }; - assert_eq!(checksum.to_string(), "00".repeat(BYTES_BUF_SIZE)); + assert_eq!(checksum.to_string(), "00".repeat(BYTES_LEN)); } #[test] @@ -157,6 +161,12 @@ mod tests { assert_eq!(csum.to_string(), CHECKSUM_STRING); } + #[test] + #[should_panic] + fn should_panic_for_too_short_hex_string() { + Checksum::from_hex(&"FF".repeat(31)); + } + #[test] fn should_convert_checksum_to_base64() { let csum = Checksum::from_hex(CHECKSUM_STRING); @@ -171,9 +181,15 @@ mod tests { } #[test] - fn should_be_zeros_for_empty_base64_string() { - let csum = Checksum::from_base64(""); - assert_eq!(csum.to_string(), "00".repeat(BYTES_BUF_SIZE)); + #[should_panic] + fn should_panic_for_too_short_b64_string() { + Checksum::from_base64("abcdefghi"); + } + + #[test] + fn should_be_all_zeroes_for_invalid_base64_string() { + let csum = Checksum::from_base64(&"\n".repeat(43)); + assert_eq!(csum.to_string(), "00".repeat(32)); } #[test] diff --git a/rust-bindings/rust/tests/functions/checksum_file_at.rs b/rust-bindings/rust/tests/functions/checksum_file_at.rs deleted file mode 100644 index 7db75921..00000000 --- a/rust-bindings/rust/tests/functions/checksum_file_at.rs +++ /dev/null @@ -1,27 +0,0 @@ -use gio::NONE_CANCELLABLE; -use ostree::{checksum_file_at, ChecksumFlags, ObjectType, RepoMode}; -use std::path::PathBuf; -use util::TestRepo; - -#[test] -fn should_checksum_file_at() { - let repo = TestRepo::new_with_mode(RepoMode::BareUser); - repo.test_commit("test"); - - let result = checksum_file_at( - repo.repo.get_dfd(), - &PathBuf::from( - "objects/89/f84ca9854a80e85b583e46a115ba4985254437027bad34f0b113219323d3f8.file", - ), - None, - ObjectType::File, - ChecksumFlags::IGNORE_XATTRS, - NONE_CANCELLABLE, - ) - .expect("checksum file at"); - - assert_eq!( - result.as_str(), - "89f84ca9854a80e85b583e46a115ba4985254437027bad34f0b113219323d3f8", - ); -} diff --git a/rust-bindings/rust/tests/functions/mod.rs b/rust-bindings/rust/tests/functions/mod.rs index 3656d95d..465418b8 100644 --- a/rust-bindings/rust/tests/functions/mod.rs +++ b/rust-bindings/rust/tests/functions/mod.rs @@ -1,29 +1,7 @@ use gio::NONE_CANCELLABLE; -use glib::prelude::*; -use ostree::prelude::*; -use ostree::{checksum_file, checksum_file_from_input, ObjectType, RepoFile}; +use ostree::{checksum_file_from_input, ObjectType}; use util::TestRepo; -#[cfg(feature = "v2017_13")] -mod checksum_file_at; - -#[test] -fn should_checksum_file() { - let repo = TestRepo::new(); - repo.test_commit("test"); - - let file = repo - .repo - .read_commit("test", NONE_CANCELLABLE) - .expect("read commit") - .0 - .downcast::() - .expect("downcast"); - let result = checksum_file(&file, ObjectType::File, NONE_CANCELLABLE).expect("checksum file"); - - assert_eq!(file.get_checksum().unwrap(), result.to_string()); -} - #[test] fn should_checksum_file_from_input() { let repo = TestRepo::new(); @@ -49,6 +27,6 @@ fn should_checksum_file_from_input() { NONE_CANCELLABLE, ) .expect("checksum file from input"); - assert_eq!(obj.checksum(), result.to_string()); + assert_eq!(result.to_string(), obj.checksum()); } }