- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 148
 
Description
Background: I was having some odd issues that ended up boiling down to "I was accidentally trying to parse gzipped files instead of DICOM files" - oddly, the library did not fail quickly and ended up reading the whole file into memory before failing.
Upon calling Parse on input data that is not DICOM-formatted, the library tries to parse the header, fails, and continues in "compatibility mode" where it tries to parse an element to determine a transfer syntax. The attempt to parse an element based on a few transfer syntaxes, but readElement can succeed with garbage data, as long as it doesn't attempt to read past the 100 bytes allocated for this purpose.
In practice, this reads a few bytes for the tag, a few bytes for the VR (if trying an explicit syntax), then a few bytes for the VL, and if the VL bytes happen to be a value less than the remainder of the 100 bytes, it will succeed. The rest of the file is parsed similarly until the VL happens to run off the end of the file.
Lines 228 to 232 in 4c45b44
| _, err := subR.readElement(nil, nil) | |
| if err == nil { | |
| return true | |
| } | |
| return false | 
The element itself here isn't checked. I think it would be reasonable to check that the VR is a known VR, and that the tag is known, or is a valid private creator data element, and fail the parsing if this does not succeed. I know the intention is to be permissive, but by this point the file has already failed to present a valid header, and has failed three attempts to parse an element out of the first 100 bytes.