changeset 2445:6e5bc5c6d1a4

Fix to allow creating DICOM instances with empty Specific Character Set (0008,0005)
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 14 Dec 2017 13:02:06 +0100
parents d9e3781d2023
children a894adc8bb03
files Core/DicomParsing/ParsedDicomFile.cpp NEWS UnitTestsSources/FromDcmtkTests.cpp
diffstat 3 files changed, 131 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/Core/DicomParsing/ParsedDicomFile.cpp	Fri Dec 08 10:21:24 2017 +0100
+++ b/Core/DicomParsing/ParsedDicomFile.cpp	Thu Dec 14 13:02:06 2017 +0100
@@ -39,38 +39,38 @@
   Program: GDCM (Grassroots DICOM). A DICOM library
   Module:  http://gdcm.sourceforge.net/Copyright.html
 
-Copyright (c) 2006-2011 Mathieu Malaterre
-Copyright (c) 1993-2005 CREATIS
-(CREATIS = Centre de Recherche et d'Applications en Traitement de l'Image)
-All rights reserved.
+  Copyright (c) 2006-2011 Mathieu Malaterre
+  Copyright (c) 1993-2005 CREATIS
+  (CREATIS = Centre de Recherche et d'Applications en Traitement de l'Image)
+  All rights reserved.
 
-Redistribution and use in source and binary forms, with or without
-modification, are permitted provided that the following conditions are met:
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions are met:
 
- * Redistributions of source code must retain the above copyright notice,
-   this list of conditions and the following disclaimer.
+  * Redistributions of source code must retain the above copyright notice,
+  this list of conditions and the following disclaimer.
 
- * Redistributions in binary form must reproduce the above copyright notice,
-   this list of conditions and the following disclaimer in the documentation
-   and/or other materials provided with the distribution.
+  * Redistributions in binary form must reproduce the above copyright notice,
+  this list of conditions and the following disclaimer in the documentation
+  and/or other materials provided with the distribution.
 
- * Neither name of Mathieu Malaterre, or CREATIS, nor the names of any
-   contributors (CNRS, INSERM, UCB, Universite Lyon I), may be used to
-   endorse or promote products derived from this software without specific
-   prior written permission.
+  * Neither name of Mathieu Malaterre, or CREATIS, nor the names of any
+  contributors (CNRS, INSERM, UCB, Universite Lyon I), may be used to
+  endorse or promote products derived from this software without specific
+  prior written permission.
 
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS''
-AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
-ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR
-ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
-DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
-SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
-CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
-OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS''
+  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+  ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR
+  ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+  DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+  OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-=========================================================================*/
+  =========================================================================*/
 
 
 #include "../PrecompiledHeaders.h"
@@ -911,36 +911,59 @@
   void ParsedDicomFile::CreateFromDicomMap(const DicomMap& source,
                                            Encoding defaultEncoding)
   {
-    pimpl_->file_.reset(new DcmFileFormat);
-
-    const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET);
-    if (tmp != NULL)
+    try
     {
-      Encoding encoding;
-      if (tmp->IsNull() ||
-          tmp->IsBinary() ||
-          !GetDicomEncoding(encoding, tmp->GetContent().c_str()))
+      pimpl_->file_.reset(new DcmFileFormat);
+
+      const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET);
+
+      if (tmp == NULL)
       {
+        SetEncoding(defaultEncoding);
+      }
+      else if (tmp->IsBinary())
+      {
+        LOG(ERROR) << "Invalid binary string in the SpecificCharacterSet (0008,0005) tag";
         throw OrthancException(ErrorCode_ParameterOutOfRange);
       }
+      else if (tmp->IsNull() ||
+               tmp->GetContent().empty())
+      {
+        SetEncoding(defaultEncoding);
+      }
       else
       {
-        SetEncoding(encoding);
+        Encoding encoding;
+
+        if (GetDicomEncoding(encoding, tmp->GetContent().c_str()))
+        {
+          SetEncoding(encoding);
+        }
+        else
+        {
+          LOG(ERROR) << "Unsupported value for the SpecificCharacterSet (0008,0005) tag: \""
+                     << tmp->GetContent() << "\"";        
+          throw OrthancException(ErrorCode_ParameterOutOfRange);
+        }
+      }
+
+      for (DicomMap::Map::const_iterator 
+             it = source.map_.begin(); it != source.map_.end(); ++it)
+      {
+        if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET &&
+            !it->second->IsNull())
+        {
+          ReplacePlainString(it->first, it->second->GetContent());
+        }
       }
     }
