Mercurial > hg > orthanc-transfers
changeset 84:3a460db3e6cb
added upport for cookies in push jobs (by James Manners)
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Mon, 19 May 2025 17:00:51 +0200 |
| parents | e90266cc3d9b |
| children | b1d9e2310431 |
| files | AUTHORS Framework/PushMode/PushJob.cpp Framework/TransferToolbox.cpp Framework/TransferToolbox.h NEWS Resources/Orthanc/Plugins/OrthancPluginCppWrapper.cpp Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h |
| diffstat | 7 files changed, 265 insertions(+), 53 deletions(-) [+] |
line wrap: on
line diff
--- a/AUTHORS Mon May 12 13:02:53 2025 +0200 +++ b/AUTHORS Mon May 19 17:00:51 2025 +0200 @@ -25,3 +25,6 @@ 1348 Ottignies-Louvain-la-Neuve Belgium https://uclouvain.be/icteam + +* James Manners + Australia
--- a/Framework/PushMode/PushJob.cpp Mon May 12 13:02:53 2025 +0200 +++ b/Framework/PushMode/PushJob.cpp Mon May 19 17:00:51 2025 +0200 @@ -25,12 +25,78 @@ #include "../HttpQueries/HttpQueriesRunner.h" #include "../TransferScheduler.h" -#include <Compatibility.h> // For std::unique_ptr +#include <boost/algorithm/string.hpp> // For boost::iequals and boost::split +#include <Compatibility.h> // For std::unique_ptr #include <Logging.h> - namespace OrthancPlugins { + + /** + * This is a helper function to extract cookie name and value into a single + * string from the response http headers. The extracted cookie string can + * then be used to set the "Cookie" header in subsequent requests. + * + * Orthanc appears not to support multiple "Set-Cookie" headers in the + * response, so only the last `set-cookie` header is included in the headers + * map. This means that if the server responds with multiple cookies, only the + * last one will be extracted. + * + * This is a very simlified implementation for use by the Accelerator plugin. + * It is not a substitute for a full cookie jar implementation. However, with + * the Authenication plugin, a simplified implementation where any cookie from + * the initial request can be used in the finite number of subsequent + * requests is sufficient. + */ + static std::string ExtractCookiesFromHeaders(const std::map<std::string, std::string> &headers) + { + // an array of cookies returned by the server + std::vector<std::string> cookies; + + for (const auto &header : headers) + { + auto headerName = header.first; + auto headerValue = header.second; + + // Check if the header is a Set-Cookie header (case-insensitive) + if (boost::iequals(headerName, "set-cookie")) + { + // Set-Cookie headers are formatted as: + // Set-Cookie: <cookie-name>=<cookie-value>; <attributes> + // or + // Set-Cookie: <cookie-name>=<cookie-value> + // + // We only need the cookie name and value, so we split on ';' + // and take the first part + + // Split the cookie string by ';' and take the first part (the actual cookie) + std::vector<std::string> tokens; + Orthanc::Toolbox::SplitString(tokens, headerValue, ';'); + + if(!tokens.empty()) + { + std::string cookie = tokens[0]; + std::string trimmedCookie = boost::trim_copy(cookie); + cookies.push_back(trimmedCookie); + } + } + } + + // Join all cookies with "; " + std::ostringstream result; + for (size_t i = 0; i < cookies.size(); ++i) + { + if (i > 0) + { + result << "; "; + } + + result << cookies[i]; + } + + return result.str(); + } + class PushJob::FinalState : public IState { private: @@ -38,16 +104,24 @@ JobInfo& info_; std::string transactionUri_; bool isCommit_; - + /** + * Stores any cookies to be sent in the http request. These + * cookies are obtained from the response headers of the + * initialisation request for the push job. + */ + std::string cookieHeader_; + public: FinalState(const PushJob& job, JobInfo& info, const std::string& transactionUri, - bool isCommit) : + bool isCommit, + const std::string &cookieHeader) : job_(job), info_(info), transactionUri_(transactionUri), - isCommit_(isCommit) + isCommit_(isCommit), + cookieHeader_(cookieHeader) { } @@ -58,6 +132,11 @@ std::map<std::string, std::string> headers; job_.query_.GetHttpHeaders(headers); + if (!cookieHeader_.empty()) + { + headers["Cookie"] = cookieHeader_; + } + if (isCommit_) { success = DoPostPeer(answer, job_.peers_, job_.peerIndex_, transactionUri_ + "/commit", "", job_.maxHttpRetries_, headers, job_.commitTimeout_); @@ -76,7 +155,7 @@ } return StateUpdate::Failure(); - } + } else if (isCommit_) { return StateUpdate::Success(); @@ -96,11 +175,17 @@ class PushJob::PushBucketsState : public IState { private: - const PushJob& job_; - JobInfo& info_; - std::string transactionUri_; - HttpQueriesQueue queue_; - std::unique_ptr<HttpQueriesRunner> runner_; + const PushJob& job_; + JobInfo& info_; + std::string transactionUri_; + HttpQueriesQueue queue_; + std::unique_ptr<HttpQueriesRunner> runner_; + /** + * Stores any cookies to be sent in the http request. These + * cookies are obtained from the response headers of the + * initialisation request for the push job. + */ + std::string cookieHeader_; void UpdateInfo() { @@ -128,15 +213,21 @@ PushBucketsState(const PushJob& job, JobInfo& info, const std::string& transactionUri, - const std::vector<TransferBucket>& buckets) : + const std::vector<TransferBucket>& buckets, + const std::string& cookieHeader) : job_(job), info_(info), - transactionUri_(transactionUri) + transactionUri_(transactionUri), + cookieHeader_(cookieHeader) { std::map<std::string, std::string> headers; job_.query_.GetHttpHeaders(headers); headers["Content-Type"] = "application/octet-stream"; + if (!cookieHeader_.empty()) + { + headers["Cookie"] = cookieHeader_; + } queue_.SetMaxRetries(job.maxHttpRetries_); queue_.Reserve(buckets.size()); @@ -168,11 +259,11 @@ case HttpQueriesQueue::Status_Success: // Commit transaction on remote peer - return StateUpdate::Next(new FinalState(job_, info_, transactionUri_, true)); + return StateUpdate::Next(new FinalState(job_, info_, transactionUri_, true, cookieHeader_)); case HttpQueriesQueue::Status_Failure: // Discard transaction on remote peer - return StateUpdate::Next(new FinalState(job_, info_, transactionUri_, false)); + return StateUpdate::Next(new FinalState(job_, info_, transactionUri_, false, cookieHeader_)); default: throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); @@ -222,11 +313,12 @@ { Json::Value answer; std::map<std::string, std::string> headers; + std::map<std::string, std::string> answerHeaders; job_.query_.GetHttpHeaders(headers); headers["Content-Type"] = "application/json"; - if (!DoPostPeer(answer, job_.peers_, job_.peerIndex_, URI_PUSH, createTransaction_, job_.maxHttpRetries_, headers)) + if (!DoPostPeer(answer, answerHeaders, job_.peers_, job_.peerIndex_, URI_PUSH, createTransaction_, job_.maxHttpRetries_, headers, job_.commitTimeout_)) { LOG(ERROR) << "Cannot create a push transaction to peer \"" << job_.query_.GetPeer() @@ -243,8 +335,34 @@ } std::string transactionUri = answer[KEY_PATH].asString(); + /** + * Some load balancers such as AWS Application Load Balancer use Sticky + * Session Cookies, which are set by the load balancer on a request. If + * subsequent requests include these cookies, then the load balancer will + * route the request to the same backend server. + * + * This is important for the Accelerated Transfers plugin as the push + * transactions are statefull, meaning all requests (initialisation, each + * bucket and commit) must be sent to the same backend server, otherwise + * the transfer job will fail. + * + * In order to support cookie based sticky sessions, we need to extract the + * cookies from the initilisation request and include them in the + * subsequent requests for this transfer job.answerHeaders + * + * Currently, the answerHeaders maps only contains 1 Set-Cookie headers + * (the last one). This is a limitation of the current HttpClient + * implementation. Meaning that any subsequent requests in this transfer + * job will only include the last `Set-Cookie` header from the initial + * request. + * + * The cookieHeader is passed into each subsequent JobStates + * (PushBucketsState and FinalState) so that the cookies are included in + * the headers of each request. + */ + std::string cookieHeader = ExtractCookiesFromHeaders(answerHeaders); - return StateUpdate::Next(new PushBucketsState(job_, info_, transactionUri, buckets_)); + return StateUpdate::Next(new PushBucketsState(job_, info_, transactionUri, buckets_, cookieHeader)); } virtual void Stop(OrthancPluginJobStopReason reason)
--- a/Framework/TransferToolbox.cpp Mon May 12 13:02:53 2025 +0200 +++ b/Framework/TransferToolbox.cpp Mon May 19 17:00:51 2025 +0200 @@ -90,13 +90,37 @@ unsigned int timeout ) { + std::map<std::string, std::string> answerHeadersNotUsed; + return DoPostPeer(answer, + answerHeadersNotUsed, + peers, + peerIndex, + uri, + body, + maxRetries, + headers, + timeout); + } + + + bool DoPostPeer(Json::Value& answer, + std::map<std::string, std::string>& answerHeaders, + const OrthancPeers& peers, + size_t peerIndex, + const std::string& uri, + const std::string& body, + unsigned int maxRetries, + const std::map<std::string, std::string>& headers, + unsigned int timeout +) + { unsigned int retry = 0; for (;;) { try { - if (peers.DoPost(answer, peerIndex, uri, body, headers, timeout)) + if (peers.DoPost(answer, answerHeaders, peerIndex, uri, body, headers, timeout)) { return true; }
--- a/Framework/TransferToolbox.h Mon May 12 13:02:53 2025 +0200 +++ b/Framework/TransferToolbox.h Mon May 19 17:00:51 2025 +0200 @@ -98,6 +98,16 @@ unsigned int timeout); bool DoPostPeer(Json::Value& answer, + std::map<std::string, std::string>& answerHeaders, + const OrthancPeers& peers, + size_t peerIndex, + const std::string& uri, + const std::string& body, + unsigned int maxRetries, + const std::map<std::string, std::string>& headers, + unsigned int timeout); + + bool DoPostPeer(Json::Value& answer, const OrthancPeers& peers, const std::string& peerName, const std::string& uri,
--- a/NEWS Mon May 12 13:02:53 2025 +0200 +++ b/NEWS Mon May 19 17:00:51 2025 +0200 @@ -4,6 +4,12 @@ Minimum plugin SDK (for build): 1.12.1 Minimum Orthanc runtime: 1.12.1 +* For "Push" jobs, if the initial response from the remote peer contains a cookie, this + cookie is copied in all subsequent requests to the same peer for this push job: + Some load balancers such as AWS Application Load Balancer use Sticky + Session Cookies, which are set by the load balancer on a request. If + subsequent requests include these cookies, then the load balancer will + route the request to the same backend server. * now setting "Content-Type" HTTP headers in outgoing POST requests. (https://discourse.orthanc-server.org/t/transfer-accelerator-plugin-http-header-compatibility/5725) * new "PeerCommitTimeout" configuration to configure the HTTP Timeout when committing
--- a/Resources/Orthanc/Plugins/OrthancPluginCppWrapper.cpp Mon May 12 13:02:53 2025 +0200 +++ b/Resources/Orthanc/Plugins/OrthancPluginCppWrapper.cpp Mon May 19 17:00:51 2025 +0200 @@ -327,6 +327,34 @@ } } + static void DecodeHttpHeaders(HttpHeaders& target, + const MemoryBuffer& source) + { + Json::Value v; + source.ToJson(v); + + if (v.type() != Json::objectValue) + { + ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError); + } + + Json::Value::Members members = v.getMemberNames(); + target.clear(); + + for (size_t i = 0; i < members.size(); i++) + { + const Json::Value& h = v[members[i]]; + if (h.type() != Json::stringValue) + { + ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError); + } + else + { + target[members[i]] = h.asString(); + } + } + } + // helper class to convert std::map of headers to the plugin SDK C structure class PluginHttpHeaders { @@ -2060,8 +2088,30 @@ unsigned int timeout) const { MemoryBuffer buffer; - - if (DoPost(buffer, index, uri, body, headers, timeout)) + HttpHeaders answerHeaders; + + if (DoPost(buffer, answerHeaders, index, uri, body, headers, timeout)) + { + buffer.ToJson(target); + return true; + } + else + { + return false; + } + } + + bool OrthancPeers::DoPost(Json::Value& target, + HttpHeaders& answerHeaders, + size_t index, + const std::string& uri, + const std::string& body, + const HttpHeaders& headers, + unsigned int timeout) const + { + MemoryBuffer buffer; + + if (DoPost(buffer, answerHeaders, index, uri, body, headers, timeout)) { buffer.ToJson(target); return true; @@ -2119,10 +2169,10 @@ const std::string& body, const HttpHeaders& headers) const { - return DoPost(target, index, uri, body, headers, timeout_); + HttpHeaders answerHeaders; + return DoPost(target, answerHeaders, index, uri, body, headers, timeout_); } - bool OrthancPeers::DoPost(MemoryBuffer& target, size_t index, const std::string& uri, @@ -2130,6 +2180,18 @@ const HttpHeaders& headers, unsigned int timeout) const { + HttpHeaders answerHeaders; + return DoPost(target, answerHeaders, index, uri, body, headers, timeout); + } + + bool OrthancPeers::DoPost(MemoryBuffer& target, + HttpHeaders& answerHeaders, + size_t index, + const std::string& uri, + const std::string& body, + const HttpHeaders& headers, + unsigned int timeout) const + { if (index >= index_.size()) { ORTHANC_PLUGINS_THROW_PLUGIN_ERROR_CODE(OrthancPluginErrorCode_ParameterOutOfRange); @@ -2142,17 +2204,20 @@ } OrthancPlugins::MemoryBuffer answer; + OrthancPlugins::MemoryBuffer answerHeadersBuffer; uint16_t status; PluginHttpHeaders pluginHeaders(headers); OrthancPluginErrorCode code = OrthancPluginCallPeerApi - (GetGlobalContext(), *answer, NULL, &status, peers_, + (GetGlobalContext(), *answer, *answerHeadersBuffer, &status, peers_, static_cast<uint32_t>(index), OrthancPluginHttpMethod_Post, uri.c_str(), pluginHeaders.GetSize(), pluginHeaders.GetKeys(), pluginHeaders.GetValues(), body.empty() ? NULL : body.c_str(), body.size(), timeout); if (code == OrthancPluginErrorCode_Success) { target.Swap(answer); + DecodeHttpHeaders(answerHeaders, answerHeadersBuffer); + return (status == 200); } else @@ -3199,36 +3264,6 @@ } #endif - - static void DecodeHttpHeaders(HttpHeaders& target, - const MemoryBuffer& source) - { - Json::Value v; - source.ToJson(v); - - if (v.type() != Json::objectValue) - { - ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError); - } - - Json::Value::Members members = v.getMemberNames(); - target.clear(); - - for (size_t i = 0; i < members.size(); i++) - { - const Json::Value& h = v[members[i]]; - if (h.type() != Json::stringValue) - { - ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError); - } - else - { - target[members[i]] = h.asString(); - } - } - } - - void HttpClient::ExecuteWithoutStream(uint16_t& httpStatus, HttpHeaders& answerHeaders, std::string& answerBody,
--- a/Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h Mon May 12 13:02:53 2025 +0200 +++ b/Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h Mon May 19 17:00:51 2025 +0200 @@ -862,6 +862,14 @@ unsigned int timeout) const; bool DoPost(MemoryBuffer& target, + HttpHeaders& answerHeaders, + size_t index, + const std::string& uri, + const std::string& body, + const HttpHeaders& headers, + unsigned int timeout) const; + + bool DoPost(MemoryBuffer& target, const std::string& name, const std::string& uri, const std::string& body, @@ -879,6 +887,14 @@ const std::string& body, const HttpHeaders& headers, unsigned int timeout) const; + + bool DoPost(Json::Value& target, + HttpHeaders& answerHeaders, + size_t index, + const std::string& uri, + const std::string& body, + const HttpHeaders& headers, + unsigned int timeout) const; bool DoPost(Json::Value& target, const std::string& name,