Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Validate dimensionality #43

Merged
merged 3 commits into from
Feb 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added resources/fuzz_artifacts/crash-1.nii
Binary file not shown.
14 changes: 12 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//! Types for error handling go here.

use std::io::Error as IOError;
use typedef::NiftiType;

Expand All @@ -13,6 +12,17 @@ quick_error! {
InvalidFormat {
description("Invalid NIfTI-1 file")
}
/// The field `dim` is in an invalid state, as a consequence of
/// `dim[0]` or one of the elements in `1..dim[0] + 1` not being
/// positive.
InconsistentDim(index: u8, value: u16) {
description("Inconsistent dim in file header")
display("Inconsistent value `{}` in header field dim[{}] ({})", value, index, match index {
0 if *value > 7 => "must not be higher than 7",
_ => "must be positive"
})
}

/// Attempted to read volume outside boundaries.
OutOfBounds(coords: Vec<u16>) {
description("Out of bounds access to volume")
Expand Down Expand Up @@ -59,7 +69,7 @@ quick_error! {
/// Header contains a code which is not valid for the given attribute
InvalidCode(typename: &'static str, code: i16) {
description("invalid code")
display("invalid code `{}` for {}", code, typename)
display("invalid code `{}` for header field {}", code, typename)
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,37 @@ impl NiftiHeader {
parse_header_1(input)
}

/// Retrieve and validate the dimensions of the volume. Unlike how NIfTI-1
/// stores dimensions, the returned slice does not include `dim[0]` and is
/// clipped to the effective number of dimensions.
///
/// # Error
///
/// `NiftiError::InconsistentDim` if `dim[0]` does not represent a valid
/// dimensionality, or any of the real dimensions are zero.
pub fn dim(&self) -> Result<&[u16]> {
let ndim = self.dimensionality()?;
let o = &self.dim[1..ndim + 1];
if let Some(i) = o.into_iter().position(|&x| x == 0) {
return Err(NiftiError::InconsistentDim(i as u8, self.dim[i]));
}
Ok(o)
}

/// Retrieve and validate the number of dimensions of the volume. This is
/// `dim[0]` after the necessary byte order conversions.
///
/// # Error
///
/// `NiftiError::` if `dim[0]` does not represent a valid dimensionality
/// (it must be positive and not higher than 7).
pub fn dimensionality(&self) -> Result <usize> {
if self.dim[0] == 0 || self.dim[0] > 7 {
return Err(NiftiError::InconsistentDim(0, self.dim[0]));
}
Ok(usize::from(self.dim[0]))
}

/// Get the data type as a validated enum.
pub fn data_type(&self) -> Result<NiftiType> {
FromPrimitive::from_i16(self.datatype)
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
//!
#![deny(missing_debug_implementations)]
#![warn(missing_docs, unused_extern_crates, trivial_casts, unused_results)]
#![recursion_limit = "128"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please tell me why we need a recursion limit? I don't remember any use of recursion in this lib.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the compiler's recursive operation limit. Recursion is used internally by the quick_error! macro. Once a significant number of variants are declared, which is the case here, we need to push this value a bit higher.


#[macro_use] extern crate quick_error;
#[macro_use] extern crate num_derive;
Expand Down
9 changes: 4 additions & 5 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use std::io::{Read, Seek};
use std::mem;
use std::path::{Path, PathBuf};
use byteordered::Endian;

use safe_transmute::{guarded_transmute_pod_vec_permissive, PodTransmutable};

use error::Result;
use NiftiHeader;

/// A trait that is both Read and Seek.
Expand Down Expand Up @@ -60,13 +60,12 @@ where
}
}

pub fn nb_bytes_for_data(header: &NiftiHeader) -> usize {
let ndims = header.dim[0];
let resolution: usize = header.dim[1..(ndims + 1) as usize]
pub fn nb_bytes_for_data(header: &NiftiHeader) -> Result<usize> {
let resolution: usize = header.dim()?
.iter()
.map(|d| *d as usize)
.product();
resolution * header.bitpix as usize / 8
Ok(resolution * header.bitpix as usize / 8)
}

#[cfg(feature = "ndarray_volumes")]
Expand Down
6 changes: 3 additions & 3 deletions src/volume/inmem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl InMemNiftiVolume {
/// Build an InMemNiftiVolume from a header and a buffer. The buffer length and the dimensions
/// declared in the header are expected to fit.
pub fn from_raw_data(header: &NiftiHeader, raw_data: Vec<u8>) -> Result<Self> {
if nb_bytes_for_data(header) != raw_data.len() {
if nb_bytes_for_data(header)? != raw_data.len() {
return Err(NiftiError::IncompatibleLength);
}

Expand All @@ -65,7 +65,7 @@ impl InMemNiftiVolume {
/// following bytes represent the first voxels of the volume (and not part of the
/// extensions).
pub fn from_stream<R: Read>(mut source: R, header: &NiftiHeader) -> Result<Self> {
let mut raw_data = vec![0u8; nb_bytes_for_data(header)];
let mut raw_data = vec![0u8; nb_bytes_for_data(header)?];
source.read_exact(&mut raw_data)?;

let datatype = header.data_type()?;
Expand Down Expand Up @@ -323,7 +323,7 @@ impl<'a> NiftiVolume for &'a InMemNiftiVolume {

impl NiftiVolume for InMemNiftiVolume {
fn dim(&self) -> &[u16] {
&self.dim[1..(self.dim[0] + 1) as usize]
&self.dim[1..self.dimensionality() + 1]
}

fn dimensionality(&self) -> usize {
Expand Down
6 changes: 6 additions & 0 deletions tests/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,9 @@ fn f32_nii_gz() {
assert_eq!(volume.get_f32(&[5, 0, 4]).unwrap(), 0.4);
assert_eq!(volume.get_f32(&[0, 8, 5]).unwrap(), 0.8);
}

#[test]
fn bad_file_1() {
let _ = InMemNiftiObject::from_file("resources/fuzz_artifacts/crash-1.nii");
// must not panic
}