-    else
+    catch (OrthancException&)
     {
-      SetEncoding(defaultEncoding);
-    }
-
-    for (DicomMap::Map::const_iterator 
-           it = source.map_.begin(); it != source.map_.end(); ++it)
-    {
-      if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET &&
-          !it->second->IsNull())
-      {
-        ReplacePlainString(it->first, it->second->GetContent());
-      }
+      // Manually delete the PImpl to avoid a memory leak due to
+      // throwing the exception in the constructor
+      delete pimpl_;
+      pimpl_ = NULL;
+      throw;
     }
   }
 
@@ -1070,7 +1093,7 @@
   }
 
 
-#if (ORTHANC_ENABLE_JPEG == 1 &&  \
+#if (ORTHANC_ENABLE_JPEG == 1 &&                \
      ORTHANC_ENABLE_PNG == 1)
   void ParsedDicomFile::EmbedImage(const std::string& mime,
                                    const std::string& content)
@@ -1377,8 +1400,8 @@
   ParsedDicomFile* ParsedDicomFile::CreateFromJson(const Json::Value& json,
                                                    DicomFromJsonFlags flags)
   {
-	const bool generateIdentifiers = (flags & DicomFromJsonFlags_GenerateIdentifiers) ? true : false;
-	const bool decodeDataUriScheme = (flags & DicomFromJsonFlags_DecodeDataUriScheme) ? true : false;
+    const bool generateIdentifiers = (flags & DicomFromJsonFlags_GenerateIdentifiers) ? true : false;
+    const bool decodeDataUriScheme = (flags & DicomFromJsonFlags_DecodeDataUriScheme) ? true : false;
 
     std::auto_ptr<ParsedDicomFile> result(new ParsedDicomFile(generateIdentifiers));
     result->SetEncoding(FromDcmtkBridge::ExtractEncoding(json, GetDefaultDicomEncoding()));
--- a/NEWS	Fri Dec 08 10:21:24 2017 +0100
+++ b/NEWS	Thu Dec 14 13:02:06 2017 +0100
@@ -7,6 +7,11 @@
 * "/system" URI returns the version of the Orthanc REST API
 * added ?expand argument to /peers and /modalities routes
 
+Maintenance
+-----------
+
+* Fix to allow creating DICOM instances with empty Specific Character Set (0008,0005)
+
 
 Version 1.3.1 (2017-11-29)
 ==========================
--- a/UnitTestsSources/FromDcmtkTests.cpp	Fri Dec 08 10:21:24 2017 +0100
+++ b/UnitTestsSources/FromDcmtkTests.cpp	Thu Dec 14 13:02:06 2017 +0100
@@ -1250,3 +1250,56 @@
 {
   ASSERT_EQ(toUpperResult, Toolbox::ToUpperCaseWithAccents(toUpperSource));
 }
+
+
+
+TEST(ParsedDicomFile, InvalidCharacterSets)
+{
+  {
+    // No encoding provided, fallback to default encoding
+    DicomMap m;
+    m.SetValue(DICOM_TAG_PATIENT_NAME, "HELLO", false);
+
+    ParsedDicomFile d(m, Encoding_Latin3 /* default encoding */);
+    ASSERT_EQ(Encoding_Latin3, d.GetEncoding());
+  }
+  
+  {
+    // Valid encoding, "ISO_IR 13" is Japanese
+    DicomMap m;
+    m.SetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET, "ISO_IR 13", false);
+    m.SetValue(DICOM_TAG_PATIENT_NAME, "HELLO", false);
+
+    ParsedDicomFile d(m, Encoding_Latin3 /* default encoding */);
+    ASSERT_EQ(Encoding_Japanese, d.GetEncoding());
+  }
+  
+  {
+    // Invalid value for an encoding ("nope" is not in the DICOM standard)
+    DicomMap m;
+    m.SetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET, "nope", false);
+    m.SetValue(DICOM_TAG_PATIENT_NAME, "HELLO", false);
+
+    ASSERT_THROW(ParsedDicomFile d(m, Encoding_Latin3), OrthancException);
+  }
+  
+  {
+    // Invalid encoding, as provided as a binary string
+    DicomMap m;
+    m.SetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET, "ISO_IR 13", true);
+    m.SetValue(DICOM_TAG_PATIENT_NAME, "HELLO", false);
+
+    ASSERT_THROW(ParsedDicomFile d(m, Encoding_Latin3), OrthancException);
+  }
+  
+  {
+    // Encoding provided as an empty string, fallback to default encoding
+    // In Orthanc <= 1.3.1, this test was throwing an exception
+    DicomMap m;
+    m.SetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET, "", false);
+    m.SetValue(DICOM_TAG_PATIENT_NAME, "HELLO", false);
+
+    ParsedDicomFile d(m, Encoding_Latin3 /* default encoding */);
+    ASSERT_EQ(Encoding_Latin3, d.GetEncoding());
+  }
+}