search

Enet4 / dicom-rs

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

Error while converting decimal string(DS) to float64 #333

Closed lalberge closed 1 year ago

lalberge commented 1 year ago

Hi,

We found a bug while converting decimal strings to float64, it happened while reading the RescaleIntercept attribute with the value " -1024". As stated in the DICOM norm, decimal strings should ignore trimming and leading spaces: "A string of characters representing either a fixed point number or a floating point number. A fixed point number shall contain only the characters 0-9 with an optional leading "+" or "-" and an optional "." to mark the decimal point. A floating point number shall be conveyed as defined in ANSI X3.9, with an "E" or "e" to indicate the start of the exponent. Decimal Strings may be padded with leading or trailing spaces. Embedded spaces are not allowed."

The bug could be easily fixed by adding trim .trim_start() before casting to float64. https://github.com/Enet4/dicom-rs/blob/c1644cf355f160946f947273bb89283546be2c98/core/src/value/primitive.rs#L1654

I would be happy to contribute to that fix if you accept external contributions.

Enet4 commented 1 year ago

@lalberge Hello! Thank you for reaching out with this bug! It may indeed be an oversight on what is allowed in values with the DS and IS value representation. I would gladly take a pull request for this (see also the contribution guidelines).

lalberge commented 1 year ago

@Enet4 Hello. I have implemented the bug fix and its corresponding test cases. The branch is ready locally, but I am not allowed to push it and create the associated PR. I could not find in the contributing readme how to do that :)

Léo

Enet4 commented 1 year ago

Hello @lalberge! You can create your own fork here on GitHub and create a pull request from it. See the link for more details and let me know if you need more assistance.

lalberge commented 1 year ago

Hi @Enet4,

The PR is ready to be reviewed here: https://github.com/Enet4/dicom-rs/pull/335