# HG changeset patch # User Sebastien Jodogne # Date 1569498049 -7200 # Node ID f6fe095f713058d0a3a03defa6455938ea25d86e # Parent 40c80049fac732510ef041b65a7cce991aa6ca3b don't open a multipart stream if plugin only sends one part diff -r 40c80049fac7 -r f6fe095f7130 Core/HttpServer/HttpOutput.cpp --- a/Core/HttpServer/HttpOutput.cpp Thu Sep 26 10:50:58 2019 +0200 +++ b/Core/HttpServer/HttpOutput.cpp Thu Sep 26 13:40:49 2019 +0200 @@ -410,8 +410,25 @@ } - void HttpOutput::StateMachine::StartMultipart(const std::string& subType, - const std::string& contentType) + void HttpOutput::StateMachine::CheckHeadersCompatibilityWithMultipart() const + { + for (std::list::const_iterator + it = headers_.begin(); it != headers_.end(); ++it) + { + if (!Toolbox::StartsWith(*it, "Set-Cookie: ")) + { + throw OrthancException(ErrorCode_BadSequenceOfCalls, + "The only headers that can be set in multipart answers " + "are Set-Cookie (here: " + *it + " is set)"); + } + } + } + + + static void PrepareMultipartMainHeader(std::string& boundary, + std::string& contentTypeHeader, + const std::string& subType, + const std::string& contentType) { if (subType != "mixed" && subType != "related") @@ -419,6 +436,40 @@ throw OrthancException(ErrorCode_ParameterOutOfRange); } + /** + * Fix for issue 54 ("Decide what to do wrt. quoting of multipart + * answers"). The "type" parameter in the "Content-Type" HTTP + * header must be quoted if it contains a forward slash "/". This + * is necessary for DICOMweb compatibility with OsiriX, but breaks + * compatibility with old releases of the client in the Orthanc + * DICOMweb plugin <= 0.3 (releases >= 0.4 work fine). + * + * Full history is available at the following locations: + * - In changeset 2248:69b0f4e8a49b: + * # hg history -v -r 2248 + * - https://bitbucket.org/sjodogne/orthanc/issues/54/ + * - https://groups.google.com/d/msg/orthanc-users/65zhIM5xbKI/TU5Q1_LhAwAJ + **/ + std::string tmp; + if (contentType.find('/') == std::string::npos) + { + // No forward slash in the content type + tmp = contentType; + } + else + { + // Quote the content type because of the forward slash + tmp = "\"" + contentType + "\""; + } + + boundary = Toolbox::GenerateUuid() + "-" + Toolbox::GenerateUuid(); + contentTypeHeader = ("multipart/" + subType + "; type=" + tmp + "; boundary=" + boundary); + } + + + void HttpOutput::StateMachine::StartMultipart(const std::string& subType, + const std::string& contentType) + { if (state_ != State_WritingHeader) { throw OrthancException(ErrorCode_BadSequenceOfCalls); @@ -464,65 +515,31 @@ } // Possibly add the cookies + CheckHeadersCompatibilityWithMultipart(); + for (std::list::const_iterator it = headers_.begin(); it != headers_.end(); ++it) { - if (!Toolbox::StartsWith(*it, "Set-Cookie: ")) - { - throw OrthancException(ErrorCode_BadSequenceOfCalls, - "The only headers that can be set in multipart answers " - "are Set-Cookie (here: " + *it + " is set)"); - } - header += *it; } - /** - * Fix for issue 54 ("Decide what to do wrt. quoting of multipart - * answers"). The "type" parameter in the "Content-Type" HTTP - * header must be quoted if it contains a forward slash "/". This - * is necessary for DICOMweb compatibility with OsiriX, but breaks - * compatibility with old releases of the client in the Orthanc - * DICOMweb plugin <= 0.3 (releases >= 0.4 work fine). - * - * Full history is available at the following locations: - * - In changeset 2248:69b0f4e8a49b: - * # hg history -v -r 2248 - * - https://bitbucket.org/sjodogne/orthanc/issues/54/ - * - https://groups.google.com/d/msg/orthanc-users/65zhIM5xbKI/TU5Q1_LhAwAJ - **/ - std::string tmp; - if (contentType.find('/') == std::string::npos) - { - // No forward slash in the content type - tmp = contentType; - } - else - { - // Quote the content type because of the forward slash - tmp = "\"" + contentType + "\""; - } - - multipartBoundary_ = Toolbox::GenerateUuid() + "-" + Toolbox::GenerateUuid(); + std::string contentTypeHeader; + PrepareMultipartMainHeader(multipartBoundary_, contentTypeHeader, subType, contentType); multipartContentType_ = contentType; - header += ("Content-Type: multipart/" + subType + "; type=" + - tmp + "; boundary=" + multipartBoundary_ + "\r\n\r\n"); + header += ("Content-Type: " + contentTypeHeader + "\r\n\r\n"); stream_.Send(true, header.c_str(), header.size()); state_ = State_WritingMultipart; } - void HttpOutput::StateMachine::SendMultipartItem(const void* item, - size_t length, - const std::map& headers) + static void PrepareMultipartItemHeader(std::string& target, + size_t length, + const std::map& headers, + const std::string& boundary, + const std::string& contentType) { - if (state_ != State_WritingMultipart) - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); - } - - std::string header = "--" + multipartBoundary_ + "\r\n"; + target = "--" + boundary + "\r\n"; bool hasContentType = false; bool hasContentLength = false; @@ -531,7 +548,7 @@ for (std::map::const_iterator it = headers.begin(); it != headers.end(); ++it) { - header += it->first + ": " + it->second + "\r\n"; + target += it->first + ": " + it->second + "\r\n"; std::string tmp; Toolbox::ToLowerCase(tmp, it->first); @@ -554,19 +571,32 @@ if (!hasContentType) { - header += "Content-Type: " + multipartContentType_ + "\r\n"; + target += "Content-Type: " + contentType + "\r\n"; } if (!hasContentLength) { - header += "Content-Length: " + boost::lexical_cast(length) + "\r\n"; + target += "Content-Length: " + boost::lexical_cast(length) + "\r\n"; } if (!hasMimeVersion) { - header += "MIME-Version: 1.0\r\n\r\n"; + target += "MIME-Version: 1.0\r\n\r\n"; + } + } + + + void HttpOutput::StateMachine::SendMultipartItem(const void* item, + size_t length, + const std::map& headers) + { + if (state_ != State_WritingMultipart) + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); } + std::string header; + PrepareMultipartItemHeader(header, length, headers, multipartBoundary_, multipartContentType_); stream_.Send(false, header.c_str(), header.size()); if (length > 0) @@ -685,4 +715,43 @@ stateMachine_.CloseBody(); } + + void HttpOutput::AnswerMultipartWithoutChunkedTransfer( + const std::string& subType, + const std::string& contentType, + const std::vector& parts, + const std::vector& sizes, + const std::vector*>& headers) + { + if (parts.size() != sizes.size()) + { + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + + stateMachine_.CheckHeadersCompatibilityWithMultipart(); + + std::string boundary, contentTypeHeader; + PrepareMultipartMainHeader(boundary, contentTypeHeader, subType, contentType); + SetContentType(contentTypeHeader); + + std::map empty; + + ChunkedBuffer chunked; + for (size_t i = 0; i < parts.size(); i++) + { + std::string partHeader; + PrepareMultipartItemHeader(partHeader, sizes[i], headers[i] == NULL ? empty : *headers[i], + boundary, contentType); + + chunked.AddChunk(partHeader); + chunked.AddChunk(parts[i], sizes[i]); + chunked.AddChunk("\r\n"); + } + + chunked.AddChunk("--" + boundary + "--\r\n"); + + std::string body; + chunked.Flatten(body); + Answer(body); + } } diff -r 40c80049fac7 -r f6fe095f7130 Core/HttpServer/HttpOutput.h --- a/Core/HttpServer/HttpOutput.h Thu Sep 26 10:50:58 2019 +0200 +++ b/Core/HttpServer/HttpOutput.h Thu Sep 26 13:40:49 2019 +0200 @@ -41,6 +41,7 @@ #include #include #include +#include namespace Orthanc { @@ -113,6 +114,8 @@ { return state_; } + + void CheckHeadersCompatibilityWithMultipart() const; }; StateMachine stateMachine_; @@ -229,5 +232,19 @@ } void Answer(IHttpStreamAnswer& stream); + + /** + * This method is a replacement to the combination + * "StartMultipart()" + "SendMultipartItem()". It generates the + * same answer, but it gives a chance to compress the body if + * "Accept-Encoding: gzip" is provided by the client, which is not + * possible in chunked transfers. + **/ + void AnswerMultipartWithoutChunkedTransfer( + const std::string& subType, + const std::string& contentType, + const std::vector& parts, + const std::vector& sizes, + const std::vector*>& headers); }; } diff -r 40c80049fac7 -r f6fe095f7130 Plugins/Engine/OrthancPlugins.cpp --- a/Plugins/Engine/OrthancPlugins.cpp Thu Sep 26 10:50:58 2019 +0200 +++ b/Plugins/Engine/OrthancPlugins.cpp Thu Sep 26 13:40:49 2019 +0200 @@ -422,25 +422,46 @@ boost::mutex contextMutex_; ServerContext* context_; - public: class PluginHttpOutput : public boost::noncopyable { private: + enum MultipartState + { + MultipartState_None, + MultipartState_FirstPart, + MultipartState_SecondPart, + MultipartState_NextParts + }; + HttpOutput& output_; std::auto_ptr errorDetails_; bool logDetails_; + MultipartState multipartState_; + std::string multipartSubType_; + std::string multipartContentType_; + std::string multipartFirstPart_; + std::map multipartFirstHeaders_; public: PluginHttpOutput(HttpOutput& output) : - output_(output), - logDetails_(false) + output_(output), + logDetails_(false), + multipartState_(MultipartState_None) { } HttpOutput& GetOutput() { - return output_; + if (multipartState_ == MultipartState_None) + { + return output_; + } + else + { + // Must use "SendMultipartItem()" on multipart streams + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } } void SetErrorDetails(const std::string& details, @@ -472,14 +493,100 @@ } } + void StartMultipart(const char* subType, + const char* contentType) + { + if (multipartState_ != MultipartState_None) + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } + else + { + multipartState_ = MultipartState_FirstPart; + multipartSubType_ = subType; + multipartContentType_ = contentType; + } + } + + void SendMultipartItem(const void* data, + size_t size, + const std::map& headers) + { + if (size != 0 && data == NULL) + { + throw OrthancException(ErrorCode_NullPointer); + } + + switch (multipartState_) + { + case MultipartState_None: + // Must call "StartMultipart()" before + throw OrthancException(ErrorCode_BadSequenceOfCalls); + + case MultipartState_FirstPart: + multipartFirstPart_.assign(reinterpret_cast(data), size); + multipartFirstHeaders_ = headers; + multipartState_ = MultipartState_SecondPart; + break; + + case MultipartState_SecondPart: + // Start an actual stream for chunked transfer as soon as + // there are more than 2 elements in the multipart stream + output_.StartMultipart(multipartSubType_, multipartContentType_); + output_.SendMultipartItem(multipartFirstPart_.c_str(), multipartFirstPart_.size(), + multipartFirstHeaders_); + multipartFirstPart_.clear(); // Release memory + + output_.SendMultipartItem(data, size, headers); + multipartState_ = MultipartState_NextParts; + break; + + case MultipartState_NextParts: + output_.SendMultipartItem(data, size, headers); + break; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + void Close(OrthancPluginErrorCode error, PluginsErrorDictionary& dictionary) { if (error == OrthancPluginErrorCode_Success) { - if (GetOutput().IsWritingMultipart()) + switch (multipartState_) { - GetOutput().CloseMultipart(); + case MultipartState_None: + assert(!output_.IsWritingMultipart()); + break; + + case MultipartState_FirstPart: // Multipart started, but no part was sent + case MultipartState_SecondPart: // Multipart started, first part is pending + { + assert(!output_.IsWritingMultipart()); + std::vector parts; + std::vector sizes; + std::vector*> headers; + + if (multipartState_ == MultipartState_SecondPart) + { + parts.push_back(multipartFirstPart_.c_str()); + sizes.push_back(multipartFirstPart_.size()); + headers.push_back(&multipartFirstHeaders_); + } + + output_.AnswerMultipartWithoutChunkedTransfer(multipartSubType_, multipartContentType_, + parts, sizes, headers); + break; + } + + case MultipartState_NextParts: + assert(output_.IsWritingMultipart()); + output_.CloseMultipart(); + + default: + throw OrthancException(ErrorCode_InternalError); } } else @@ -2911,10 +3018,8 @@ const _OrthancPluginAnswerBuffer& p = *reinterpret_cast(parameters); - HttpOutput& output = reinterpret_cast(p.output)->GetOutput(); - std::map headers; // No custom headers - output.SendMultipartItem(p.answer, p.answerSize, headers); + reinterpret_cast(p.output)->SendMultipartItem(p.answer, p.answerSize, headers); } @@ -2924,7 +3029,6 @@ // connection was closed by the HTTP client. const _OrthancPluginSendMultipartItem2& p = *reinterpret_cast(parameters); - HttpOutput& output = reinterpret_cast(p.output)->GetOutput(); std::map headers; for (uint32_t i = 0; i < p.headersCount; i++) @@ -2932,7 +3036,7 @@ headers[p.headersKeys[i]] = p.headersValues[i]; } - output.SendMultipartItem(p.answer, p.answerSize, headers); + reinterpret_cast(p.output)->SendMultipartItem(p.answer, p.answerSize, headers); } @@ -3207,8 +3311,7 @@ { const _OrthancPluginStartMultipartAnswer& p = *reinterpret_cast(parameters); - HttpOutput& output = reinterpret_cast(p.output)->GetOutput(); - output.StartMultipart(p.subType, p.contentType); + reinterpret_cast(p.output)->StartMultipart(p.subType, p.contentType); return true; }