changeset 3415:2a821deece64

refactoring to handle "not allowed" HTTP status 405
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 11 Jun 2019 21:06:57 +0200
parents b9cba6a91780
children 541c787e2230
files Plugins/Engine/OrthancPlugins.cpp Plugins/Include/orthanc/OrthancCPlugin.h Plugins/Samples/Common/OrthancPluginCppWrapper.h
diffstat 3 files changed, 301 insertions(+), 128 deletions(-) [+]
line wrap: on
line diff
--- a/Plugins/Engine/OrthancPlugins.cpp	Tue Jun 11 19:44:10 2019 +0200
+++ b/Plugins/Engine/OrthancPlugins.cpp	Tue Jun 11 21:06:57 2019 +0200
@@ -554,7 +554,7 @@
     {
     private:
       _OrthancPluginChunkedRestCallback parameters_;
-      boost::regex                        regex_;
+      boost::regex                      regex_;
 
     public:
       ChunkedRestCallback(_OrthancPluginChunkedRestCallback parameters) :
@@ -1374,50 +1374,115 @@
   }
 
 
+  static std::string GetAllowedMethods(_OrthancPluginChunkedRestCallback parameters)
+  {
+    std::string s;
+
+    if (parameters.getHandler != NULL)
+    {
+      s += "GET";
+    }
+
+    if (parameters.postHandler != NULL)
+    {
+      if (!s.empty())
+      {
+        s+= ",";
+      }
+      
+      s += "POST";
+    }
+
+    if (parameters.deleteHandler != NULL)
+    {
+      if (!s.empty())
+      {
+        s+= ",";
+      }
+      
+      s += "DELETE";
+    }
+
+    if (parameters.putHandler != NULL)
+    {
+      if (!s.empty())
+      {
+        s+= ",";
+      }
+      
+      s += "PUT";
+    }
+
+    return s;
+  }
+
+
   bool OrthancPlugins::HandleChunkedGetDelete(HttpOutput& output,
                                               HttpMethod method,
                                               const UriComponents& uri,
                                               const Arguments& headers,
                                               const GetArguments& getArguments)
   {
-    if (method == HttpMethod_Get ||
-        method == HttpMethod_Delete)
+    RestCallbackMatcher matcher(uri);
+
+    PImpl::ChunkedRestCallback* callback = NULL;
+
+    // Loop over the callbacks registered by the plugins
+    for (PImpl::ChunkedRestCallbacks::const_iterator it = pimpl_->chunkedRestCallbacks_.begin(); 
+         it != pimpl_->chunkedRestCallbacks_.end(); ++it)
     {
-      RestCallbackMatcher matcher(uri);
-
-      PImpl::ChunkedRestCallback* callback = NULL;
-
-      // Loop over the callbacks registered by the plugins
-      for (PImpl::ChunkedRestCallbacks::const_iterator it = pimpl_->chunkedRestCallbacks_.begin(); 
-           it != pimpl_->chunkedRestCallbacks_.end(); ++it)
+      if (matcher.IsMatch((*it)->GetRegularExpression()))
       {
-        if (matcher.IsMatch((*it)->GetRegularExpression()))
-        {
-          callback = *it;
-          break;
-        }
+        callback = *it;
+        break;
       }
-
-      if (callback != NULL)
+    }
+
+    if (callback == NULL)
+    {
+      return false;
+    }
+    else
+    {
+      LOG(INFO) << "Delegating HTTP request to plugin for URI: " << matcher.GetFlatUri();
+
+      OrthancPluginRestCallback handler;
+
+      switch (method)
       {
-        LOG(INFO) << "Delegating HTTP request to plugin for URI: " << matcher.GetFlatUri();
-
+        case HttpMethod_Get:
+          handler = callback->GetParameters().getHandler;
+          break;
+
+        case HttpMethod_Delete:
+          handler = callback->GetParameters().deleteHandler;
+          break;
+
+        default:
+          handler = NULL;
+          break;
+      }
+
+      if (handler == NULL)
+      {
+        output.SendMethodNotAllowed(GetAllowedMethods(callback->GetParameters()));
+      }
+      else
+      {
         HttpRequestConverter converter(matcher, method, headers);
         converter.SetGetArguments(getArguments);
-        
+      
         PImpl::PluginHttpOutput pluginOutput(output);
         
-        assert(callback != NULL);
-        OrthancPluginErrorCode error = callback->GetParameters().handler
-          (reinterpret_cast<OrthancPluginRestOutput*>(&pluginOutput), 
-           NULL /* no reader */, matcher.GetFlatUri().c_str(), &converter.GetRequest());
+        OrthancPluginErrorCode error = handler(
+          reinterpret_cast<OrthancPluginRestOutput*>(&pluginOutput), 
+          matcher.GetFlatUri().c_str(), &converter.GetRequest());
         
         pluginOutput.Close(error, GetErrorDictionary());
-        return true;
       }
+      
+      return true;
     }
-
-    return false;
   }
 
 
