changeset 3528:f6fe095f7130

don't open a multipart stream if plugin only sends one part
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 26 Sep 2019 13:40:49 +0200
parents 40c80049fac7
children 625625ed098f
files Core/HttpServer/HttpOutput.cpp Core/HttpServer/HttpOutput.h Plugins/Engine/OrthancPlugins.cpp
diffstat 3 files changed, 253 insertions(+), 64 deletions(-) [+]
line wrap: on
line diff
--- 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<std::string>::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<std::string>::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<std::string, std::string>& headers)
+  static void PrepareMultipartItemHeader(std::string& target,
+                                         size_t length,
+                                         const std::map<std::string, std::string>& 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<std::string, std::string>::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<std::string>(length) + "\r\n";
+      target += "Content-Length: " + boost::lexical_cast<std::string>(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<std::string, std::string>& 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<const void*>& parts,
+    const std::vector<size_t>& sizes,
+    const std::vector<const std::map<std::string, std::string>*>& 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<std::string, std::string> 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);
+  }
 }
--- 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 <string>
 #include <stdint.h>
 #include <map>
+#include <vector>
 
 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<const void*>& parts,
+      const std::vector<size_t>& sizes,
+      const std::vector<const std::map<std::string, std::string>*>& headers);
   };
 }
--- 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<std::string>  errorDetails_;
       bool                        logDetails_;
+      MultipartState              multipartState_;
+      std::string                 multipartSubType_;
+      std::string                 multipartContentType_;
+      std::string                 multipartFirstPart_;
+      std::map<std::string, std::string>  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<std::string, std::string>& 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<const char*>(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<const void*> parts;
+              std::vector<size_t> sizes;
+              std::vector<const std::map<std::string, std::string>*> 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<const _OrthancPluginAnswerBuffer*>(parameters);
 
-    HttpOutput& output = reinterpret_cast<PImpl::PluginHttpOutput*>(p.output)->GetOutput();
-
     std::map<std::string, std::string> headers;  // No custom headers
-    output.SendMultipartItem(p.answer, p.answerSize, headers);
+    reinterpret_cast<PImpl::PluginHttpOutput*>(p.output)->SendMultipartItem(p.answer, p.answerSize, headers);
   }
 
 
@@ -2924,7 +3029,6 @@
     // connection was closed by the HTTP client.
     const _OrthancPluginSendMultipartItem2& p =
       *reinterpret_cast<const _OrthancPluginSendMultipartItem2*>(parameters);
-    HttpOutput& output = reinterpret_cast<PImpl::PluginHttpOutput*>(p.output)->GetOutput();
     
     std::map<std::string, std::string> 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<PImpl::PluginHttpOutput*>(p.output)->SendMultipartItem(p.answer, p.answerSize, headers);
   }
       
 
@@ -3207,8 +3311,7 @@
       {
         const _OrthancPluginStartMultipartAnswer& p =
           *reinterpret_cast<const _OrthancPluginStartMultipartAnswer*>(parameters);
-        HttpOutput& output = reinterpret_cast<PImpl::PluginHttpOutput*>(p.output)->GetOutput();
-        output.StartMultipart(p.subType, p.contentType);
+        reinterpret_cast<PImpl::PluginHttpOutput*>(p.output)->StartMultipart(p.subType, p.contentType);
         return true;
       }