Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Handle ExplicitVR DICOM file contains invalid VR #1660

Open
DanielHMH opened this issue Sep 22, 2023 · 2 comments
Open

Handle ExplicitVR DICOM file contains invalid VR #1660

DanielHMH opened this issue Sep 22, 2023 · 2 comments

Comments

@DanielHMH
Copy link

With an ExplicitVR DICOM file.
DicomFile.Open method return partial dataset when dicom item contains invalid VR.

To Reproduce
With an ExplicitVR DICOM file.
Change one dicom item's VR to a string other than defined VRs.
For example, if the dataset contains item: 0010 21B0, the formal VR is LT, but -- is placed.
Bytes placed in file:
10 00 B0 21 2D 2D 54 00 47 30 30 30 30 ......
When item 0010 21B0 is read, DicomFileReader returns DicomReaderResult.Suspended.

Expected behavior
Continue to parse items.

Environment
Fellow Oak DICOM version: 5.1.1
OS: Windows 10
Platform: NET Framework 4.6.1

@DanielHMH DanielHMH added the new label Sep 22, 2023
@gofal gofal added evaluating and removed new labels Sep 25, 2023
@gofal
Copy link
Contributor

gofal commented Sep 25, 2023

If you suggest that fo-dicom should continue reading and silently ignore the obviously wrong data, how should fo-dicom then proceed with that data?
If the code then accesses the DicomTag and conversion of the value has to be done, what should then be returned? the binary stream has to be interpreted as text, as integer value, as float value, or whatever.
And if then the DicomDataset is saved to file or sent via network, fo-dicom then should spread the invalid data (which might cause errors from the scp), or should fo-dicom skip/drop those invalid data?

The expected behavior is not as trivial as one might expect.

@DanielHMH
Copy link
Author

With _isExplicitVR.
Currently, DicomReader tries best to ParseVR when _badPrivateSequence or VR IsNullOrEmpty(trim).
If ExplicitVR DICOM file contains such invalid VR item, it treats this item with DicomVR.Implicit.
Maybe the real world exists many mixed VR DICOM file although it claims explicit.
The length of data is mistaken, that's why DicomReaderResult.Suspended is returned.

Some other programs handle such file keeps the wrong VR and go on.

Suggest:
For compatibility with current code:
If the item is defined in dictionary, use the defined VR instead of invalid VR can solve this problem.

For flexibility:
Maybe add options indicate how to treat the invalid file.
1st option is check DICOM file format, when not valid, return error.
This is suit for those need strictly correct DICOM format.
2nd option is the DICOM file VR format is claimed, NO MIXED VR after DICOM File Meta Information.
Try to fix VR with defined dictionary.
When not valid, return error.
This is suit for sort of modify-route purpose.
3rd option is the DICOM file VR format is MIXED. This is too complex to handle.
Such condition shall be site (or manufacture) specific, not generic. (?)
Overridable parse* methods maybe required.
But follow current logic maybe solve some problems:
Try to fix VR with defined dictionary.
If not possible to fix VR with defined dictionary, it's implicit VR.
If it's implicit VR and read data exceeds end of file, return error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants