changeset 6499:8fc86cc3b31c

added safeguards
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 25 Nov 2025 20:46:17 +0100
parents cde9c325f25e
children 0c3e38427616
files OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.cpp OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.h OrthancFramework/Sources/DicomNetworking/IFindRequestHandler.h OrthancFramework/Sources/DicomNetworking/IGetRequestHandler.h OrthancFramework/Sources/DicomNetworking/IMoveRequestHandler.h OrthancFramework/Sources/DicomNetworking/IStorageCommitmentRequestHandler.h OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp OrthancFramework/Sources/DicomNetworking/Internals/MoveScp.cpp OrthancServer/Plugins/Engine/OrthancPlugins.cpp OrthancServer/Plugins/Engine/OrthancPlugins.h OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp OrthancServer/Resources/RunCppCheck-2.17.1.sh OrthancServer/Sources/ServerJobs/IStorageCommitmentFactory.h OrthancServer/Sources/ServerJobs/StorageCommitmentScpJob.cpp
diffstat 16 files changed, 85 insertions(+), 49 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -37,7 +37,7 @@
   static const char* const REMOTE_AET = "RemoteAet";
   static const char* const REMOTE_IP = "RemoteIp";
   static const char* const MANUFACTURER = "Manufacturer";
-  
+
   void DicomConnectionInfo::Serialize(Json::Value& target) const
   {
     if (target.type() != Json::objectValue)
@@ -62,5 +62,4 @@
     std::string manufacturer = SerializationToolbox::ReadString(serialized, MANUFACTURER);
     manufacturer_ = StringToModalityManufacturer(manufacturer);
   }
-
-}
\ No newline at end of file
+}
--- a/OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.h	Tue Nov 25 20:46:17 2025 +0100
@@ -25,8 +25,9 @@
 #pragma once
 
 #include "../Compatibility.h"
+#include "../Enumerations.h"
+
 #include <json/value.h>
-#include "../Enumerations.h"
 
 namespace Orthanc
 {
@@ -41,9 +42,8 @@
     ModalityManufacturer      manufacturer_;
 
   public:
-    
     DicomConnectionInfo(const std::string& remoteIp,
-                        const std::string& remoteAet,                  
+                        const std::string& remoteAet,
                         const std::string& calledAet,
                         const ModalityManufacturer& manufacturer) :
       remoteIp_(remoteIp),
@@ -54,7 +54,7 @@
     }
 
     DicomConnectionInfo(const std::string& remoteIp,
-                        const std::string& remoteAet,                  
+                        const std::string& remoteAet,
                         const std::string& calledAet) :
       remoteIp_(remoteIp),
       remoteAet_(remoteAet),
--- a/OrthancFramework/Sources/DicomNetworking/IFindRequestHandler.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/IFindRequestHandler.h	Tue Nov 25 20:46:17 2025 +0100
@@ -24,14 +24,13 @@
 
 #pragma once
 
