diff Core/DicomParsing/FromDcmtkBridge.cpp @ 3691:4922bdd046dd

Fix issue #140 (Modifying private tags with REST API changes VR from LO to UN) - DANGEROUS COMMIT
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 25 Feb 2020 21:44:09 +0100
parents 46cb00e4adbb
children fd302ec6a502
line wrap: on
line diff
--- a/Core/DicomParsing/FromDcmtkBridge.cpp	Tue Feb 25 13:57:43 2020 +0100
+++ b/Core/DicomParsing/FromDcmtkBridge.cpp	Tue Feb 25 21:44:09 2020 +0100
@@ -115,6 +115,16 @@
 
 namespace Orthanc
 {
+  static bool IsBinaryTag(const DcmTag& key)
+  {
+    return (key.isUnknownVR() ||
+            key.getEVR() == EVR_OB ||
+            key.getEVR() == EVR_OW ||
+            key.getEVR() == EVR_UN ||
+            key.getEVR() == EVR_ox);
+  }
+
+
 #if DCMTK_USE_EMBEDDED_DICTIONARIES == 1
   static void LoadEmbeddedDictionary(DcmDataDictionary& dictionary,
                                      EmbeddedResources::FileResourceId resource)
@@ -938,18 +948,13 @@
       if (!(flags & DicomToJsonFlags_IncludeUnknownTags))
       {
         DictionaryLocker locker;
-        if (locker->findEntry(element->getTag(), NULL) == NULL)
+        if (locker->findEntry(element->getTag(), element->getTag().getPrivateCreator()) == NULL)
         {
           continue;
         }
       }
 
-      DcmEVR evr = element->getTag().getEVR();
-      if (evr == EVR_OB ||
-          evr == EVR_OF ||
-          evr == EVR_OW ||
-          evr == EVR_UN ||
-          evr == EVR_ox)
+      if (IsBinaryTag(element->getTag()))
       {
         // This is a binary tag
         if ((tag == DICOM_TAG_PIXEL_DATA && !(flags & DicomToJsonFlags_IncludePixelData)) ||
@@ -1371,189 +1376,49 @@
   }
 
 
-  static bool IsBinaryTag(const DcmTag& key)
+  DcmElement* FromDcmtkBridge::CreateElementForTag(const DicomTag& tag,
+                                                   const std::string& privateCreator)
   {
-    return (key.isUnknownVR() ||
-#if DCMTK_VERSION_NUMBER >= 361
-            key.getEVR() == EVR_OD ||
-#endif
-            
-#if DCMTK_VERSION_NUMBER >= 362
-            key.getEVR() == EVR_OL ||
-#endif            
-            key.getEVR() == EVR_OB ||
-            key.getEVR() == EVR_OF ||
-            key.getEVR() == EVR_OW ||
-            key.getEVR() == EVR_UN ||
-            key.getEVR() == EVR_ox);
-  }
-
-
-  DcmElement* FromDcmtkBridge::CreateElementForTag(const DicomTag& tag)
-  {
-    DcmTag key(tag.GetGroup(), tag.GetElement());
-
-    if (tag.IsPrivate() ||
-        IsBinaryTag(key))
+    if (tag.IsPrivate() &&
+        privateCreator.empty())
     {
-      return new DcmOtherByteOtherWord(key);
+      // This solves issue 140 (Modifying private tags with REST API
+      // changes VR from LO to UN)
+      // https://bitbucket.org/sjodogne/orthanc/issues/140
+      LOG(WARNING) << "Private creator should not be empty while creating a private tag: " << tag.Format();
     }
-
-    switch (key.getEVR())
-    {
-      // http://support.dcmtk.org/docs/dcvr_8h-source.html
-
-      /**
-       * Binary types, handled above
-       **/
     
 #if DCMTK_VERSION_NUMBER >= 361
-      case EVR_OD:
-#endif            
-
-#if DCMTK_VERSION_NUMBER >= 362
-      case EVR_OL:
-#endif            
-
-      case EVR_OB:  // other byte
-      case EVR_OF:  // other float
-      case EVR_OW:  // other word
-      case EVR_UN:  // unknown value representation
-      case EVR_ox:  // OB or OW depending on context
-        throw OrthancException(ErrorCode_InternalError);
-
-
-      /**
-       * String types.
-       * http://support.dcmtk.org/docs/classDcmByteString.html
-       **/
-      
-      case EVR_AS:  // age string
-        return new DcmAgeString(key);
-
-      case EVR_AE:  // application entity title
-        return new DcmApplicationEntity(key);
-
-      case EVR_CS:  // code string
-        return new DcmCodeString(key);        
-
-      case EVR_DA:  // date string
-        return new DcmDate(key);
-        
-      case EVR_DT:  // date time string
-        return new DcmDateTime(key);
-
-      case EVR_DS:  // decimal string
-        return new DcmDecimalString(key);
-
-      case EVR_IS:  // integer string
-        return new DcmIntegerString(key);
-
-      case EVR_TM:  // time string
-        return new DcmTime(key);
-
-      case EVR_UI:  // unique identifier
-        return new DcmUniqueIdentifier(key);
-
-      case EVR_ST:  // short text
-        return new DcmShortText(key);
-
-      case EVR_LO:  // long string
-        return new DcmLongString(key);
-
-      case EVR_LT:  // long text
-        return new DcmLongText(key);
-
-      case EVR_UT:  // unlimited text
-        return new DcmUnlimitedText(key);
-
-      case EVR_SH:  // short string
-        return new DcmShortString(key);
-
-      case EVR_PN:  // person name
-        return new DcmPersonName(key);
-
-#if DCMTK_VERSION_NUMBER >= 361
-      case EVR_UC:  // unlimited characters
-        return new DcmUnlimitedCharacters(key);
-#endif
-
-#if DCMTK_VERSION_NUMBER >= 361
-      case EVR_UR:  // URI/URL
-        return new DcmUniversalResourceIdentifierOrLocator(key);
-#endif
-          
-        
-      /**
-       * Numerical types
-       **/ 
-      
-      case EVR_SL:  // signed long
-        return new DcmSignedLong(key);
-
-      case EVR_SS:  // signed short
-        return new DcmSignedShort(key);
-
-      case EVR_UL:  // unsigned long
-        return new DcmUnsignedLong(key);
-
-      case EVR_US:  // unsigned short
-        return new DcmUnsignedShort(key);
-
-      case EVR_FL:  // float single-precision
-        return new DcmFloatingPointSingle(key);
-
-      case EVR_FD:  // float double-precision
-        return new DcmFloatingPointDouble(key);
-
-
-      /**
-       * Sequence types, should never occur at this point.
-       **/
-
-      case EVR_SQ:  // sequence of items
-        throw OrthancException(ErrorCode_ParameterOutOfRange);
-
-
-      /**
-       * TODO
-       **/
-
-      case EVR_AT:  // attribute tag
-        throw OrthancException(ErrorCode_NotImplemented);
-
-
-      /**
-       * Internal to DCMTK.
-       **/ 
-
-      case EVR_xs:  // SS or US depending on context
-      case EVR_lt:  // US, SS or OW depending on context, used for LUT Data (thus the name)
-      case EVR_na:  // na="not applicable", for data which has no VR
-      case EVR_up:  // up="unsigned pointer", used internally for DICOMDIR suppor
-      case EVR_item:  // used internally for items
-      case EVR_metainfo:  // used internally for meta info datasets
-      case EVR_dataset:  // used internally for datasets
-      case EVR_fileFormat:  // used internally for DICOM files
-      case EVR_dicomDir:  // used internally for DICOMDIR objects
-      case EVR_dirRecord:  // used internally for DICOMDIR records
-      case EVR_pixelSQ:  // used internally for pixel sequences in a compressed image
-      case EVR_pixelItem:  // used internally for pixel items in a compressed image
-      case EVR_UNKNOWN: // used internally for elements with unknown VR (encoded with 4-byte length field in explicit VR)
-      case EVR_PixelData:  // used internally for uncompressed pixeld data
-      case EVR_OverlayData:  // used internally for overlay data
-      case EVR_UNKNOWN2B:  // used internally for elements with unknown VR with 2-byte length field in explicit VR
-      default:
-        break;
+    DcmTag key(tag.GetGroup(), tag.GetElement());
+    if (tag.IsPrivate())
+    {
+      return DcmItem::newDicomElement(key, privateCreator.c_str());
+    }
+    else
+    {
+      return DcmItem::newDicomElement(key, NULL);
     }
-
-    throw OrthancException(ErrorCode_InternalError);          
+    
+#else
+    DcmTag key(tag.GetGroup(), tag.GetElement());
+    if (tag.IsPrivate())
+    {
+      // https://forum.dcmtk.org/viewtopic.php?t=4527
+      LOG(WARNING) << "You are using DCMTK <= 3.6.0: All the private tags "
+        "are considered as having a binary value representation";
+      key.setPrivateCreator(privateCreator.c_str());
+      return new DcmOtherByteOtherWord(key);
+    }
+    else
+    {
+      return newDicomElement(key);
+    }
+#endif      
   }
 
 
 
   void FromDcmtkBridge::FillElementWithString(DcmElement& element,
-                                              const DicomTag& tag,
                                               const std::string& utf8Value,
                                               bool decodeDataUriScheme,
                                               Encoding dicomEncoding)
@@ -1578,14 +1443,11 @@
       decoded = &binary;
     }
 
-    DcmTag key(tag.GetGroup(), tag.GetElement());
-
-    if (tag.IsPrivate() ||
-        IsBinaryTag(key))
+    if (IsBinaryTag(element.getTag()))
     {
       bool ok;
 
-      switch (key.getEVR())
+      switch (element.getTag().getEVR())
       {
         case EVR_OW:
           if (decoded->size() % sizeof(Uint16) != 0)
@@ -1619,7 +1481,7 @@
     
     try
     {
-      switch (key.getEVR())
+      switch (element.getTag().getEVR())
       {
         // http://support.dcmtk.org/docs/dcvr_8h-source.html
 
@@ -1628,7 +1490,6 @@
          **/
 
         case EVR_OB:  // other byte
-        case EVR_OF:  // other float
         case EVR_OW:  // other word
         case EVR_AT:  // attribute tag
           throw OrthancException(ErrorCode_NotImplemented);
@@ -1683,6 +1544,9 @@
         }
 
         case EVR_UL:  // unsigned long
+#if DCMTK_VERSION_NUMBER >= 361
+        case EVR_OL:  // other long (requires byte-swapping)
+#endif
         {
           ok = element.putUint32(boost::lexical_cast<Uint32>(*decoded)).good();
           break;
@@ -1695,12 +1559,16 @@
         }
 
         case EVR_FL:  // float single-precision
+        case EVR_OF:  // other float (requires byte swapping)
         {
           ok = element.putFloat32(boost::lexical_cast<float>(*decoded)).good();
           break;
         }
 
         case EVR_FD:  // float double-precision
+#if DCMTK_VERSION_NUMBER >= 361
+        case EVR_OD:  // other double (requires byte-swapping)
+#endif
         {
           ok = element.putFloat64(boost::lexical_cast<double>(*decoded)).good();
           break;
@@ -1750,6 +1618,7 @@
 
     if (!ok)
     {
+      DicomTag tag(element.getTag().getGroup(), element.getTag().getElement());
       throw OrthancException(ErrorCode_BadFileFormat,
                              "While creating a DICOM instance, tag (" + tag.Format() +
                              ") has out-of-range value: \"" + (*decoded) + "\"");
@@ -1760,20 +1629,21 @@
   DcmElement* FromDcmtkBridge::FromJson(const DicomTag& tag,
                                         const Json::Value& value,
                                         bool decodeDataUriScheme,
-                                        Encoding dicomEncoding)
+                                        Encoding dicomEncoding,
+                                        const std::string& privateCreator)
   {
     std::auto_ptr<DcmElement> element;
 
     switch (value.type())
     {
       case Json::stringValue:
-        element.reset(CreateElementForTag(tag));
-        FillElementWithString(*element, tag, value.asString(), decodeDataUriScheme, dicomEncoding);
+        element.reset(CreateElementForTag(tag, privateCreator));
+        FillElementWithString(*element, value.asString(), decodeDataUriScheme, dicomEncoding);
         break;
 
       case Json::nullValue:
-        element.reset(CreateElementForTag(tag));
-        FillElementWithString(*element, tag, "", decodeDataUriScheme, dicomEncoding);
+        element.reset(CreateElementForTag(tag, privateCreator));
+        FillElementWithString(*element, "", decodeDataUriScheme, dicomEncoding);
         break;
 
       case Json::arrayValue:
@@ -1798,7 +1668,7 @@
               Json::Value::Members members = value[i].getMemberNames();
               for (Json::Value::ArrayIndex j = 0; j < members.size(); j++)
               {
-                item->insert(FromJson(ParseTag(members[j]), value[i][members[j]], decodeDataUriScheme, dicomEncoding));
+                item->insert(FromJson(ParseTag(members[j]), value[i][members[j]], decodeDataUriScheme, dicomEncoding, privateCreator));
               }
               break;
             }
@@ -1907,7 +1777,8 @@
   DcmDataset* FromDcmtkBridge::FromJson(const Json::Value& json,  // Encoded using UTF-8
                                         bool generateIdentifiers,
                                         bool decodeDataUriScheme,
-                                        Encoding defaultEncoding)
+                                        Encoding defaultEncoding,
+                                        const std::string& privateCreator)
   {
     std::auto_ptr<DcmDataset> result(new DcmDataset);
     Encoding encoding = ExtractEncoding(json, defaultEncoding);
@@ -1945,7 +1816,7 @@
 
       if (tag != DICOM_TAG_SPECIFIC_CHARACTER_SET)
       {
-        std::auto_ptr<DcmElement> element(FromDcmtkBridge::FromJson(tag, value, decodeDataUriScheme, encoding));
+        std::auto_ptr<DcmElement> element(FromDcmtkBridge::FromJson(tag, value, decodeDataUriScheme, encoding, privateCreator));
         const DcmTagKey& tag = element->getTag();
 
         result->findAndDeleteElement(tag);
@@ -2268,13 +2139,6 @@
      **/
 
     if (evr == EVR_OB ||  // other byte
-        evr == EVR_OF ||  // other float
-#if DCMTK_VERSION_NUMBER >= 361
-        evr == EVR_OD ||  // other double
-#endif
-#if DCMTK_VERSION_NUMBER >= 362
-        evr == EVR_OL ||  // other long
-#endif
         evr == EVR_OW ||  // other word
         evr == EVR_UN)    // unknown value representation
     {
@@ -2464,6 +2328,9 @@
         }
 
         case EVR_UL:  // unsigned long
+#if DCMTK_VERSION_NUMBER >= 362
+        case EVR_OL:
+#endif
         {
           DcmUnsignedLong& content = dynamic_cast<DcmUnsignedLong&>(element);
 
@@ -2504,6 +2371,7 @@
         }
 
         case EVR_FL:  // float single-precision
+        case EVR_OF:
         {
           DcmFloatingPointSingle& content = dynamic_cast<DcmFloatingPointSingle&>(element);
 
@@ -2524,6 +2392,9 @@
         }
 
         case EVR_FD:  // float double-precision
+#if DCMTK_VERSION_NUMBER >= 361
+        case EVR_OD:
+#endif
         {
           DcmFloatingPointDouble& content = dynamic_cast<DcmFloatingPointDouble&>(element);