Improve api error handling

refs #10194

diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp
index a9f580758c1783ec068605603d8fb463339fd663..d7ac3b353c0dba23b496bc4e2d58826b5dc2dac2 100644 (file)
--- a/lib/remote/actionshandler.cpp
+++ b/lib/remote/actionshandler.cpp
@@ -33,22 +33,18 @@ REGISTER_URLHANDLER("/v1/actions", ActionsHandler);
 
 bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod != "POST") {
-               HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be POST.");
-               return true;
-       }
+       if (request.RequestUrl->GetPath().size() != 3)
+               return false;
 
-       if (request.RequestUrl->GetPath().size() < 3) {
-               HttpUtility::SendJsonError(response, 400, "Action is missing.");
-               return true;
-       }
+       if (request.RequestMethod != "POST")
+               return false;
 
        String actionName = request.RequestUrl->GetPath()[2];
 
        ApiAction::Ptr action = ApiAction::GetByName(actionName);
 
        if (!action) {
-               HttpUtility::SendJsonError(response, 404, "Action '" + actionName + "' could not be found.");
+               HttpUtility::SendJsonError(response, 404, "Action '" + actionName + "' does not exist.");
                return true;
        }
 

diff --git a/lib/remote/apilistener-configsync.cpp b/lib/remote/apilistener-configsync.cpp
index e40fb2d18b8a15dc314c3cfe970b5ae52375fbaf..3824b4cb43e2a6fcffd6c2c15b8d628a9f35106d 100644 (file)
--- a/lib/remote/apilistener-configsync.cpp
+++ b/lib/remote/apilistener-configsync.cpp
@@ -44,10 +44,8 @@ void ApiListener::ConfigUpdateObjectHandler(const ConfigObject::Ptr& object, con
 {
        ApiListener::Ptr listener = ApiListener::GetInstance();
 
-       if (!listener) {
-               Log(LogCritical, "ApiListener", "No instance available.");
+       if (!listener)
                return;
-       }
 
        if (object->IsActive()) {
                /* Sync object config */
@@ -66,10 +64,8 @@ Value ApiListener::ConfigUpdateObjectAPIHandler(const MessageOrigin::Ptr& origin
        /* check permissions */
        ApiListener::Ptr listener = ApiListener::GetInstance();
 
-       if (!listener) {
-               Log(LogCritical, "ApiListener", "No instance available.");
+       if (!listener)
                return Empty;
-       }
 
        if (!listener->GetAcceptConfig()) {
                Log(LogWarning, "ApiListener")
@@ -174,10 +170,8 @@ Value ApiListener::ConfigDeleteObjectAPIHandler(const MessageOrigin::Ptr& origin
        /* check permissions */
        ApiListener::Ptr listener = ApiListener::GetInstance();
 
-       if (!listener) {
-               Log(LogCritical, "ApiListener", "No instance available.");
+       if (!listener)
                return Empty;
-       }
 
        if (!listener->GetAcceptConfig()) {
                Log(LogWarning, "ApiListener")

diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp
index cda6a96280b48cd63adc7b25f0755fbf40400a7c..5969e39fc33740a7a86a3969a3c2219da76ad306 100644 (file)
--- a/lib/remote/configfileshandler.cpp
+++ b/lib/remote/configfileshandler.cpp
@@ -31,16 +31,9 @@ REGISTER_URLHANDLER("/v1/config/files", ConfigFilesHandler);
 
 bool ConfigFilesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod == "GET")
-               HandleGet(user, request, response);
-       else
-               HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET.");
+       if (request.RequestMethod != "GET")
+               return false;
 
-       return true;
-}
-
-void ConfigFilesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
-{
        Dictionary::Ptr params = HttpUtility::FetchRequestParameters(request);
 
        const std::vector<String>& urlPath = request.RequestUrl->GetPath();
@@ -61,21 +54,29 @@ void ConfigFilesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& reques
        String packageName = HttpUtility::GetLastParameter(params, "package");
        String stageName = HttpUtility::GetLastParameter(params, "stage");
 
-       if (!ConfigPackageUtility::ValidateName(packageName))
-               return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+       if (!ConfigPackageUtility::ValidateName(packageName)) {
+               HttpUtility::SendJsonError(response, 400, "Invalid package name.");
+               return true;
+       }
 
-       if (!ConfigPackageUtility::ValidateName(stageName))
-               return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist.");
+       if (!ConfigPackageUtility::ValidateName(stageName)) {
+               HttpUtility::SendJsonError(response, 400, "Invalid stage name.");
+               return true;
+       }
 
        String relativePath = HttpUtility::GetLastParameter(params, "path");
 
-       if (ConfigPackageUtility::ContainsDotDot(relativePath))
-               return HttpUtility::SendJsonError(response, 403, "Path contains '..' (not allowed).");
+       if (ConfigPackageUtility::ContainsDotDot(relativePath)) {
+               HttpUtility::SendJsonError(response, 400, "Path contains '..' (not allowed).");
+               return true;
+       }
 
        String path = ConfigPackageUtility::GetPackageDir() + "/" + packageName + "/" + stageName + "/" + relativePath;
 
-       if (!Utility::PathExists(path))
-               return HttpUtility::SendJsonError(response, 404, "Path not found.");
+       if (!Utility::PathExists(path)) {
+               HttpUtility::SendJsonError(response, 404, "Path not found.");
+               return true;
+       }
 
        try {
                std::ifstream fp(path.CStr(), std::ifstream::in | std::ifstream::binary);
@@ -86,9 +87,9 @@ void ConfigFilesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& reques
                response.AddHeader("Content-Type", "application/octet-stream");
                response.WriteBody(content.CStr(), content.GetLength());
        } catch (const std::exception& ex) {
-               return HttpUtility::SendJsonError(response, 500, "Could not read file.",
+               HttpUtility::SendJsonError(response, 500, "Could not read file.",
                    request.GetVerboseErrors() ? DiagnosticInformation(ex) : "");
        }
-}
-
 
+       return true;
+}

diff --git a/lib/remote/configfileshandler.hpp b/lib/remote/configfileshandler.hpp
index 6c36a20b57e5e1039d89165ab66215e63b69731e..604a72d98b319d68422342f1334751a1975705ed 100644 (file)
--- a/lib/remote/configfileshandler.hpp
+++ b/lib/remote/configfileshandler.hpp
@@ -31,9 +31,6 @@ public:
        DECLARE_PTR_TYPEDEFS(ConfigFilesHandler);
 
        virtual bool HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) override;
-
-private:
-       void HandleGet(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response);
 };
 
 }

diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp
index 23a1fc57942f2dcf146d0bfae3e539cfcf37b7a9..d6906f639e582a0acb3aae4177286ce6b0a41137 100644 (file)
--- a/lib/remote/configpackageshandler.cpp
+++ b/lib/remote/configpackageshandler.cpp
@@ -30,11 +30,8 @@ REGISTER_URLHANDLER("/v1/config/packages", ConfigPackagesHandler);
 
 bool ConfigPackagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestUrl->GetPath().size() > 4) {
-               String path = boost::algorithm::join(request.RequestUrl->GetPath(), "/");
-               HttpUtility::SendJsonError(response, 404, "The requested path is too long to match any config package requests");
-               return true;
-       }
+       if (request.RequestUrl->GetPath().size() > 4)
+               return false;
 
        if (request.RequestMethod == "GET")
                HandleGet(user, request, response);
@@ -43,7 +40,7 @@ bool ConfigPackagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest&
        else if (request.RequestMethod == "DELETE")
                HandleDelete(user, request, response);
        else
-               HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET, POST or DELETE.");
+               return false;
 
        return true;
 }