+#include "DicomConnectionInfo.h"
 #include "DicomFindAnswers.h"
 
 #include <list>
 
 namespace Orthanc
 {
-  class DicomConnectionInfo;
-
   class IFindRequestHandler : public boost::noncopyable
   {
   public:
--- a/OrthancFramework/Sources/DicomNetworking/IGetRequestHandler.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/IGetRequestHandler.h	Tue Nov 25 20:46:17 2025 +0100
@@ -24,10 +24,9 @@
 
 #pragma once
 
-#include <dcmtk/dcmnet/assoc.h>
-
 #include "../DicomFormat/DicomMap.h"
 
+#include <dcmtk/dcmnet/assoc.h>
 #include <string>
 
 
--- a/OrthancFramework/Sources/DicomNetworking/IMoveRequestHandler.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/IMoveRequestHandler.h	Tue Nov 25 20:46:17 2025 +0100
@@ -24,6 +24,7 @@
 
 #pragma once
 
+#include "DicomConnectionInfo.h"
 #include "../DicomFormat/DicomMap.h"
 
 #include <vector>
@@ -32,8 +33,6 @@
 
 namespace Orthanc
 {
-  class DicomConnectionInfo;
-
   class IMoveRequestIterator : public boost::noncopyable
   {
   public:
--- a/OrthancFramework/Sources/DicomNetworking/IStorageCommitmentRequestHandler.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/IStorageCommitmentRequestHandler.h	Tue Nov 25 20:46:17 2025 +0100
@@ -24,14 +24,14 @@
 
 #pragma once
 
+#include "DicomConnectionInfo.h"
+
 #include <boost/noncopyable.hpp>
 #include <string>
 #include <vector>
 
 namespace Orthanc
 {
-  class DicomConnectionInfo;
-
   class IStorageCommitmentRequestHandler : public boost::noncopyable
   {
   public:
--- a/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -83,7 +83,6 @@
 #include "../../Logging.h"
 #include "../../OrthancException.h"
 #include "../../Toolbox.h"
-#include "../DicomConnectionInfo.h"
 #include "FindScp.h"
 #include "GetScp.h"
 #include "MoveScp.h"
--- a/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -78,7 +78,6 @@
 #include "../../DicomFormat/DicomArray.h"
 #include "../../DicomParsing/FromDcmtkBridge.h"
 #include "../../DicomParsing/ToDcmtkBridge.h"
-#include "../DicomConnectionInfo.h"
 #include "../../Logging.h"
 #include "../../OrthancException.h"
 
--- a/OrthancFramework/Sources/DicomNetworking/Internals/MoveScp.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/MoveScp.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -78,7 +78,6 @@
 
 #include "../../DicomParsing/FromDcmtkBridge.h"
 #include "../../DicomParsing/ToDcmtkBridge.h"
-#include "../DicomConnectionInfo.h"
 #include "../../Logging.h"
 #include "../../OrthancException.h"
 
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -1584,6 +1584,11 @@
           {
             parameters_->destructor(handler_);
           }
+          else
+          {
+            assert(0);  // Don't throw exceptions in destructors
+          }
+
           handler_ = NULL;
         }
 
@@ -1603,7 +1608,11 @@
           {
             error = parameters_->lookup(&reason, handler_, sopClassUid.c_str(), sopInstanceUid.c_str());
           }
-           
+          else
+          {
+            throw OrthancException(ErrorCode_InternalError);
+          }
+
           if (error == OrthancPluginErrorCode_Success)
           {
             return Plugins::Convert(reason);
@@ -1670,6 +1679,10 @@
             a.empty() ? NULL : &a[0], b.empty() ? NULL : &b[0], static_cast<uint32_t>(n),
             connection.GetRemoteAet().c_str(), connection.GetCalledAet().c_str());
         }
+        else
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
 
         if (error != OrthancPluginErrorCode_Success)
         {
@@ -1929,6 +1942,10 @@
             throw OrthancException(static_cast<ErrorCode>(error));
           }
         }
+        else
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
 
         Reset();
       }
@@ -2020,7 +2037,8 @@
             (reinterpret_cast<OrthancPluginFindAnswers*>(&answers),
              reinterpret_cast<const OrthancPluginFindQuery*>(this),
              reinterpret_cast<const OrthancPluginDicomConnection*>(&connection));
-        } else if (that_.pimpl_->findCallback_)
+        }
+        else if (that_.pimpl_->findCallback_)
         {
           error = that_.pimpl_->findCallback_
             (reinterpret_cast<OrthancPluginFindAnswers*>(&answers),
@@ -2028,6 +2046,10 @@
              connection.GetRemoteAet().c_str(),
              connection.GetCalledAet().c_str());
         }
+        else
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
 
         if (error != OrthancPluginErrorCode_Success)
         {
@@ -2202,6 +2224,10 @@
           throw OrthancException(ErrorCode_Plugin);
         }
       }
+      else
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
     }
 
     virtual IMoveRequestIterator* Handle(const std::string& targetAet,
@@ -2249,6 +2275,10 @@
                                    targetAet.c_str(),
                                    originatorId);
       }
+      else
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
 
       if (driver == NULL)
       {
@@ -2262,12 +2292,16 @@
 
         return new Driver(driver, size, params2_->applyMove, params2_->freeMove);
       }
-      else
+      else if (params_.get() != NULL)
       {
         unsigned int size = params_->getMoveSize(driver);
 
         return new Driver(driver, size, params_->applyMove, params_->freeMove);
       }
+      else
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
     }
   };
 
