search

rexcardan / Evil-DICOM

A C# DICOM Library
171 stars 98 forks source link

DICOMElementReader.CheckForExplicitness Messing up on PixelData element #72

Closed itwouldbewise closed 4 years ago

itwouldbewise commented 4 years ago

Hi - thanks so much for this library, it's been super useful for me and saved me a ton of time on my projects.

Was having an issue where loading the DICOM would "freeze" - I stepped through and found that it happened while parsing the PixelData element due to DICOMElementReader.CheckForExplicitness.

My thinking is that if the PixelData happens to have two bytes at the front that encodes for a valid VR (in my case this CheckForExplicitness function changes the VR from OtherWord to PersonName) which causes it to call "ReadElementExplicitLittleEndian" instead of "ReadLittleEndian" here: public static IDICOMElement ReadElementImplicitLittleEndian(DICOMBinaryReader dr) { tag = TagReader.ReadLittleEndian(dr); VR vr = TagDictionary.GetVRFromTag(tag); if (CheckForExplicitness(tag, dr, ref vr)) { return ReadElementExplicitLittleEndian(tag, vr, dr); } else { int length = LengthReader.ReadLittleEndian(VR.Null, dr); byte[] data = DataReader.ReadLittleEndian(length, dr, TransferSyntax.IMPLICIT_VR_LITTLE_ENDIAN); IDICOMElement el = ElementFactory.GenerateElement(tag, vr, data, TransferSyntax.IMPLICIT_VR_LITTLE_ENDIAN); return el; } }

This results in it eventually reading garbage data (I see tag 0000,0000 among others) until it starts reading all the bytes using Multiplicity.ReadMultipleBinary which (since this is a 3D dose matrix PixelData object) takes a long time.

Anyway, my plan is to just remove the DICOMElementReader.CheckForExplicitness check which should be fine (unless it is badly formed DICOM) - is that a correct assumption? bad_dose_dicom.zip

Thanks & take care!

itwouldbewise commented 4 years ago

Just uploaded a sample anonymized DICOM dose file that reproduces the error. If you insert tag = TagReader.ReadLittleEndian(dr); if (tag == TagHelper.PIXEL_DATA) { int whatever = 5; } VR vr = TagDictionary.GetVRFromTag(tag);

and then set a breakpoint on the "whatever" line, you can step in and see how CheckForExplicitness changes the VR from OtherWord to PersonName. Hope this helps.

rexcardan commented 4 years ago

Ok. Thanks for digging into it. I will look at and correct this weekend.

rexcardan commented 4 years ago

Ok after taking a look, I realized that method is never called (in the current version of Evil-DICOM). Are you using an old version or something?

itwouldbewise commented 4 years ago

You're right, I am on an older version - I should have pulled the latest and tried it out. Sorry!