bug in Opener.java

[Ilan]

There is
public int getFileType(String path)
which has a bug. The dicom check reads as

                 // DICOM ("DICM" at offset 128)
                if (buf[128]==68 && buf[129]==73 && buf[130]==67 && buf[131]==77 || name.endsWith(".dcm")) {
                        return DICOM;
                }

The problem is there can be nonsense at buf 128 to 131 and the file will still be marked as dicom just because of its extension. I happened to have a file of zero length which nicely passed itself off as valid dicom.

Ilan

[Rasband, Wayne (NIH/NIMH) [E]]

On Apr 21, 2016, at 3:16 AM, Ilan <[hidden email]> wrote:

>
> There is
> public int getFileType(String path)
> which has a bug. The dicom check reads as
>
> // DICOM ("DICM" at offset 128)
> if (buf[128]==68 && buf[129]==73 && buf[130]==67 && buf[131]==77 ||
> name.endsWith(".dcm")) {
> return DICOM;
> }
>
> The problem is there can be nonsense at buf 128 to 131 and the file will
> still be marked as dicom just because of its extension. I happened to have a
> file of zero length which nicely passed itself off as valid dicom.

I get the message

   This does not appear to be a valid
   DICOM file. It does not have the
   characters 'DICM' at offset 128.

when I try to open a zero length file with a “.dcm” extension, which seems like a reasonable error message.

-wayne


--
ImageJ mailing list: http://imagej.nih.gov/ij/list.html

[Ilan]

Thanks for the response Wayne.
With Pet-Ct I built my own GUI which lets the user choose either the entire study or individual series.
Most important is that after reading in the data, it checks if there are PET and CT series.
If so it will automatically bring up the Pet-Ct viewer. If not it will just leave the ImageJ images as is.

The relevant code fragment is
                        k = opener.getFileType(flName);
                        if( k == Opener.UNKNOWN || k == Opener.TEXT) {
                                tmp = results[j-1].getName();
                                if( tmp.startsWith("graphic") && tmp.endsWith("gr1")) {
                                        frameText = ChoosePetCt.getFrameText(flName);
                                }
                                continue;
                        }
                        opener.setSilentMode(true);
                        imp = opener.openImage(flName);

Perhaps I don't see the error message because of the set silent mode but it is needed if I may be opening 500+ files.
What happens with my bad study (some 150 empty files out of 400), is that the program hangs.
I looked with the debugger and opener returns Opener.DICOM because of the OR statement with file type dcm.
Had it returned Opener.UNKNOWN it would have jumped over the bad files.

I solved the problem by checking for an empty file before calling the getFileType, so that it is no longer a real problem.
I just thought that the getFileType is weak if it goes to the trouble of checking the buffer variable, but is willing to accept
the result if the empty file has an extension of dcm. It will also accept a non empty file which has garbage in the buffer
on the condition that it has the right extension.

If later on you reject the file because it lacks the letters, you might just as well reject it in getFileType, instead of having
it give false hope that it is really a dicom file just because it has the right extension.