@@ -3044,7 +3078,8 @@
 
     boost::mutex::scoped_lock lock(pimpl_->worklistCallbackMutex_);
 
-    if (pimpl_->worklistCallback_ != NULL || pimpl_->worklistCallback2_ != NULL)
+    if (pimpl_->worklistCallback_ != NULL ||
+        pimpl_->worklistCallback2_ != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle modality worklists");
@@ -3064,7 +3099,8 @@
 
     boost::mutex::scoped_lock lock(pimpl_->worklistCallbackMutex_);
 
-    if (pimpl_->worklistCallback_ != NULL || pimpl_->worklistCallback2_ != NULL)
+    if (pimpl_->worklistCallback_ != NULL ||
+        pimpl_->worklistCallback2_ != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle modality worklists");
@@ -3084,7 +3120,8 @@
 
     boost::mutex::scoped_lock lock(pimpl_->findCallbackMutex_);
 
-    if (pimpl_->findCallback2_ != NULL || pimpl_->findCallback_ != NULL)
+    if (pimpl_->findCallback2_ != NULL ||
+        pimpl_->findCallback_ != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle C-FIND requests");
@@ -3104,7 +3141,8 @@
 
     boost::mutex::scoped_lock lock(pimpl_->findCallbackMutex_);
 
-    if (pimpl_->findCallback2_ != NULL || pimpl_->findCallback_ != NULL)
+    if (pimpl_->findCallback2_ != NULL ||
+        pimpl_->findCallback_ != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle C-FIND requests");
@@ -3124,7 +3162,8 @@
     const _OrthancPluginMoveCallback& p = 
       *reinterpret_cast<const _OrthancPluginMoveCallback*>(parameters);
 
-    if (pimpl_->moveCallbacks_.get() != NULL || pimpl_->moveCallbacks2_.get() != NULL)
+    if (pimpl_->moveCallbacks_.get() != NULL ||
+        pimpl_->moveCallbacks2_.get() != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle C-MOVE requests");
@@ -3144,7 +3183,8 @@
     const _OrthancPluginMoveCallback2& p = 
       *reinterpret_cast<const _OrthancPluginMoveCallback2*>(parameters);
 
-    if (pimpl_->moveCallbacks_.get() != NULL || pimpl_->moveCallbacks2_.get() != NULL)
+    if (pimpl_->moveCallbacks_.get() != NULL ||
+        pimpl_->moveCallbacks2_.get() != NULL)
     {
       throw OrthancException(ErrorCode_Plugin,
                              "Can only register one plugin to handle C-MOVE requests");
@@ -5395,9 +5435,9 @@
       case _OrthancPluginService_GetConnectionRemoteIp:
       case _OrthancPluginService_GetConnectionCalledAet:
         AccessDicomConnection(service, parameters);
-        return true;        
-      
-        case _OrthancPluginService_SetGlobalProperty:
+        return true;
+
+      case _OrthancPluginService_SetGlobalProperty:
       {
         const _OrthancPluginGlobalProperty& p = 
           *reinterpret_cast<const _OrthancPluginGlobalProperty*>(parameters);
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Plugins/Engine/OrthancPlugins.h	Tue Nov 25 20:46:17 2025 +0100
@@ -184,7 +184,7 @@
 
     void AccessDicomConnection(_OrthancPluginService service,
                                const void* parameters);
-                              
+
     void SendHttpStatusCode(const void* parameters);
 
     void SendHttpStatus(const void* parameters);
--- a/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h	Tue Nov 25 20:46:17 2025 +0100
@@ -676,7 +676,7 @@
     _OrthancPluginService_GetConnectionRemoteAet = 10000,  /* New in SDK 1.12.10 */
     _OrthancPluginService_GetConnectionRemoteIp = 10001,   /* New in SDK 1.12.10 */
     _OrthancPluginService_GetConnectionCalledAet = 10002,  /* New in SDK 1.12.10 */
-    
+
     _OrthancPluginService_INTERNAL = 0x7fffffff
   } _OrthancPluginService;
 
--- a/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -2720,14 +2720,17 @@
 
         return;
       }
-      else if (state == "Running" || state == "Pending" || state == "Paused" || state == "Retry")
+      else if (state == "Running" ||
+               state == "Pending" ||
+               state == "Paused" ||
+               state == "Retry")
       {
         continue;
       }
       else if (state == "Failure")
       {
         if (!status.isMember("ErrorCode") ||
-                status["ErrorCode"].type() != Json::intValue)
+            status["ErrorCode"].type() != Json::intValue)
         {
           ORTHANC_PLUGINS_THROW_PLUGIN_ERROR_CODE(OrthancPluginErrorCode_InternalError);
         }
--- a/OrthancServer/Resources/RunCppCheck-2.17.1.sh	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Resources/RunCppCheck-2.17.1.sh	Tue Nov 25 20:46:17 2025 +0100
@@ -17,7 +17,7 @@
 constParameterCallback:../../OrthancFramework/Sources/DicomNetworking/Internals/StoreScp.cpp:112
 constParameterCallback:../../OrthancFramework/Sources/DicomNetworking/Internals/StoreScp.cpp:113
 constParameterCallback:../../OrthancFramework/Sources/Pkcs11.cpp:125
-constParameterCallback:../../OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp:3500
+constParameterCallback:../../OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp:3503
 constParameterCallback:../../OrthancServer/Sources/OrthancGetRequestHandler.cpp:47
 constParameterPointer:../../OrthancFramework/Sources/Logging.cpp:447
 constParameterPointer:../../OrthancFramework/Sources/Logging.cpp:451
@@ -26,19 +26,19 @@
 cstyleCast:../../OrthancServer/Plugins/Engine/PluginsManager.cpp:124
 cstyleCast:../../OrthancServer/Plugins/Engine/PluginsManager.cpp:140
 cstyleCast:../../OrthancServer/Plugins/Engine/PluginsManager.cpp:85
-knownConditionTrueFalse:../../OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp:115
+knownConditionTrueFalse:../../OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp:114
 knownConditionTrueFalse:../../OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.cpp:425
 knownConditionTrueFalse:../../OrthancFramework/Sources/JobsEngine/Operations/SequenceOfOperationsJob.cpp:345
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2404
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2405
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2406
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2407
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2408
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2409
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2410
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2411
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2412
-knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2413
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2438
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2439
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2440
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2441
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2442
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2443
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2444
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2445
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2446
+knownConditionTrueFalse:../../OrthancServer/Plugins/Engine/OrthancPlugins.cpp:2447
 nullPointer:../../OrthancFramework/UnitTestsSources/RestApiTests.cpp:321
 stlFindInsert:../../OrthancFramework/Sources/RestApi/RestApiCallDocumentation.cpp:166
 stlFindInsert:../../OrthancFramework/Sources/RestApi/RestApiCallDocumentation.cpp:74
--- a/OrthancServer/Sources/ServerJobs/IStorageCommitmentFactory.h	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Sources/ServerJobs/IStorageCommitmentFactory.h	Tue Nov 25 20:46:17 2025 +0100
@@ -23,13 +23,13 @@
 
 #pragma once
 
+#include "../../OrthancFramework/Sources/DicomNetworking/DicomConnectionInfo.h"
+
 #include <string>
 #include <vector>
 
 namespace Orthanc
 {
-  class DicomConnectionInfo;
-
   class IStorageCommitmentFactory : public boost::noncopyable
   {
   public:
--- a/OrthancServer/Sources/ServerJobs/StorageCommitmentScpJob.cpp	Tue Nov 25 17:54:34 2025 +0100
+++ b/OrthancServer/Sources/ServerJobs/StorageCommitmentScpJob.cpp	Tue Nov 25 20:46:17 2025 +0100
@@ -433,7 +433,7 @@
       std::string calledAet = SerializationToolbox::ReadString(serialized, CALLED_AET);
       std::string remoteAet = SerializationToolbox::ReadString(serialized[REMOTE_MODALITY], "AET");
       std::string remoteIp = SerializationToolbox::ReadString(serialized[REMOTE_MODALITY], "Host");
-      connection_.reset(new DicomConnectionInfo(remoteIp, remoteAet, calledAet));      
+      connection_.reset(new DicomConnectionInfo(remoteIp, remoteAet, calledAet));
     } 
 
     if (connection_.get() == NULL)