Enet4 / dicom-rs

Rust implementation of the DICOM standard
https://dicom-rs.github.io
Apache License 2.0
415 stars 82 forks source link

Cannot read dicom file with some objects that contains dicom_date_time tag #556

Closed qarmin closed 2 hours ago

qarmin commented 2 weeks ago

Code

use std::fs;

use dicom_core::{PrimitiveValue, VR};
use dicom_core::chrono::{DateTime, FixedOffset, NaiveDate, NaiveDateTime, NaiveTime, Utc};
use dicom_core::DicomValue::Primitive;
use dicom_core::prelude::{DicomDate, DicomDateTime};
use dicom_dictionary_std::tags::ACQUISITION_DATE_TIME;
use dicom_dictionary_std::uids::{ENCAPSULATED_PDF_STORAGE, EXPLICIT_VR_LITTLE_ENDIAN};
use dicom_object::{FileMetaTableBuilder, InMemDicomObject, OpenFileOptions};
use dicom_object::mem::InMemElement;

fn main() {
    let mut dicom_in_memory_object = InMemDicomObject::new_empty();

    // Broken date
    let date_time: DateTime<Utc> = DateTime::from_naive_utc_and_offset(
        NaiveDateTime::new(
            NaiveDate::from_ymd_opt(2024, 8, 9).unwrap(),
            NaiveTime::from_hms_opt(9, 9, 39).unwrap(),
        ),
        Utc,
    );
    let dicom_date_time = DicomDateTime::try_from(&date_time.with_timezone(&FixedOffset::east_opt(0).unwrap())).unwrap();

    // Working date - uncomment to fix problem
    // let dicom_date_time = DicomDateTime::from_date_with_time_zone(DicomDate::from_y(1991).unwrap(), FixedOffset::east_opt(0).unwrap());

    let in_mem_element = InMemElement::new(
        ACQUISITION_DATE_TIME, VR::DT, Primitive(PrimitiveValue::DateTime(vec![dicom_date_time].into())),
    );
    dicom_in_memory_object.put_element(in_mem_element);

    // save
    let _ = fs::remove_file("test.dcm");
    let meta_builder = FileMetaTableBuilder::new()
        .transfer_syntax(EXPLICIT_VR_LITTLE_ENDIAN)
        .media_storage_sop_class_uid(ENCAPSULATED_PDF_STORAGE)
        .media_storage_sop_instance_uid("124.0.0.0.1");

    let d_object = dicom_in_memory_object.with_meta(meta_builder).unwrap();
    d_object.write_to_file("test.dcm").unwrap();

    let data = OpenFileOptions::new().open_file("test.dcm");
    dbg!(&data);
}

error

[src/main.rs:45:5] &data = Err(
    ReadToken {
        source: ReadValue {
            len: 26,
            tag: Tag(0x0008, 0x002A),
            source: ReadValueData {
                position: 8,
                source: Error {
                    kind: UnexpectedEof,
                    message: "failed to fill whole buffer",
                },
                backtrace: Backtrace [
                    { fn: "snafu::backtrace_impl::<impl snafu::GenerateImplicitData for std::backtrace::Backtrace>::generate", file: "/home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/snafu-0.8.4/src/backtrace_impl_std.rs", line: 5 },
                    { fn: "snafu::GenerateImplicitData::generate_with_source", file: "/home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/snafu-0.8.4/src/lib.rs", line: 1281 },
                    { fn: "<dicom_parser::stateful::decode::ReadValueDataSnafu<__T0> as snafu::IntoError<dicom_parser::stateful::decode::Error>>::into_error", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/parser/src/stateful/decode.rs", line: 25 },
                    { fn: "<core::result::Result<T,E> as snafu::ResultExt<T,E>>::context", file: "/home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/snafu-0.8.4/src/lib.rs", line: 812 },
                    { fn: "dicom_parser::stateful::decode::StatefulDecoder<D,S,BD,TC>::read_value_strs", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/parser/src/stateful/decode.rs", line: 409 },
                    { fn: "<dicom_parser::stateful::decode::StatefulDecoder<D,S,BD> as dicom_parser::stateful::decode::StatefulDecode>::read_value_preserved", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/parser/src/stateful/decode.rs", line: 985 },
                    { fn: "dicom_parser::dataset::read::DataSetReader<S>::read_value", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/parser/src/dataset/read.rs", line: 587 },
                    { fn: "<dicom_parser::dataset::read::DataSetReader<S> as core::iter::traits::iterator::Iterator>::next", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/parser/src/dataset/read.rs", line: 412 },
                    { fn: "dicom_object::mem::InMemDicomObject<D>::build_object", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/object/src/mem.rs", line: 1865 },
                    { fn: "dicom_object::mem::<impl dicom_object::FileDicomObject<dicom_object::mem::InMemDicomObject<D>>>::open_file_with_all_options", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/object/src/mem.rs", line: 373 },
                    { fn: "dicom_object::file::OpenFileOptions<D,T>::open_file", file: "/home/rafal/.cargo/git/checkouts/dicom-rs-165e2a1669724caa/de7dc58/object/src/file.rs", line: 130 },
                    { fn: "testy::main", file: "./src/main.rs", line: 44 },
                    { fn: "core::ops::function::FnOnce::call_once", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs", line: 250 },
                    { fn: "std::sys_common::backtrace::__rust_begin_short_backtrace", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/sys_common/backtrace.rs", line: 155 },
                    { fn: "std::rt::lang_start::{{closure}}", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/rt.rs", line: 159 },
                    { fn: "core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs", line: 284 },
                    { fn: "std::panicking::try::do_call", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs", line: 559 },
                    { fn: "std::panicking::try", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs", line: 523 },
                    { fn: "std::panic::catch_unwind", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panic.rs", line: 149 },
                    { fn: "std::rt::lang_start_internal::{{closure}}", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/rt.rs", line: 141 },
                    { fn: "std::panicking::try::do_call", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs", line: 559 },
                    { fn: "std::panicking::try", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs", line: 523 },
                    { fn: "std::panic::catch_unwind", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panic.rs", line: 149 },
                    { fn: "std::rt::lang_start_internal", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/rt.rs", line: 141 },
                    { fn: "std::rt::lang_start", file: "/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/rt.rs", line: 158 },
                    { fn: "main" },
                    { fn: "__libc_start_call_main", file: "./csu/../sysdeps/nptl/libc_start_call_main.h", line: 58 },
                    { fn: "__libc_start_main_impl", file: "./csu/../csu/libc-start.c", line: 392 },
                    { fn: "_start" },
                ],
            },
        },
    },
)

file - test.dcm.zip

even dcmdump cannot read this file

E: DcmElement: AcquisitionDateTime (0008,002a) larger (26) than remaining bytes (22) in file, premature end of stream
E: dcmdump: Invalid stream: reading file: test.dcm
Enet4 commented 2 weeks ago

Thank you for reporting. There seems to be a mismatch between the encoded DICOM datetime and its precalculated length. I believe that the chrono DateTime provided should yield a DICOM datetime of maximum precision in this case (20240809090939.000000+0000, 26 bytes), but right now it is being encoded as 20240809090939.0+0000.

marn13 commented 2 weeks ago

Hi,

I think that the same issue you fixed in https://github.com/Enet4/dicom-rs/pull/557 exists also in

impl fmt::Display for DicomTime{
}

Regards, Marcin

Enet4 commented 2 weeks ago

Good catch @marn13, that could probably justify how this happened. In this case I would let that impl suppress the trailing zeros, because Display is intended for human readability and does not have to comply with DICOM value encoding constraints, unlike to_encoded().