Merge pull request #4501 from thewtex/4801-dcmtk-preamble

BUG: DCMTK reader wrongly rejects file with preamble
InsightSoftwareConsortium · Mar 14, 2024 · be79ceb · be79ceb
2 parents 07a3979 + a099f08
commit be79ceb
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 1 deletion.
diff --git a/Modules/IO/DCMTK/src/itkDCMTKImageIO.cxx b/Modules/IO/DCMTK/src/itkDCMTKImageIO.cxx
@@ -31,6 +31,26 @@
 #include "dcmtk/dcmdata/dcrledrg.h"
 #include "dcmtk/oflog/oflog.h"
 
+namespace
+{
+
+/**
+ * Helper function to test for 128 byte dicom preamble
+ * @param file A stream to test if the file is dicom like
+ * @return true if the stream has a dicom preamble
+ */
+static bool
+readPreambleDicom(std::ifstream & file)
+{
+  char preamble[132] = "";
+
+  file.read(preamble, sizeof(preamble));
+
+  return (preamble[128] == 'D' && preamble[129] == 'I' && preamble[130] == 'C' && preamble[131] == 'M');
+}
+
+} // end anonymous namespace
+
 namespace itk
 {
 /** Constructor */
@@ -220,6 +240,8 @@ DCMTKImageIO::CanReadFile(const char * filename)
 #if !defined(__EMSCRIPTEN__)
   {
     std::ifstream file;
+
+    // look for a preamble
     try
     {
       this->OpenFileForReading(file, filename);
@@ -228,9 +250,11 @@ DCMTKImageIO::CanReadFile(const char * filename)
     {
       return false;
     }
+    const bool hasdicompreamble = readPreambleDicom(file);
+    file.seekg(0, std::ios::beg);
     const bool hasdicomsig = readNoPreambleDicom(file);
     file.close();
-    if (!hasdicomsig)
+    if (!hasdicomsig && !hasdicompreamble)
     {
       return false;
     }

diff --git a/Modules/IO/DCMTK/test/CMakeLists.txt b/Modules/IO/DCMTK/test/CMakeLists.txt
@@ -58,6 +58,28 @@ itk_add_test(
   ${ITK_TEST_OUTPUT_DIR}/itkDCMTKImageIOTest4.png
   ${ITK_TEST_OUTPUT_DIR}/itkDCMTKRescaleImageIOTest4.dcm)
 
+itk_add_test(
+  NAME
+  itkDCMTKImageIOTest5
+  COMMAND
+  ITKIODCMTKTestDriver
+  itkDCMTKImageIOTest
+  DATA{Input/preamble.dcm}
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKImageIOTest5.dcm
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKImageIOTest5.png
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKRescaleImageIOTest5.dcm)
+
+itk_add_test(
+  NAME
+  itkDCMTKImageIOTest6
+  COMMAND
+  ITKIODCMTKTestDriver
+  itkDCMTKImageIOTest
+  DATA{Input/no_preamble.dcm}
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKImageIOTest6.dcm
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKImageIOTest6.png
+  ${ITK_TEST_OUTPUT_DIR}/itkDCMTKRescaleImageIOTest6.dcm)
+
 itk_add_test(
   NAME
   itkDCMTKSeriesReadImageWrite
@@ -190,6 +212,16 @@ itk_add_test(
   itkDCMTKImageIONoPreambleTest
   DATA{Input/NoPreambleDicomTest.dcm})
 
+# Re-use the test driver to ensure that CanReadFile returns true for a file
+# with a preamble.
+itk_add_test(
+  NAME
+  itkDCMTKImageIOPreambleCanReadTest
+  COMMAND
+  ITKIODCMTKTestDriver
+  itkDCMTKImageIONoPreambleTest
+  DATA{Input/preamble.dcm})
+
 itk_add_test(
   NAME itkDCMTKImageIOSpacingTest
   COMMAND ITKIODCMTKTestDriver itkDCMTKImageIOSpacingTest

diff --git a/Modules/IO/DCMTK/test/Input/no_preamble.dcm.cid b/Modules/IO/DCMTK/test/Input/no_preamble.dcm.cid
@@ -0,0 +1 @@
+bafkreidvhgcecqrku7u63wbzocw52mragf5i4ycbixcon5yohdlfctr63y
diff --git a/Modules/IO/DCMTK/test/Input/preamble.dcm.cid b/Modules/IO/DCMTK/test/Input/preamble.dcm.cid
@@ -0,0 +1 @@
+bafkreibhyvtpyhwha2y5zjtx5mn4uod5mf33vwvtem3f5whyvivjpcde2a
diff --git a/Modules/IO/GDCM/test/Baseline/no_preamble.mha.cid b/Modules/IO/GDCM/test/Baseline/no_preamble.mha.cid
@@ -0,0 +1 @@
+bafkreicwoy4frgs5faincl6oqr6osu4zgn3dt74iyqphmthj7hs3x2y2w4
diff --git a/Modules/IO/GDCM/test/Baseline/preamble.mha.cid b/Modules/IO/GDCM/test/Baseline/preamble.mha.cid
@@ -0,0 +1 @@
+bafkreicwoy4frgs5faincl6oqr6osu4zgn3dt74iyqphmthj7hs3x2y2w4
diff --git a/Modules/IO/GDCM/test/CMakeLists.txt b/Modules/IO/GDCM/test/CMakeLists.txt
@@ -255,6 +255,16 @@ itk_add_test(
   itkGDCMImageIONoPreambleTest
   DATA{Input/NoPreambleDicomTest.dcm})
 
+# Re-use the test driver to ensure that CanReadFile returns true for a file
+# with a preamble.
+itk_add_test(
+  NAME
+  itkGDCMImageIOPreambleCanReadTest
+  COMMAND
+  ITKIOGDCMTestDriver
+  itkGDCMImageIONoPreambleTest
+  DATA{Input/preamble.dcm})
+
 itk_add_test(
   NAME
   itkGDCMImageReadWriteTest_RGB
@@ -350,6 +360,32 @@ itk_add_test(
   ${ITK_TEST_OUTPUT_DIR}/single-bit.mha
   scalar)
 
+itk_add_test(
+  NAME
+  itkGDCMImageReadWriteTest_preamble
+  COMMAND
+  ITKIOGDCMTestDriver
+  --compare
+  DATA{Baseline/preamble.mha}
+  ${ITK_TEST_OUTPUT_DIR}/preamble.mha
+  itkGDCMImageReadWriteTest
+  DATA{Input/preamble.dcm}
+  ${ITK_TEST_OUTPUT_DIR}/preamble.mha
+  scalar)
+
+itk_add_test(
+  NAME
+  itkGDCMImageReadWriteTest_no_preamble
+  COMMAND
+  ITKIOGDCMTestDriver
+  --compare
+  DATA{Baseline/no_preamble.mha}
+  ${ITK_TEST_OUTPUT_DIR}/no_preamble.mha
+  itkGDCMImageReadWriteTest
+  DATA{Input/no_preamble.dcm}
+  ${ITK_TEST_OUTPUT_DIR}/no_preamble.mha
+  scalar)
+
 function(AddComplianceTest fileName)
   itk_add_test(
     NAME

diff --git a/Modules/IO/GDCM/test/Input/no_preamble.dcm.cid b/Modules/IO/GDCM/test/Input/no_preamble.dcm.cid
@@ -0,0 +1 @@
+bafkreidvhgcecqrku7u63wbzocw52mragf5i4ycbixcon5yohdlfctr63y
diff --git a/Modules/IO/GDCM/test/Input/preamble.dcm.cid b/Modules/IO/GDCM/test/Input/preamble.dcm.cid
@@ -0,0 +1 @@
+bafkreibhyvtpyhwha2y5zjtx5mn4uod5mf33vwvtem3f5whyvivjpcde2a
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		bafkreidvhgcecqrku7u63wbzocw52mragf5i4ycbixcon5yohdlfctr63y
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		bafkreibhyvtpyhwha2y5zjtx5mn4uod5mf33vwvtem3f5whyvivjpcde2a
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		bafkreicwoy4frgs5faincl6oqr6osu4zgn3dt74iyqphmthj7hs3x2y2w4