# HG changeset patch # User Sebastien Jodogne # Date 1763141765 -3600 # Node ID 342980a5192d218cebb34b2c70761dd2c1ef01e3 # Parent 700e04a724656445fd813e2c3cf80b5c3be9d30c exploiting ErrorPayloadType_Dimse diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/DicomNetworking/DicomControlUserConnection.cpp --- a/OrthancFramework/Sources/DicomNetworking/DicomControlUserConnection.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DicomControlUserConnection.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -395,6 +395,7 @@ parameters_.GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(response.DimseStatus) + " (unable to process - invalid query ?)", + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(response.DimseStatus)); } else @@ -402,6 +403,7 @@ throw OrthancException(ErrorCode_NetworkProtocol, "C-FIND SCU to AET \"" + parameters_.GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(response.DimseStatus), + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(response.DimseStatus)); } } @@ -525,6 +527,7 @@ parameters_.GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(response.DimseStatus) + " (unable to process - resource not found ?)", + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(response.DimseStatus)); } else @@ -532,6 +535,7 @@ throw OrthancException(ErrorCode_NetworkProtocol, "C-MOVE SCU to AET \"" + parameters_.GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(response.DimseStatus), + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(response.DimseStatus)); } } @@ -668,6 +672,7 @@ "C-GET SCU to AET \"" + parameters_.GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(rsp.msg.CGetRSP.DimseStatus), + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(rsp.msg.CGetRSP.DimseStatus)); } } diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp --- a/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -465,6 +465,7 @@ "C-STORE SCU to AET \"" + GetParameters().GetRemoteModality().GetApplicationEntityTitle() + "\" has failed with DIMSE status " + DimseToHexString(response.DimseStatus), + ErrorPayloadType_Dimse, MakeDimseErrorStatusPayload(response.DimseStatus)); } } diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.cpp --- a/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -24,34 +24,33 @@ #include "../PrecompiledHeaders.h" #include "DimseErrorPayload.h" -#include + +#include "../SerializationToolbox.h" namespace Orthanc { - static const char* const DIMSE_ERROR_PAYLOAD_TYPE = "DimseErrorPayload"; - static const char* const DIMSE_ERROR_STATUS = "DimseErrorStatus"; // uint16_t + static const char* const DIMSE_ERROR_STATUS = "DimseErrorStatus"; // uint16_t + Json::Value MakeDimseErrorStatusPayload(uint16_t dimseErrorStatus) { Json::Value payload; - payload[ERROR_PAYLOAD_TYPE] = DIMSE_ERROR_PAYLOAD_TYPE; payload[DIMSE_ERROR_STATUS] = dimseErrorStatus; return payload; } - bool IsDimseErrorStatusPayload(const Json::Value& payload) - { - assert(payload.isMember(ERROR_PAYLOAD_TYPE)); - - return payload[ERROR_PAYLOAD_TYPE].asString() == DIMSE_ERROR_PAYLOAD_TYPE; - } uint16_t GetDimseErrorStatusFromPayload(const Json::Value& payload) { - assert(IsDimseErrorStatusPayload(payload)); - assert(payload.isMember(DIMSE_ERROR_STATUS)); + unsigned int status = SerializationToolbox::ReadUnsignedInteger(payload, DIMSE_ERROR_STATUS); - return static_cast(payload[DIMSE_ERROR_STATUS].asUInt()); + if (status <= 65535) + { + return static_cast(status); + } + else + { + throw OrthancException(ErrorCode_BadFileFormat); + } } - } diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h --- a/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h Fri Nov 14 18:36:05 2025 +0100 @@ -35,8 +35,5 @@ { Json::Value MakeDimseErrorStatusPayload(uint16_t dimseErrorStatus); - bool IsDimseErrorStatusPayload(const Json::Value& payload); - uint16_t GetDimseErrorStatusFromPayload(const Json::Value& payload); - } diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/Enumerations.cpp --- a/OrthancFramework/Sources/Enumerations.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/Enumerations.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -1192,6 +1192,19 @@ } + const char* EnumerationToString(ErrorPayloadType type) + { + switch (type) + { + case ErrorPayloadType_Dimse: + return "Dimse"; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + + Encoding StringToEncoding(const char* encoding) { std::string s(encoding); @@ -1913,6 +1926,19 @@ } + ErrorPayloadType StringToErrorPayloadType(const std::string& type) + { + if (type == "Dimse") + { + return ErrorPayloadType_Dimse; + } + else + { + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + + unsigned int GetBytesPerPixel(PixelFormat format) { switch (format) diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/Enumerations.h --- a/OrthancFramework/Sources/Enumerations.h Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/Enumerations.h Fri Nov 14 18:36:05 2025 +0100 @@ -867,6 +867,9 @@ const char* EnumerationToString(RetrieveMethod method); ORTHANC_PUBLIC + const char* EnumerationToString(ErrorPayloadType type); + + ORTHANC_PUBLIC Encoding StringToEncoding(const char* encoding); ORTHANC_PUBLIC @@ -899,6 +902,9 @@ ORTHANC_PUBLIC DicomToJsonFormat StringToDicomToJsonFormat(const std::string& format); + + ORTHANC_PUBLIC + ErrorPayloadType StringToErrorPayloadType(const std::string& type); ORTHANC_PUBLIC bool LookupMimeType(MimeType& target, @@ -921,7 +927,7 @@ bool IsResourceLevelAboveOrEqual(ResourceType level, ResourceType reference); -ORTHANC_PUBLIC + ORTHANC_PUBLIC const char* ResourceTypeToDicomQueryRetrieveLevel(ResourceType type); ORTHANC_PUBLIC diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/JobsEngine/JobInfo.cpp --- a/OrthancFramework/Sources/JobsEngine/JobInfo.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobInfo.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -196,9 +196,13 @@ target["UserData"] = status_.GetUserData(); } - if (status_.HasErrorPayload()) + if (status_.GetErrorPayload().HasContent()) { - target["ErrorPayload"] = status_.GetErrorPayload(); + Json::Value error; + error["Type"] = EnumerationToString(status_.GetErrorPayload().GetType()); + error["Content"] = status_.GetErrorPayload().GetContent(); + + target["ErrorPayload"] = error; } target["Type"] = status_.GetJobType(); diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/JobsEngine/JobStatus.cpp --- a/OrthancFramework/Sources/JobsEngine/JobStatus.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobStatus.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -38,7 +38,15 @@ { } - void JobStatus::InitInternal(const IJob& job) + + JobStatus::JobStatus(ErrorCode code, + const std::string& details, + const IJob& job) : + errorCode_(code), + progress_(job.GetProgress()), + publicContent_(Json::objectValue), + hasSerialized_(false), + details_(details) { if (progress_ < 0) { @@ -57,33 +65,6 @@ hasSerialized_ = job.Serialize(serialized_); } - JobStatus::JobStatus(ErrorCode code, - const std::string& details, - const IJob& job, - const Json::Value& errorPayload) : - errorCode_(code), - progress_(job.GetProgress()), - publicContent_(Json::objectValue), - hasSerialized_(false), - details_(details), - errorPayload_(errorPayload) - { - InitInternal(job); - } - - - JobStatus::JobStatus(ErrorCode code, - const std::string& details, - const IJob& job) : - errorCode_(code), - progress_(job.GetProgress()), - publicContent_(Json::objectValue), - hasSerialized_(false), - details_(details) - { - InitInternal(job); - } - const Json::Value& JobStatus::GetSerialized() const { diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/JobsEngine/JobStatus.h --- a/OrthancFramework/Sources/JobsEngine/JobStatus.h Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobStatus.h Fri Nov 14 18:36:05 2025 +0100 @@ -25,8 +25,7 @@ #pragma once #include "IJob.h" -#include "../OrthancException.h" -#include +#include "../OrthancException.h" // For ErrorPayload namespace Orthanc { @@ -40,21 +39,14 @@ Json::Value serialized_; bool hasSerialized_; std::string details_; - Json::Value errorPayload_; Json::Value userData_; + ErrorPayload errorPayload_; - void InitInternal(const IJob& job); - public: JobStatus(); JobStatus(ErrorCode code, const std::string& details, - const IJob& job, - const Json::Value& errorPayload); - - JobStatus(ErrorCode code, - const std::string& details, const IJob& job); ErrorCode GetErrorCode() const @@ -109,18 +101,13 @@ return userData_; } - bool HasErrorPayload() const + ErrorPayload& GetErrorPayload() { - return !errorPayload_.isNull(); + return errorPayload_; } - const Json::Value& GetErrorPayload() const + const ErrorPayload& GetErrorPayload() const { - if (!HasErrorPayload()) - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); - } - return errorPayload_; } }; diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp --- a/OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -44,6 +44,7 @@ static const char* ERROR_CODE = "ErrorCode"; static const char* ERROR_DETAILS = "ErrorDetails"; static const char* USER_DATA = "UserData"; + static const char* ERROR_PAYLOAD_TYPE = "ErrorPayloadType"; static const char* ERROR_PAYLOAD = "ErrorPayload"; @@ -305,9 +306,10 @@ target[USER_DATA] = userData; } - if (lastStatus_.HasErrorPayload()) + if (lastStatus_.GetErrorPayload().HasContent()) { - target[ERROR_PAYLOAD] = lastStatus_.GetErrorPayload(); + target[ERROR_PAYLOAD_TYPE] = lastStatus_.GetErrorPayload().GetType(); + target[ERROR_PAYLOAD] = lastStatus_.GetErrorPayload().GetContent(); } return true; @@ -361,13 +363,13 @@ job_->SetUserData(serialized[USER_DATA]); } - if (serialized.isMember(ERROR_PAYLOAD)) + lastStatus_ = JobStatus(errorCode, details, *job_); + + if (serialized.isMember(ERROR_PAYLOAD_TYPE) && + serialized.isMember(ERROR_PAYLOAD)) { - lastStatus_ = JobStatus(errorCode, details, *job_, serialized[ERROR_PAYLOAD]); - } - else - { - lastStatus_ = JobStatus(errorCode, details, *job_); + const std::string type = SerializationToolbox::ReadString(serialized, ERROR_PAYLOAD_TYPE); + lastStatus_.GetErrorPayload().SetContent(StringToErrorPayloadType(type.c_str()), serialized[ERROR_PAYLOAD]); } job_->Start(); @@ -902,11 +904,12 @@ if (jobErrorPayload.HasContent()) { - throw OrthancException(code, details, jobErrorPayload.GetContent()); + throw OrthancException(code, details, jobErrorPayload.GetType(), jobErrorPayload.GetContent()); } - else if (it->second->GetLastStatus().HasErrorPayload()) + else if (it->second->GetLastStatus().GetErrorPayload().HasContent()) { - throw OrthancException(code, details, it->second->GetLastStatus().GetErrorPayload()); + throw OrthancException(code, details, it->second->GetLastStatus().GetErrorPayload().GetType(), + it->second->GetLastStatus().GetErrorPayload().GetContent()); } else if (!details.empty()) { @@ -1481,7 +1484,8 @@ } else { - JobStatus status(code, details, *job_, errorPayload); + JobStatus status(code, details, *job_); + status.GetErrorPayload().SetContent(ErrorPayloadType_None /* TODO PAYLOAD */, errorPayload); boost::mutex::scoped_lock lock(registry_.mutex_); registry_.CheckInvariants(); diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/OrthancException.cpp --- a/OrthancFramework/Sources/OrthancException.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/OrthancException.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -40,6 +40,28 @@ } + ErrorPayload& ErrorPayload::operator= (const ErrorPayload& other) + { + if (other.HasContent()) + { + SetContent(other.GetType(), other.GetContent()); + } + else + { + ClearContent(); + } + + return *this; + } + + + void ErrorPayload::ClearContent() + { + type_ = ErrorPayloadType_None; + content_.reset(NULL); + } + + void ErrorPayload::SetContent(ErrorPayloadType type, const Json::Value& content) { @@ -85,7 +107,8 @@ OrthancException::OrthancException(const OrthancException& other) : errorCode_(other.errorCode_), httpStatus_(other.httpStatus_), - logged_(false) + logged_(false), + payloadType_(ErrorPayloadType_None) { if (other.details_.get() != NULL) { @@ -102,7 +125,8 @@ OrthancException::OrthancException(ErrorCode errorCode) : errorCode_(errorCode), httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)), - logged_(false) + logged_(false), + payloadType_(ErrorPayloadType_None) { } @@ -112,7 +136,8 @@ errorCode_(errorCode), httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)), logged_(log == LogException_Yes), - details_(new std::string(details)) + details_(new std::string(details)), + payloadType_(ErrorPayloadType_None) { #if ORTHANC_ENABLE_LOGGING == 1 if (log == LogException_Yes) @@ -127,19 +152,22 @@ HttpStatus httpStatus) : errorCode_(errorCode), httpStatus_(httpStatus), - logged_(false) + logged_(false), + payloadType_(ErrorPayloadType_None) { } OrthancException::OrthancException(ErrorCode errorCode, const std::string& details, + ErrorPayloadType payloadType, const Json::Value& payload, LogException log) : errorCode_(errorCode), httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)), logged_(log == LogException_Yes), - details_(new std::string(details)) + details_(new std::string(details)), + payloadType_(payloadType) { payload_.reset(new Json::Value(payload)); @@ -155,12 +183,14 @@ OrthancException::OrthancException(ErrorCode errorCode, HttpStatus httpStatus, const std::string& details, + ErrorPayloadType payloadType, const Json::Value& payload, LogException log) : errorCode_(errorCode), httpStatus_(httpStatus), logged_(log == LogException_Yes), - details_(new std::string(details)) + details_(new std::string(details)), + payloadType_(payloadType) { payload_.reset(new Json::Value(payload)); @@ -179,7 +209,8 @@ errorCode_(errorCode), httpStatus_(httpStatus), logged_(log == LogException_Yes), - details_(new std::string(details)) + details_(new std::string(details)), + payloadType_(ErrorPayloadType_None) { #if ORTHANC_ENABLE_LOGGING == 1 if (log == LogException_Yes) diff -r 700e04a72465 -r 342980a5192d OrthancFramework/Sources/OrthancException.h --- a/OrthancFramework/Sources/OrthancException.h Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancFramework/Sources/OrthancException.h Fri Nov 14 18:36:05 2025 +0100 @@ -33,8 +33,6 @@ namespace Orthanc { - static const char* const ERROR_PAYLOAD_TYPE = "Type"; - // From 1.12.10, we use this enumeration instead of a bool to avoid implicit // conversions to bool in OrthancException constructors because any type is // automaticaly casted to a bool without a single warning which led to wrong @@ -49,8 +47,6 @@ class ORTHANC_PUBLIC ErrorPayload { private: - ErrorPayload& operator= (const ErrorPayload&); // Forbidden - ErrorPayloadType type_; std::unique_ptr content_; @@ -62,6 +58,10 @@ explicit ErrorPayload(const ErrorPayload& other); + ErrorPayload& operator= (const ErrorPayload& other); + + void ClearContent(); + void SetContent(ErrorPayloadType type, const Json::Value& content); @@ -91,6 +91,7 @@ std::unique_ptr details_; // New in Orthanc 1.12.10 + ErrorPayloadType payloadType_; std::unique_ptr payload_; public: @@ -104,6 +105,7 @@ OrthancException(ErrorCode errorCode, const std::string& details, + ErrorPayloadType type, const Json::Value& payload, LogException log = LogException_Yes); @@ -119,6 +121,7 @@ OrthancException(ErrorCode errorCode, HttpStatus httpStatus, const std::string& details, + ErrorPayloadType type, const Json::Value& payload, LogException log = LogException_Yes); @@ -136,6 +139,11 @@ bool HasPayload() const; + ErrorPayloadType GetPayloadType() const + { + return payloadType_; + } + const Json::Value& GetPayload() const; }; } diff -r 700e04a72465 -r 342980a5192d OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp --- a/OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp Fri Nov 14 17:54:51 2025 +0100 +++ b/OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp Fri Nov 14 18:36:05 2025 +0100 @@ -101,7 +101,8 @@ } catch (OrthancException& e) { - if (e.HasPayload() && IsDimseErrorStatusPayload(e.GetPayload())) + if (e.HasPayload() && + e.GetPayloadType() == ErrorPayloadType_Dimse) { dimseErrorStatus_ = GetDimseErrorStatusFromPayload(e.GetPayload()); }