# HG changeset patch # User Sebastien Jodogne # Date 1560280017 -7200 # Node ID 2a821deece64b2227ace161ea8b017e7f86f639a # Parent b9cba6a9178085a2667d15b6f4ded5d98145bbe7 refactoring to handle "not allowed" HTTP status 405 diff -r b9cba6a91780 -r 2a821deece64 Plugins/Engine/OrthancPlugins.cpp --- 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(&pluginOutput), - NULL /* no reader */, matcher.GetFlatUri().c_str(), &converter.GetRequest()); + OrthancPluginErrorCode error = handler( + reinterpret_cast(&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)); - } 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)); + } + else + { + target.reset(new HttpServerChunkedReader(reader, callback->GetParameters(), GetErrorDictionary())); + return true; + } + } } } } diff -r b9cba6a91780 -r 2a821deece64 Plugins/Include/orthanc/OrthancCPlugin.h --- 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; diff -r b9cba6a91780 -r 2a821deece64 Plugins/Samples/Common/OrthancPluginCppWrapper.h --- 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 - 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 - 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(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(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(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 - 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 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 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 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 + + 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, + GetGlobalContext(), uri.c_str(), + GetHandler == Internals::NullRestCallback ? NULL : Internals::Protect, + PostHandler == Internals::NullChunkedRestCallback ? NULL : Internals::Protect, + DeleteHandler == Internals::NullRestCallback ? NULL : Internals::Protect, + PutHandler == Internals::NullChunkedRestCallback ? NULL : Internals::Protect, 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 >); + Internals::Protect< Internals::ChunkedRestCompatibility< + GetHandler, PostHandler, DeleteHandler, PutHandler> >); #endif } }