@@ -83,7 +80,7 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re
        String packageName = HttpUtility::GetLastParameter(params, "package");
 
        if (!ConfigPackageUtility::ValidateName(packageName)) {
-               HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+               HttpUtility::SendJsonError(response, 400, "Invalid package name.");
                return;
        }
 
@@ -121,7 +118,7 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest&
        String packageName = HttpUtility::GetLastParameter(params, "package");
 
        if (!ConfigPackageUtility::ValidateName(packageName)) {
-               HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+               HttpUtility::SendJsonError(response, 400, "Invalid package name.");
                return;
        }
 

diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp
index 4a9d9e1817ad1124db5f5b3ff241a9bca2086127..93f58fff0a0ed5edae98e6f1415beef79280318c 100644 (file)
--- a/lib/remote/configstageshandler.cpp
+++ b/lib/remote/configstageshandler.cpp
@@ -32,11 +32,8 @@ REGISTER_URLHANDLER("/v1/config/stages", ConfigStagesHandler);
 
 bool ConfigStagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestUrl->GetPath().size() > 5) {
-               String path = boost::algorithm::join(request.RequestUrl->GetPath(), "/");
-               HttpUtility::SendJsonError(response, 404, "The requested path is too long to match any config tag requests.");
-               return true;
-       }
+       if (request.RequestUrl->GetPath().size() > 5)
+               return false;
 
        if (request.RequestMethod == "GET")
                HandleGet(user, request, response);
@@ -45,7 +42,7 @@ bool ConfigStagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r
        else if (request.RequestMethod == "DELETE")
                HandleDelete(user, request, response);
        else