@@ -4285,31 +4350,57 @@
       // Callback not found
       return false;
     }
-
-    LOG(INFO) << "Delegating chunked HTTP request to plugin for URI: " << matcher.GetFlatUri();
-
-    HttpRequestConverter converter(matcher, method, headers);
-    converter.GetRequest().body = NULL;
-    converter.GetRequest().bodySize = 0;
-
-    OrthancPluginServerChunkedRequestReader* reader = NULL;
-    
-    OrthancPluginErrorCode errorCode = callback->GetParameters().handler(
-      NULL /* no HTTP output */, &reader, matcher.GetFlatUri().c_str(), &converter.GetRequest());
-    
-    if (reader == NULL)
-    {
-      // The plugin has not created a reader for chunked body
-      return false;
-    }
-    else if (errorCode != OrthancPluginErrorCode_Success)
-    {
-      throw OrthancException(static_cast<ErrorCode>(errorCode));
-    }
     else
     {
-      target.reset(new HttpServerChunkedReader(reader, callback->GetParameters(), GetErrorDictionary()));
-      return true;
+      OrthancPluginServerChunkedRequestReaderFactory handler;
+
+      switch (method)
+      {
+        case HttpMethod_Post:
+          handler = callback->GetParameters().postHandler;
+          break;
+
+        case HttpMethod_Put:
+          handler = callback->GetParameters().putHandler;
+          break;
+
+        default:
+          handler = NULL;
+          break;
+      }
+
+      if (handler == NULL)
+      {
+        return false;
+      }
+      else
+      {
+        LOG(INFO) << "Delegating chunked HTTP request to plugin for URI: " << matcher.GetFlatUri();
+
+        HttpRequestConverter converter(matcher, method, headers);
+        converter.GetRequest().body = NULL;
+        converter.GetRequest().bodySize = 0;
+
+        OrthancPluginServerChunkedRequestReader* reader = NULL;
+    
+        OrthancPluginErrorCode errorCode = handler(
+          &reader, matcher.GetFlatUri().c_str(), &converter.GetRequest());
+    
+        if (reader == NULL)
+        {
+          // The plugin has not created a reader for chunked body
+          return false;
+        }
+        else if (errorCode != OrthancPluginErrorCode_Success)
+        {
+          throw OrthancException(static_cast<ErrorCode>(errorCode));
+        }
+        else
+        {
+          target.reset(new HttpServerChunkedReader(reader, callback->GetParameters(), GetErrorDictionary()));
+          return true;
+        }
+      }
     }
   }
 }
--- a/Plugins/Include/orthanc/OrthancCPlugin.h	Tue Jun 11 19:44:10 2019 +0200
+++ b/Plugins/Include/orthanc/OrthancCPlugin.h	Tue Jun 11 21:06:57 2019 +0200
@@ -6912,9 +6912,8 @@
 
   typedef struct _OrthancPluginServerChunkedRequestReader_t OrthancPluginServerChunkedRequestReader;
 
