Merge pull request #567 from Enet4/imp/parser/odd-length-options

[parser] Add configurable strategy for handling odd length data elements

object/fuzz/fuzz_targets/open_file.rs

@@ -1,18 +1,29 @@
 #![no_main]
-use libfuzzer_sys::fuzz_target;
+use libfuzzer_sys::{fuzz_target, Corpus};
 use std::error::Error;
 
-fuzz_target!(|data: &[u8]| {
-    let _ = fuzz(data);
+fuzz_target!(|data: &[u8]| -> Corpus {
+    match fuzz(data) {
+        Ok(_) => Corpus::Keep,
+        Err(_) => Corpus::Reject,
+    }
 });
 
 fn fuzz(data: &[u8]) -> Result<(), Box<dyn Error>> {
     // deserialize random bytes
-    let obj = dicom_object::from_reader(data)?;
+    let mut obj = dicom_object::OpenFileOptions::new()
+        .read_preamble(dicom_object::file::ReadPreamble::Auto)
+        .odd_length_strategy(dicom_object::file::OddLengthStrategy::Fail)
+        .from_reader(data)?;
 
+    // remove group length elements
+    for g in 0..=0x07FF {
+        obj.remove_element(dicom_object::Tag(g, 0x0000));
+    }
     // serialize object back to bytes
     let mut bytes = Vec::new();
-    obj.write_all(&mut bytes)?;
+    obj.write_all(&mut bytes)
+        .expect("writing DICOM file should always be successful");
 
     // deserialize back to object
     let obj2 = dicom_object::from_reader(bytes.as_slice())

object/src/file.rs

@@ -3,6 +3,9 @@ use dicom_dictionary_std::StandardDataDictionary;
 use dicom_encoding::transfer_syntax::TransferSyntaxIndex;
 use dicom_transfer_syntax_registry::TransferSyntaxRegistry;
 
+// re-export from dicom_parser
+pub use dicom_parser::dataset::read::OddLengthStrategy;
+
 use crate::{DefaultDicomObject, ReadError};
 use std::io::Read;
 use std::path::Path;
@@ -58,6 +61,7 @@ pub struct OpenFileOptions<D = StandardDataDictionary, T = TransferSyntaxRegistr
     ts_index: T,
     read_until: Option<Tag>,
     read_preamble: ReadPreamble,
+    odd_length: OddLengthStrategy,
 }
 
 impl OpenFileOptions {
@@ -92,6 +96,12 @@ impl<D, T> OpenFileOptions<D, T> {
         self
     }
 
+    /// Set how data elements with an odd length should be handled.
+    pub fn odd_length_strategy(mut self, option: OddLengthStrategy) -> Self {
+        self.odd_length = option;
+        self
+    }
+
     /// Set the transfer syntax index to use when reading the file.
     pub fn tranfer_syntax_index<Tr>(self, ts_index: Tr) -> OpenFileOptions<D, Tr>
     where
@@ -102,6 +112,7 @@ impl<D, T> OpenFileOptions<D, T> {
             read_until: self.read_until,
             read_preamble: self.read_preamble,
             ts_index,
+            odd_length: self.odd_length,
         }
     }
 
@@ -116,6 +127,7 @@ impl<D, T> OpenFileOptions<D, T> {
             read_until: self.read_until,
             read_preamble: self.read_preamble,
             ts_index: self.ts_index,
+            odd_length: self.odd_length,
         }
     }
 
@@ -133,6 +145,7 @@ impl<D, T> OpenFileOptions<D, T> {
             self.ts_index,
             self.read_until,
             self.read_preamble,
+            self.odd_length,
         )
     }
 
@@ -154,6 +167,7 @@ impl<D, T> OpenFileOptions<D, T> {
             self.ts_index,
             self.read_until,
             self.read_preamble,
+            self.odd_length,
         )
     }
 }

object/src/mem.rs

@@ -39,6 +39,7 @@
 use dicom_core::ops::{
     ApplyOp, AttributeAction, AttributeOp, AttributeSelector, AttributeSelectorStep,
 };
+use dicom_parser::dataset::read::{DataSetReaderOptions, OddLengthStrategy};
 use itertools::Itertools;
 use smallvec::SmallVec;
 use snafu::{ensure, OptionExt, ResultExt};
@@ -309,7 +310,14 @@ where
         P: AsRef<Path>,
         R: TransferSyntaxIndex,
     {
-        Self::open_file_with_all_options(path, dict, ts_index, None, ReadPreamble::Auto)
+        Self::open_file_with_all_options(
+            path,
+            dict,
+            ts_index,
+            None,
+            ReadPreamble::Auto,
+            Default::default(),
+        )
     }
 
     // detect the presence of a preamble
@@ -343,6 +351,7 @@ where
         ts_index: R,
         read_until: Option<Tag>,
         mut read_preamble: ReadPreamble,
+        odd_length: OddLengthStrategy,
     ) -> Result<Self, ReadError>
     where
         P: AsRef<Path>,
@@ -369,7 +378,15 @@ where
 
         // read rest of data according to metadata, feed it to object
         if let Some(ts) = ts_index.get(&meta.transfer_syntax) {
-            let mut dataset = DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;
+            let mut options = DataSetReaderOptions::default();
+            options.odd_length = odd_length;
+            let mut dataset = DataSetReader::new_with_ts_cs_options(
+                file,
+                ts,
+                SpecificCharacterSet::default(),
+                options,
+            )
+            .context(CreateParserSnafu)?;
             let obj = InMemDicomObject::build_object(
                 &mut dataset,
                 dict,
@@ -441,7 +458,14 @@ where
         S: Read + 's,
         R: TransferSyntaxIndex,
     {
-        Self::from_reader_with_all_options(src, dict, ts_index, None, ReadPreamble::Auto)
+        Self::from_reader_with_all_options(
+            src,
+            dict,
+            ts_index,
+            None,
+            ReadPreamble::Auto,
+            Default::default(),
+        )
     }
 
     pub(crate) fn from_reader_with_all_options<'s, S, R>(
@@ -450,6 +474,7 @@ where
         ts_index: R,
         read_until: Option<Tag>,
         mut read_preamble: ReadPreamble,
+        odd_length: OddLengthStrategy,
     ) -> Result<Self, ReadError>
     where
         S: Read + 's,
@@ -473,7 +498,14 @@ where
 
         // read rest of data according to metadata, feed it to object
         if let Some(ts) = ts_index.get(&meta.transfer_syntax) {
-            let mut dataset = DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;
+            let mut options = DataSetReaderOptions::default();
+            options.odd_length = odd_length;
+            let mut dataset = DataSetReader::new_with_ts_options(
+                file,
+                ts,
+                options,
+            )
+            .context(CreateParserSnafu)?;
             let obj = InMemDicomObject::build_object(
                 &mut dataset,
                 dict,