-               HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET, POST or DELETE.");
+               return false;
 
        return true;
 }
@@ -66,10 +63,10 @@ void ConfigStagesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& reque
        String stageName = HttpUtility::GetLastParameter(params, "stage");
 
        if (!ConfigPackageUtility::ValidateName(packageName))
-               return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+               return HttpUtility::SendJsonError(response, 400, "Invalid package name.");
 
        if (!ConfigPackageUtility::ValidateName(stageName))
-               return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist.");
+               return HttpUtility::SendJsonError(response, 400, "Invalid stage name.");
 
        Array::Ptr results = new Array();
 
@@ -104,7 +101,7 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ
        String packageName = HttpUtility::GetLastParameter(params, "package");
 
        if (!ConfigPackageUtility::ValidateName(packageName))
-               return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+               return HttpUtility::SendJsonError(response, 400, "Invalid package name.");
 
        Dictionary::Ptr files = params->Get("files");
 
@@ -155,10 +152,10 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re
        String stageName = HttpUtility::GetLastParameter(params, "stage");
 
        if (!ConfigPackageUtility::ValidateName(packageName))
-               return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist.");
+               return HttpUtility::SendJsonError(response, 400, "Invalid package name.");
 
        if (!ConfigPackageUtility::ValidateName(stageName))
-               return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist.");
+               return HttpUtility::SendJsonError(response, 400, "Invalid stage name.");
 
        try {
                ConfigPackageUtility::DeleteStage(packageName, stageName);
@@ -171,7 +168,7 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re
        Dictionary::Ptr result1 = new Dictionary();
 
        result1->Set("code", 200);
-       result1->Set("status", "Stage deleted");
+       result1->Set("status", "Stage deleted.");
 
        Array::Ptr results = new Array();
        results->Add(result1);

diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp
index 3e0770e3932fe926cd1c1c94a35f6c57f59edcc1..b73b5f317ee651f24d987c165bf33ab6de59204a 100644 (file)
--- a/lib/remote/createobjecthandler.cpp
+++ b/lib/remote/createobjecthandler.cpp
@@ -31,20 +31,16 @@ REGISTER_URLHANDLER("/v1/objects", CreateObjectHandler);
 
 bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod != "PUT") {
-               /* there might be other request methods pending */
+       if (request.RequestUrl->GetPath().size() != 4)
                return false;
-       }
 
-       if (request.RequestUrl->GetPath().size() < 4) {
-               HttpUtility::SendJsonError(response, 400, "Object name is missing.");
-               return true;
-       }
+       if (request.RequestMethod != "PUT")
+               return false;
 
        Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[2]);
 
        if (!type) {
-               HttpUtility::SendJsonError(response, 403, "Erroneous type was supplied.");
+               HttpUtility::SendJsonError(response, 400, "Invalid type specified.");
                return true;
        }
 

diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp
index 8d4384672fd514b455fdbc93062e79462f8d626d..10f30cd93b7b4b8047614a7472ebfd5e72dc4f44 100644 (file)
--- a/lib/remote/deleteobjecthandler.cpp
+++ b/lib/remote/deleteobjecthandler.cpp
@@ -34,21 +34,16 @@ REGISTER_URLHANDLER("/v1/objects", DeleteObjectHandler);
 
 bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod != "DELETE") {
-               /* there might be other request methods pending */
+       if (request.RequestUrl->GetPath().size() < 3 || request.RequestUrl->GetPath().size() > 4)
                return false;
-       }
 
-       if (request.RequestUrl->GetPath().size() < 3) {
-               String path = boost::algorithm::join(request.RequestUrl->GetPath(), "/");
-               HttpUtility::SendJsonError(response, 404, "The requested path is too long to match any config tag requests.");
-               return true;
-       }
+       if (request.RequestMethod != "DELETE")
+               return false;
 
        Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[2]);
 
        if (!type) {
-               HttpUtility::SendJsonError(response, 400, "Erroneous type was supplied.");
+               HttpUtility::SendJsonError(response, 400, "Invalid type specified.");
                return true;
        }
 

diff --git a/lib/remote/httphandler.cpp b/lib/remote/httphandler.cpp
index 64da5d2ed460cae7684b640c62bdf494befe4ff5..51e0ab47700f68ac3400abc3eb94ab59a8937657 100644 (file)
--- a/lib/remote/httphandler.cpp
+++ b/lib/remote/httphandler.cpp
@@ -103,8 +103,8 @@ void HttpHandler::ProcessRequest(const ApiUser::Ptr& user, HttpRequest& request,
        }
        if (!processed) {
                String path = boost::algorithm::join(request.RequestUrl->GetPath(), "/");
-               HttpUtility::SendJsonError(response, 404, "The requested API '" +  path +
-                               "' could not be found. Please check it for common errors like spelling and consult the docs.");
+               HttpUtility::SendJsonError(response, 404, "The requested path '" + path +
+                               "' could not be found.");
                return;
        }
 }

diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp
index 12c8b343c8877ce2c10e739a6ac75847b7f83852..2c2ed5a8f1dd9c2d42a3d2a1c152d33d5cb12373 100644 (file)
--- a/lib/remote/modifyobjecthandler.cpp
+++ b/lib/remote/modifyobjecthandler.cpp
@@ -32,18 +32,18 @@ REGISTER_URLHANDLER("/v1/objects", ModifyObjectHandler);
 
 bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod != "POST") {
-               /* there might be other request methods pending */
+       if (request.RequestUrl->GetPath().size() < 3 || request.RequestUrl->GetPath().size() > 4)
                return false;
-       }
 
-       if (request.RequestUrl->GetPath().size() < 3)
+       if (request.RequestMethod != "POST")
                return false;
 
        Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[1]);
 
-       if (!type)
-               return false;
+       if (!type) {
+               HttpUtility::SendJsonError(response, 400, "Invalid type specified.");
+               return true;
+       }
 
        QueryDescription qd;
        qd.Types.insert(type->GetName());
@@ -100,4 +100,3 @@ bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r
 
        return true;
 }
-

diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp
index ae8276a19240668f38d6708ee124a5e255195227..7b1182f8abbdc656bee0cd6d3a67991fb824b79f 100644 (file)
--- a/lib/remote/objectqueryhandler.cpp
+++ b/lib/remote/objectqueryhandler.cpp
@@ -32,16 +32,18 @@ REGISTER_URLHANDLER("/v1/objects", ObjectQueryHandler);
 
 bool ObjectQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       if (request.RequestMethod != "GET")
+       if (request.RequestUrl->GetPath().size() < 3 || request.RequestUrl->GetPath().size() > 4)
                return false;
 
-       if (request.RequestUrl->GetPath().size() < 3)
+       if (request.RequestMethod != "GET")
                return false;
 
        Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[2]);
 
-       if (!type)
-               return false;
+       if (!type) {
+               HttpUtility::SendJsonError(response, 400, "Invalid type specified.");
+               return true;
+       }
 
        QueryDescription qd;
        qd.Types.insert(type->GetName());

diff --git a/lib/remote/statushandler.cpp b/lib/remote/statushandler.cpp
index d49a69f7e0fbb48e732bd7d1843c607c9387f3bf..810acf61f6c48be56370f6ea45e259f1549ea90c 100644 (file)
--- a/lib/remote/statushandler.cpp
+++ b/lib/remote/statushandler.cpp
@@ -71,15 +71,11 @@ public:
 
 bool StatusHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       Dictionary::Ptr result = new Dictionary();
-
-       if (request.RequestMethod != "GET") {
-               response.SetStatus(400, "Bad request");
-               result->Set("info", "Request must be type GET");
-               HttpUtility::SendJsonBody(response, result);
-               return true;
-       }
+       if (request.RequestUrl->GetPath().size() > 3)
+               return false;
 
+       if (request.RequestMethod != "GET")
+               return false;
 
        QueryDescription qd;
        qd.Types.insert("Status");
@@ -97,6 +93,7 @@ bool StatusHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request
 
        Array::Ptr results = Array::FromVector(objs);
 
+       Dictionary::Ptr result = new Dictionary();
        result->Set("results", results);
 
        response.SetStatus(200, "OK");

diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp
index 966323e818a253b4900ac08d0d2c6fc3e31bd37f..c278177cbe359e3a1d474c0c970e8ff096546c3b 100644 (file)
--- a/lib/remote/typequeryhandler.cpp
+++ b/lib/remote/typequeryhandler.cpp
@@ -77,14 +77,11 @@ public:
 
 bool TypeQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response)
 {
-       Dictionary::Ptr result = new Dictionary();
+       if (request.RequestUrl->GetPath().size() > 3)
+               return false;
 
-       if (request.RequestMethod != "GET") {
-               response.SetStatus(400, "Bad request");
-               result->Set("info", "Request must be type GET");
-               HttpUtility::SendJsonBody(response, result);
-               return true;
-       }
+       if (request.RequestMethod != "GET")
+               return false;
 
        QueryDescription qd;
        qd.Types.insert("Type");
@@ -157,6 +154,7 @@ bool TypeQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& requ
                }
        }
 
+       Dictionary::Ptr result = new Dictionary();
        result->Set("results", results);
 
        response.SetStatus(200, "OK");