-  typedef OrthancPluginErrorCode (*OrthancPluginServerChunkedRequestHandler) (
-    OrthancPluginRestOutput*                   output,  /* out, for GET/DELETE only, NULL if POST/PUT */
-    OrthancPluginServerChunkedRequestReader**  reader, /* out, for POST/PUT only, NULL if GET/DELETE */
+  typedef OrthancPluginErrorCode (*OrthancPluginServerChunkedRequestReaderFactory) (
+    OrthancPluginServerChunkedRequestReader**  reader, /* out, for POST/PUT only */
     const char*                                url,
     const OrthancPluginHttpRequest*            request); /* body and bodySize are not used */
 
@@ -6933,7 +6932,10 @@
   typedef struct
   {
     const char*                                      pathRegularExpression;
-    OrthancPluginServerChunkedRequestHandler         handler;
+    OrthancPluginRestCallback                        getHandler;
+    OrthancPluginServerChunkedRequestReaderFactory   postHandler;
+    OrthancPluginRestCallback                        deleteHandler;
+    OrthancPluginServerChunkedRequestReaderFactory   putHandler;
     OrthancPluginServerChunkedRequestReaderAddChunk  addChunk;
     OrthancPluginServerChunkedRequestReaderExecute   execute;
     OrthancPluginServerChunkedRequestReaderFinalize  finalize;
@@ -6942,14 +6944,20 @@
   ORTHANC_PLUGIN_INLINE void OrthancPluginRegisterChunkedRestCallback(
     OrthancPluginContext*                            context,
     const char*                                      pathRegularExpression,
-    OrthancPluginServerChunkedRequestHandler         handler,
+    OrthancPluginRestCallback                        getHandler,
+    OrthancPluginServerChunkedRequestReaderFactory   postHandler,
+    OrthancPluginRestCallback                        deleteHandler,
+    OrthancPluginServerChunkedRequestReaderFactory   putHandler,
     OrthancPluginServerChunkedRequestReaderAddChunk  addChunk,
     OrthancPluginServerChunkedRequestReaderExecute   execute,
     OrthancPluginServerChunkedRequestReaderFinalize  finalize)
   {
     _OrthancPluginChunkedRestCallback params;
     params.pathRegularExpression = pathRegularExpression;
-    params.handler = handler;
+    params.getHandler = getHandler;
+    params.postHandler = postHandler;
+    params.deleteHandler = deleteHandler;
+    params.putHandler = putHandler;
     params.addChunk = addChunk;
     params.execute = execute;
     params.finalize = finalize;
--- a/Plugins/Samples/Common/OrthancPluginCppWrapper.h	Tue Jun 11 19:44:10 2019 +0200
+++ b/Plugins/Samples/Common/OrthancPluginCppWrapper.h	Tue Jun 11 21:06:57 2019 +0200
@@ -554,9 +554,9 @@
   namespace Internals
   {
     template <RestCallback Callback>
-    OrthancPluginErrorCode Protect(OrthancPluginRestOutput* output,
-                                   const char* url,
-                                   const OrthancPluginHttpRequest* request)
+    static OrthancPluginErrorCode Protect(OrthancPluginRestOutput* output,
+                                          const char* url,
+                                          const OrthancPluginHttpRequest* request)
     {
       try
       {
@@ -934,71 +934,52 @@
   };
 
 
-  typedef IChunkedRequestReader* (*ChunkedRestCallback) (OrthancPluginRestOutput* output,
-                                                         const char* url,
+  typedef IChunkedRequestReader* (*ChunkedRestCallback) (const char* url,
                                                          const OrthancPluginHttpRequest* request);
 
 
   namespace Internals
   {
+    inline void NullRestCallback(OrthancPluginRestOutput* output,
+                                 const char* url,
+                                 const OrthancPluginHttpRequest* request)
+    {
+    }
+  
+    inline IChunkedRequestReader *NullChunkedRestCallback(const char* url,
+                                                          const OrthancPluginHttpRequest* request)
+    {
+      return NULL;
+    }
+
+
 #if HAS_ORTHANC_PLUGIN_CHUNKED_HTTP_SERVER == 1
     template <ChunkedRestCallback Callback>
-    OrthancPluginErrorCode Protect(OrthancPluginRestOutput* output,
-                                   OrthancPluginServerChunkedRequestReader** reader,
-                                   const char* url,
-                                   const OrthancPluginHttpRequest* request)
+    static OrthancPluginErrorCode Protect(OrthancPluginServerChunkedRequestReader** reader,
+                                          const char* url,
+                                          const OrthancPluginHttpRequest* request)
     {
       try
       {
-        switch (request->method)
+        if (reader == NULL)
+        {
+          return OrthancPluginErrorCode_InternalError;
+        }
+        else
         {
-          case OrthancPluginHttpMethod_Get:
-          case OrthancPluginHttpMethod_Delete:
-            if (output == NULL ||
-                reader != NULL ||
-                Callback(output, url, request) != NULL)
-            {
-              return OrthancPluginErrorCode_InternalError;
-            }
-
+          *reader = reinterpret_cast<OrthancPluginServerChunkedRequestReader*>(Callback(url, request));
+          if (*reader == NULL)
+          {
+            return OrthancPluginErrorCode_Plugin;
+          }
+          else
+          {
             return OrthancPluginErrorCode_Success;
-
-          case OrthancPluginHttpMethod_Post:
-          case OrthancPluginHttpMethod_Put:
-            if (output != NULL ||
-                reader == NULL)
-            {
-              return OrthancPluginErrorCode_InternalError;
-            }
-            else
-            {
-              *reader = reinterpret_cast<OrthancPluginServerChunkedRequestReader*>(Callback(NULL, url, request));
-              if (*reader == NULL)
-              {
-                return OrthancPluginErrorCode_Plugin;
-              }
-            }
-
-            return OrthancPluginErrorCode_Success;
-
-          default:
-            return OrthancPluginErrorCode_InternalError;
+          }
         }
       }
       catch (ORTHANC_PLUGINS_EXCEPTION_CLASS& e)
       {
-#if HAS_ORTHANC_EXCEPTION == 1 && HAS_ORTHANC_PLUGIN_EXCEPTION_DETAILS == 1
-        if (HasGlobalContext() &&
-            e.HasDetails())
-        {
-          // The "false" instructs Orthanc not to log the detailed
-          // error message. This is to avoid duplicating the details,
-          // because "OrthancException" already does it on construction.
-          OrthancPluginSetHttpErrorDetails
-            (GetGlobalContext(), output, e.GetDetails(), false);
-        }
-#endif
-
         return static_cast<OrthancPluginErrorCode>(e.GetErrorCode());
       }
       catch (boost::bad_lexical_cast&)
@@ -1011,7 +992,7 @@
       }
     }
 
-    static OrthancPluginErrorCode ChunkedRequestReaderAddChunk(
+    static inline OrthancPluginErrorCode ChunkedRequestReaderAddChunk(
       OrthancPluginServerChunkedRequestReader* reader,
       const void*                              data,
       uint32_t                                 size)
@@ -1040,7 +1021,7 @@
       }
     }
 
-    static OrthancPluginErrorCode ChunkedRequestReaderExecute(
+    static inline OrthancPluginErrorCode ChunkedRequestReaderExecute(
       OrthancPluginServerChunkedRequestReader* reader,
       OrthancPluginRestOutput*                 output)
     {
@@ -1068,7 +1049,7 @@
       }
     }
 
-    static void ChunkedRequestReaderFinalize(
+    static inline void ChunkedRequestReaderFinalize(
       OrthancPluginServerChunkedRequestReader* reader)
     {
       if (reader != NULL)
@@ -1079,39 +1060,121 @@
 
 #else
 
-    template <ChunkedRestCallback Callback>
-    void ChunkedRestCompatibility(OrthancPluginRestOutput* output,
-                                  const char* url,
-                                  const OrthancPluginHttpRequest* request)
+    template<
+      RestCallback         GetHandler,
+      ChunkedRestCallback  PostHandler,
+      RestCallback         DeleteHandler,
+      ChunkedRestCallback  PutHandler
+      >
+    static void ChunkedRestCompatibility(OrthancPluginRestOutput* output,
+                                         const char* url,
+                                         const OrthancPluginHttpRequest* request)
     {
+      std::string allowed;
+
+      if (GetHandler != Internals::NullRestCallback)
+      {
+        allowed += "GET";
+      }
+
+      if (PostHandler != Internals::NullChunkedRestCallback)
+      {
+        if (!allowed.empty())
+        {
+          allowed += ",";
+        }
+        
+        allowed += "POST";
+      }
+
+      if (DeleteHandler != Internals::NullRestCallback)
+      {
+        if (!allowed.empty())
+        {
+          allowed += ",";
+        }
+        
+        allowed += "DELETE";
+      }
+
+      if (PutHandler != Internals::NullChunkedRestCallback)
+      {
+        if (!allowed.empty())
+        {
+          allowed += ",";
+        }
+        
+        allowed += "PUT";
+      }
+      
       switch (request->method)
       {
         case OrthancPluginHttpMethod_Get:
-        case OrthancPluginHttpMethod_Delete:
-          if (Callback(output, url, request) != NULL)
+        {
+          if (GetHandler == Internals::NullRestCallback)
           {
-            ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError);
+            OrthancPluginSendMethodNotAllowed(GetGlobalContext(), output, allowed.c_str());
           }
           else
           {
-            return;
+            GetHandler(output, url, request);
           }
+          return;
+        }
 
         case OrthancPluginHttpMethod_Post:
-        case OrthancPluginHttpMethod_Put:
         {
-          std::auto_ptr<IChunkedRequestReader> reader(Callback(NULL, url, request));
-          if (reader.get() == NULL)
+          if (PostHandler == Internals::NullChunkedRestCallback)
           {
-            ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError);
+            OrthancPluginSendMethodNotAllowed(GetGlobalContext(), output, allowed.c_str());
           }
           else
           {
-            if (request->bodySize != 0)
+            std::auto_ptr<IChunkedRequestReader> reader(PostHandler(url, request));
+            if (reader.get() == NULL)
+            {
+              ORTHANC_PLUGINS_THROW_EXCEPTION(Plugin);
+            }
+            else
             {
               reader->AddChunk(request->body, request->bodySize);
+              reader->Execute(output);
             }
-            reader->Execute(output);
+          }
+          return;
+        }
+
+        case OrthancPluginHttpMethod_Delete:
+        {
+          if (DeleteHandler == Internals::NullRestCallback)
+          {
+            OrthancPluginSendMethodNotAllowed(GetGlobalContext(), output, allowed.c_str());
+          }
+          else
+          {
+            DeleteHandler(output, url, request);
+          }
+          return;
+        }
+
+        case OrthancPluginHttpMethod_Put:
+        {
+          if (PutHandler == Internals::NullChunkedRestCallback)
+          {
+            OrthancPluginSendMethodNotAllowed(GetGlobalContext(), output, allowed.c_str());
+          }
+          else
+          {
+            std::auto_ptr<IChunkedRequestReader> reader(PutHandler(url, request));
+            if (reader.get() == NULL)
+            {
+              ORTHANC_PLUGINS_THROW_EXCEPTION(Plugin);
+            }
+            else
+            {
+              reader->AddChunk(request->body, request->bodySize);
+              reader->Execute(output);
+            }
           }
           return;
         }
@@ -1124,12 +1187,22 @@
   }
 
 
-  template <ChunkedRestCallback Callback>
+  
+  template<
+    RestCallback         GetHandler    = Internals::NullRestCallback,
+    ChunkedRestCallback  PostHandler   = Internals::NullChunkedRestCallback,
+    RestCallback         DeleteHandler = Internals::NullRestCallback,
+    ChunkedRestCallback  PutHandler    = Internals::NullChunkedRestCallback
+    >
   void RegisterChunkedRestCallback(const std::string& uri)
   {
 #if HAS_ORTHANC_PLUGIN_CHUNKED_HTTP_SERVER == 1
     OrthancPluginRegisterChunkedRestCallback(
-      GetGlobalContext(), uri.c_str(), Internals::Protect<Callback>,
+      GetGlobalContext(), uri.c_str(),
+      GetHandler == Internals::NullRestCallback         ? NULL : Internals::Protect<GetHandler>,
+      PostHandler == Internals::NullChunkedRestCallback ? NULL : Internals::Protect<PostHandler>,
+      DeleteHandler == Internals::NullRestCallback      ? NULL : Internals::Protect<DeleteHandler>,
+      PutHandler == Internals::NullChunkedRestCallback  ? NULL : Internals::Protect<PutHandler>,
       Internals::ChunkedRequestReaderAddChunk,
       Internals::ChunkedRequestReaderExecute,
       Internals::ChunkedRequestReaderFinalize);
@@ -1138,7 +1211,8 @@
                "of the Orthanc SDK. Multipart transfers will be entirely stored in RAM.");
     OrthancPluginRegisterRestCallback(
       GetGlobalContext(), uri.c_str(), 
-      Internals::Protect< Internals::ChunkedRestCompatibility<Callback> >);
+      Internals::Protect< Internals::ChunkedRestCompatibility<
+      GetHandler, PostHandler, DeleteHandler, PutHandler> >);
 #endif
   }
 }