From: Jean-Marcel Flach Date: Tue, 22 Sep 2015 15:58:12 +0000 (+0200) Subject: Improve API error handling and fix some whitespace X-Git-Tag: v2.4.0~276 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5ef4204d06216dc4e70d63f3741d871f459c61b6;p=icinga2 Improve API error handling and fix some whitespace fixes #10194 --- diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 26fb662c6..7db830b67 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -33,11 +33,13 @@ REGISTER_URLHANDLER("/v1/actions", ActionsHandler); bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) { - if (request.RequestUrl->GetPath().size() < 3) - return false; - if (request.RequestMethod != "POST") { - response.SetStatus(400, "Bad request"); + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be POST."); + return true; + } + + if (request.RequestUrl->GetPath().size() < 3) { + HttpUtility::SendJsonError(response, 400, "Action is missing."); return true; } @@ -45,8 +47,10 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques ApiAction::Ptr action = ApiAction::GetByName(actionName); - if (!action) - return false; + if (!action) { + HttpUtility::SendJsonError(response, 404, "Action '" + actionName + "' could not be found."); + return true; + } QueryDescription qd; @@ -58,7 +62,14 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques if (!types.empty()) { qd.Types = std::set(types.begin(), types.end()); - objs = FilterUtility::GetFilterTargets(qd, params); + try { + objs = FilterUtility::GetFilterTargets(qd, params); + } catch (const std::exception& ex) { + HttpUtility::SendJsonError(response, 400, + "Type/Filter was required but not provided or was invalid.", + request.GetVerboseErrors() ? DiagnosticInformation(ex) : ""); + return true; + } } else objs.push_back(ConfigObject::Ptr()); @@ -72,8 +83,10 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques results->Add(action->Invoke(obj, params)); } catch (const std::exception& ex) { Dictionary::Ptr fail = new Dictionary(); - fail->Set("code", 501); - fail->Set("status", "Error: " + DiagnosticInformation(ex)); + fail->Set("code", 500); + fail->Set("status", "Action execution failed."); + if (request.GetVerboseErrors()) + fail->Set("diagnostic information", DiagnosticInformation(ex)); results->Add(fail); } } diff --git a/lib/remote/apilistener-configsync.cpp b/lib/remote/apilistener-configsync.cpp index d15fbfcdc..e40fb2d18 100644 --- a/lib/remote/apilistener-configsync.cpp +++ b/lib/remote/apilistener-configsync.cpp @@ -227,12 +227,12 @@ Value ApiListener::ConfigDeleteObjectAPIHandler(const MessageOrigin::Ptr& origin Array::Ptr errors = new Array(); bool cascade = true; //TODO pass that through the cluster if (!ConfigObjectUtility::DeleteObject(object, cascade, errors)) { - Log(LogCritical, "ApiListener", "Could not delete object:"); + Log(LogCritical, "ApiListener", "Could not delete object:"); - ObjectLock olock(errors); - BOOST_FOREACH(const String& error, errors) { - Log(LogCritical, "ApiListener", error); - } + ObjectLock olock(errors); + BOOST_FOREACH(const String& error, errors) { + Log(LogCritical, "ApiListener", error); + } } } else { Log(LogNotice, "ApiListener") diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 4cd9be70d..c2914d013 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -57,13 +57,15 @@ void ApiListener::OnConfigLoaded(void) try { cert = GetX509Certificate(GetCertPath()); } catch (const std::exception&) { - BOOST_THROW_EXCEPTION(ScriptError("Cannot get certificate from cert path: '" + GetCertPath() + "'.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Cannot get certificate from cert path: '" + + GetCertPath() + "'.", GetDebugInfo())); } try { SetIdentity(GetCertificateCN(cert)); } catch (const std::exception&) { - BOOST_THROW_EXCEPTION(ScriptError("Cannot get certificate common name from cert path: '" + GetCertPath() + "'.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Cannot get certificate common name from cert path: '" + + GetCertPath() + "'.", GetDebugInfo())); } Log(LogInformation, "ApiListener") @@ -72,14 +74,16 @@ void ApiListener::OnConfigLoaded(void) try { m_SSLContext = MakeSSLContext(GetCertPath(), GetKeyPath(), GetCaPath()); } catch (const std::exception&) { - BOOST_THROW_EXCEPTION(ScriptError("Cannot make SSL context for cert path: '" + GetCertPath() + "' key path: '" + GetKeyPath() + "' ca path: '" + GetCaPath() + "'.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Cannot make SSL context for cert path: '" + + GetCertPath() + "' key path: '" + GetKeyPath() + "' ca path: '" + GetCaPath() + "'.", GetDebugInfo())); } if (!GetCrlPath().IsEmpty()) { try { AddCRLToSSLContext(m_SSLContext, GetCrlPath()); } catch (const std::exception&) { - BOOST_THROW_EXCEPTION(ScriptError("Cannot add certificate revocation list to SSL context for crl path: '" + GetCrlPath() + "'.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Cannot add certificate revocation list to SSL context for crl path: '" + + GetCrlPath() + "'.", GetDebugInfo())); } } } @@ -97,7 +101,8 @@ void ApiListener::Start(void) { SyncZoneDirs(); - if (std::distance(ConfigType::GetObjectsByType().first, ConfigType::GetObjectsByType().second) > 1) { + if (std::distance(ConfigType::GetObjectsByType().first, + ConfigType::GetObjectsByType().second) > 1) { Log(LogCritical, "ApiListener", "Only one ApiListener object is allowed."); return; } @@ -433,7 +438,8 @@ void ApiListener::ApiTimerHandler(void) /* only connect to endpoints in a) the same zone b) our parent zone c) immediate child zones */ if (my_zone != zone && my_zone != zone->GetParent() && zone != my_zone->GetParent()) { Log(LogDebug, "ApiListener") - << "Not connecting to Zone '" << zone->GetName() << "' because it's not in the same zone, a parent or a child zone."; + << "Not connecting to Zone '" << zone->GetName() + << "' because it's not in the same zone, a parent or a child zone."; continue; } @@ -448,21 +454,24 @@ void ApiListener::ApiTimerHandler(void) /* don't try to connect to endpoints which don't have a host and port */ if (endpoint->GetHost().IsEmpty() || endpoint->GetPort().IsEmpty()) { Log(LogDebug, "ApiListener") - << "Not connecting to Endpoint '" << endpoint->GetName() << "' because the host/port attributes are missing."; + << "Not connecting to Endpoint '" << endpoint->GetName() + << "' because the host/port attributes are missing."; continue; } /* don't try to connect if there's already a connection attempt */ if (endpoint->GetConnecting()) { Log(LogDebug, "ApiListener") - << "Not connecting to Endpoint '" << endpoint->GetName() << "' because we're already trying to connect to it."; + << "Not connecting to Endpoint '" << endpoint->GetName() + << "' because we're already trying to connect to it."; continue; } /* don't try to connect if we're already connected */ if (endpoint->IsConnected()) { Log(LogDebug, "ApiListener") - << "Not connecting to Endpoint '" << endpoint->GetName() << "' because we're already connected to it."; + << "Not connecting to Endpoint '" << endpoint->GetName() + << "' because we're already connected to it."; continue; } @@ -511,7 +520,8 @@ void ApiListener::ApiTimerHandler(void) << "Connected endpoints: " << Utility::NaturalJoin(names); } -void ApiListener::RelayMessage(const MessageOrigin::Ptr& origin, const ConfigObject::Ptr& secobj, const Dictionary::Ptr& message, bool log) +void ApiListener::RelayMessage(const MessageOrigin::Ptr& origin, + const ConfigObject::Ptr& secobj, const Dictionary::Ptr& message, bool log) { m_RelayQueue.Enqueue(boost::bind(&ApiListener::SyncRelayMessage, this, origin, secobj, message, log), true); } @@ -526,7 +536,6 @@ void ApiListener::PersistMessage(const Dictionary::Ptr& message, const ConfigObj pmessage->Set("timestamp", ts); pmessage->Set("message", JsonEncode(message)); - Dictionary::Ptr secname = new Dictionary(); secname->Set("type", secobj->GetType()->GetName()); secname->Set("name", secobj->GetName()); @@ -560,7 +569,8 @@ void ApiListener::SyncSendMessage(const Endpoint::Ptr& endpoint, const Dictionar } -void ApiListener::SyncRelayMessage(const MessageOrigin::Ptr& origin, const ConfigObject::Ptr& secobj, const Dictionary::Ptr& message, bool log) +void ApiListener::SyncRelayMessage(const MessageOrigin::Ptr& origin, + const ConfigObject::Ptr& secobj, const Dictionary::Ptr& message, bool log) { double ts = Utility::GetTime(); message->Set("ts", ts); @@ -704,12 +714,12 @@ void ApiListener::ReplayLog(const JsonRpcConnection::Ptr& client) double peer_ts = endpoint->GetLocalLogPosition(); double logpos_ts = peer_ts; bool last_sync = false; - + Endpoint::Ptr target_endpoint = client->GetEndpoint(); ASSERT(target_endpoint); - + Zone::Ptr target_zone = target_endpoint->GetZone(); - + if (!target_zone) return; @@ -771,18 +781,18 @@ void ApiListener::ReplayLog(const JsonRpcConnection::Ptr& client) continue; Dictionary::Ptr secname = pmessage->Get("secobj"); - + if (secname) { ConfigType::Ptr dtype = ConfigType::GetByName(secname->Get("type")); - + if (!dtype) continue; - + ConfigObject::Ptr secobj = dtype->GetObject(secname->Get("name")); - + if (!secobj) continue; - + if (!target_zone->CanAccessObject(secobj)) continue; } diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp index 5532bcf96..28971010f 100644 --- a/lib/remote/configfileshandler.cpp +++ b/lib/remote/configfileshandler.cpp @@ -33,7 +33,7 @@ bool ConfigFilesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re if (request.RequestMethod == "GET") HandleGet(user, request, response); else - response.SetStatus(400, "Bad request"); + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET."); return true; } @@ -58,24 +58,21 @@ 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) || !ConfigPackageUtility::ValidateName(stageName)) { - response.SetStatus(403, "Forbidden"); - return; - } + if (!ConfigPackageUtility::ValidateName(packageName)) + return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); + + if (!ConfigPackageUtility::ValidateName(stageName)) + return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist."); String relativePath = HttpUtility::GetLastParameter(params, "path"); - if (ConfigPackageUtility::ContainsDotDot(relativePath)) { - response.SetStatus(403, "Forbidden"); - return; - } + if (ConfigPackageUtility::ContainsDotDot(relativePath)) + return HttpUtility::SendJsonError(response, 403, "Path contains '..' (not allowed)."); String path = ConfigPackageUtility::GetPackageDir() + "/" + packageName + "/" + stageName + "/" + relativePath; - if (!Utility::PathExists(path)) { - response.SetStatus(404, "File not found"); - return; - } + if (!Utility::PathExists(path)) + return HttpUtility::SendJsonError(response, 404, "Path not found."); try { std::ifstream fp(path.CStr(), std::ifstream::in | std::ifstream::binary); @@ -86,7 +83,8 @@ 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) { - response.SetStatus(503, "Could not read file"); + return HttpUtility::SendJsonError(response, 500, "Could not read file.", + request.GetVerboseErrors() ? DiagnosticInformation(ex) : ""); } } diff --git a/lib/remote/configobjectutility.cpp b/lib/remote/configobjectutility.cpp index 77106cc8d..eb62f8405 100644 --- a/lib/remote/configobjectutility.cpp +++ b/lib/remote/configobjectutility.cpp @@ -78,7 +78,7 @@ String ConfigObjectUtility::CreateObjectConfig(const Type::Ptr& type, const Stri ConfigWriter::EmitConfigItem(config, type->GetName(), name, false, templates, allAttrs); ConfigWriter::EmitRaw(config, "\n"); - return config.str(); + return config.str(); } bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& fullName, @@ -135,7 +135,8 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo if (!parents.empty() && !cascade) { if (errors) - errors->Add("Object cannot be deleted because other objects depend on it. Use cascading delete to delete it anyway."); + errors->Add("Object cannot be deleted because other objects depend on it. " + "Use cascading delete to delete it anyway."); return false; } diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 47ce7cd6e..946c0c7ac 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -21,6 +21,7 @@ #include "remote/configpackageutility.hpp" #include "remote/httputility.hpp" #include "base/exception.hpp" +#include using namespace icinga; @@ -28,8 +29,11 @@ REGISTER_URLHANDLER("/v1/config/packages", ConfigPackagesHandler); bool ConfigPackagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) { - if (request.RequestUrl->GetPath().size() > 4) - return false; + 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.RequestMethod == "GET") HandleGet(user, request, response); @@ -38,7 +42,7 @@ bool ConfigPackagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& else if (request.RequestMethod == "DELETE") HandleDelete(user, request, response); else - response.SetStatus(400, "Bad request"); + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET, POST or DELETE."); return true; } @@ -74,25 +78,21 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re String packageName = HttpUtility::GetLastParameter(params, "package"); if (!ConfigPackageUtility::ValidateName(packageName)) { - response.SetStatus(403, "Forbidden"); + HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); return; } - int code = 200; - String status = "Created package."; + Dictionary::Ptr result1 = new Dictionary(); try { ConfigPackageUtility::CreatePackage(packageName); } catch (const std::exception& ex) { - code = 501; - status = "Error: " + DiagnosticInformation(ex); + HttpUtility::SendJsonError(response, 500, "Could not create package.", + request.GetVerboseErrors() ? DiagnosticInformation(ex) : ""); } - Dictionary::Ptr result1 = new Dictionary(); - - result1->Set("package", packageName); - result1->Set("code", code); - result1->Set("status", status); + result1->Set("code", 200); + result1->Set("status", "Created package."); Array::Ptr results = new Array(); results->Add(result1); @@ -100,7 +100,7 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re Dictionary::Ptr result = new Dictionary(); result->Set("results", results); - response.SetStatus(code, (code == 200) ? "OK" : "Error"); + response.SetStatus(200, "OK"); HttpUtility::SendJsonBody(response, result); } @@ -114,21 +114,23 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& String packageName = HttpUtility::GetLastParameter(params, "package"); if (!ConfigPackageUtility::ValidateName(packageName)) { - response.SetStatus(403, "Forbidden"); + HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); return; } int code = 200; String status = "Deleted package."; + Dictionary::Ptr result1 = new Dictionary(); try { ConfigPackageUtility::DeletePackage(packageName); } catch (const std::exception& ex) { - code = 501; - status = "Error: " + DiagnosticInformation(ex); + code = 500; + status = "Failed to delete package."; + if (request.GetVerboseErrors()) + result1->Set("diagnostic information", DiagnosticInformation(ex)); } - Dictionary::Ptr result1 = new Dictionary(); result1->Set("package", packageName); result1->Set("code", code); @@ -140,7 +142,7 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& Dictionary::Ptr result = new Dictionary(); result->Set("results", results); - response.SetStatus(code, (code == 200) ? "OK" : "Error"); + response.SetStatus(code, (code == 200) ? "OK" : "Internal Server Error"); HttpUtility::SendJsonBody(response, result); } diff --git a/lib/remote/configpackageutility.cpp b/lib/remote/configpackageutility.cpp index 79cb182f1..54d422962 100644 --- a/lib/remote/configpackageutility.cpp +++ b/lib/remote/configpackageutility.cpp @@ -60,7 +60,8 @@ void ConfigPackageUtility::DeletePackage(const String& name) std::vector ConfigPackageUtility::GetPackages(void) { std::vector packages; - Utility::Glob(GetPackageDir() + "/*", boost::bind(&ConfigPackageUtility::CollectDirNames, _1, boost::ref(packages)), GlobDirectory); + Utility::Glob(GetPackageDir() + "/*", boost::bind(&ConfigPackageUtility::CollectDirNames, + _1, boost::ref(packages)), GlobDirectory); return packages; } @@ -92,7 +93,7 @@ String ConfigPackageUtility::CreateStage(const String& packageName, const Dictio WriteStageConfig(packageName, stageName); bool foundDotDot = false; - + if (files) { ObjectLock olock(files); BOOST_FOREACH(const Dictionary::Pair& kv, files) { @@ -100,12 +101,12 @@ String ConfigPackageUtility::CreateStage(const String& packageName, const Dictio foundDotDot = true; break; } - + String filePath = path + "/" + kv.first; - + Log(LogInformation, "ConfigPackageUtility") << "Updating configuration file: " << filePath; - + //pass the directory and generate a dir tree, if not existing already Utility::MkDirP(Utility::DirName(filePath), 0750); std::ofstream fp(filePath.CStr(), std::ofstream::out | std::ostream::binary | std::ostream::trunc); @@ -313,4 +314,3 @@ bool ConfigPackageUtility::ValidateName(const String& name) return (!boost::regex_search(name.GetData(), what, expr)); } - diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index b3b7094d1..f7e801d78 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -23,6 +23,7 @@ #include "base/application.hpp" #include "base/exception.hpp" #include +#include using namespace icinga; @@ -30,8 +31,11 @@ REGISTER_URLHANDLER("/v1/config/stages", ConfigStagesHandler); bool ConfigStagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) { - if (request.RequestUrl->GetPath().size() > 5) - return false; + 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.RequestMethod == "GET") HandleGet(user, request, response); @@ -40,7 +44,7 @@ bool ConfigStagesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r else if (request.RequestMethod == "DELETE") HandleDelete(user, request, response); else - response.SetStatus(400, "Bad request"); + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be GET, POST or DELETE."); return true; } @@ -58,10 +62,11 @@ void ConfigStagesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& reque String packageName = HttpUtility::GetLastParameter(params, "package"); String stageName = HttpUtility::GetLastParameter(params, "stage"); - if (!ConfigPackageUtility::ValidateName(packageName) || !ConfigPackageUtility::ValidateName(stageName)) { - response.SetStatus(403, "Forbidden"); - return; - } + if (!ConfigPackageUtility::ValidateName(packageName)) + return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); + + if (!ConfigPackageUtility::ValidateName(stageName)) + return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist."); Array::Ptr results = new Array(); @@ -93,15 +98,11 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ String packageName = HttpUtility::GetLastParameter(params, "package"); - if (!ConfigPackageUtility::ValidateName(packageName)) { - response.SetStatus(403, "Forbidden"); - return; - } + if (!ConfigPackageUtility::ValidateName(packageName)) + return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); Dictionary::Ptr files = params->Get("files"); - int code = 200; - String status = "Created stage."; String stageName; try { @@ -113,16 +114,15 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ /* validate the config. on success, activate stage and reload */ ConfigPackageUtility::AsyncTryActivateStage(packageName, stageName); } catch (const std::exception& ex) { - code = 501; - status = "Error: " + DiagnosticInformation(ex); + return HttpUtility::SendJsonError(response, 500, + "Stage creation failed.", + request.GetVerboseErrors() ? DiagnosticInformation(ex) : ""); } Dictionary::Ptr result1 = new Dictionary(); - result1->Set("package", packageName); - result1->Set("stage", stageName); - result1->Set("code", code); - result1->Set("status", status); + result1->Set("code", 200); + result1->Set("status", "Created stage."); Array::Ptr results = new Array(); results->Add(result1); @@ -130,7 +130,7 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ Dictionary::Ptr result = new Dictionary(); result->Set("results", results); - response.SetStatus(code, (code == 200) ? "OK" : "Error"); + response.SetStatus(200, "OK"); HttpUtility::SendJsonBody(response, result); } @@ -147,27 +147,24 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re String packageName = HttpUtility::GetLastParameter(params, "package"); String stageName = HttpUtility::GetLastParameter(params, "stage"); - if (!ConfigPackageUtility::ValidateName(packageName) || !ConfigPackageUtility::ValidateName(stageName)) { - response.SetStatus(403, "Forbidden"); - return; - } + if (!ConfigPackageUtility::ValidateName(packageName)) + return HttpUtility::SendJsonError(response, 404, "Package is not valid or does not exist."); - int code = 200; - String status = "Deleted stage."; + if (!ConfigPackageUtility::ValidateName(stageName)) + return HttpUtility::SendJsonError(response, 404, "Stage is not valid or does not exist."); try { ConfigPackageUtility::DeleteStage(packageName, stageName); } catch (const std::exception& ex) { - code = 501; - status = "Error: " + DiagnosticInformation(ex); + return HttpUtility::SendJsonError(response, 500, + "Failed to delete stage.", + request.GetVerboseErrors() ? DiagnosticInformation(ex) : ""); } Dictionary::Ptr result1 = new Dictionary(); - result1->Set("package", packageName); - result1->Set("stage", stageName); - result1->Set("code", code); - result1->Set("status", status); + result1->Set("code", 200); + result1->Set("status", "Stage deleted"); Array::Ptr results = new Array(); results->Add(result1); @@ -175,7 +172,7 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re Dictionary::Ptr result = new Dictionary(); result->Set("results", results); - response.SetStatus(code, (code == 200) ? "OK" : "Error"); + response.SetStatus(200, "OK"); HttpUtility::SendJsonBody(response, result); } diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index 7e22e85fb..0222413e3 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -31,16 +31,22 @@ REGISTER_URLHANDLER("/v1", CreateObjectHandler); bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) { - if (request.RequestMethod != "PUT") - return false; + if (request.RequestMethod != "PUT") { + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be PUT."); + return true; + } - if (request.RequestUrl->GetPath().size() < 3) - return false; + if (request.RequestUrl->GetPath().size() < 3) { + HttpUtility::SendJsonError(response, 400, "Object name is missing."); + return true; + } Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[1]); - if (!type) - return false; + if (!type) { + HttpUtility::SendJsonError(response, 403, "Erroneous type was supplied."); + return true; + } String name = request.RequestUrl->GetPath()[2]; Dictionary::Ptr params = HttpUtility::FetchRequestParameters(request); @@ -51,20 +57,17 @@ bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r int code; String status; Array::Ptr errors = new Array(); - + String config = ConfigObjectUtility::CreateObjectConfig(type, name, templates, attrs); - + if (!ConfigObjectUtility::CreateObject(type, name, config, errors)) { result1->Set("errors", errors); - code = 500; - status = "Object could not be created."; - } else { - code = 200; - status = "Object was created."; + HttpUtility::SendJsonError(response, 500, "Object could not be created."); + return true; } - result1->Set("code", code); - result1->Set("status", status); + result1->Set("code", 200); + result1->Set("status", "Object was created"); Array::Ptr results = new Array(); results->Add(result1); @@ -72,7 +75,7 @@ bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r Dictionary::Ptr result = new Dictionary(); result->Set("results", results); - response.SetStatus(code, status); + response.SetStatus(200, "OK"); HttpUtility::SendJsonBody(response, result); return true; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 792734ab7..7d797ce73 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -34,16 +34,23 @@ REGISTER_URLHANDLER("/v1", DeleteObjectHandler); bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request, HttpResponse& response) { - if (request.RequestMethod != "DELETE") - return false; + if (request.RequestMethod != "DELETE") { + HttpUtility::SendJsonError(response, 400, "Invalid request type. Must be DELETE."); + return true; + } - if (request.RequestUrl->GetPath().size() < 2) - return false; + if (request.RequestUrl->GetPath().size() < 2) { + 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; + } Type::Ptr type = FilterUtility::TypeFromPluralName(request.RequestUrl->GetPath()[1]); - if (!type) - return false; + if (!type) { + HttpUtility::SendJsonError(response, 400, "Erroneous type was supplied."); + return true; + } QueryDescription qd; qd.Types.insert(type->GetName()); @@ -71,9 +78,9 @@ bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r results->Add(result1); Array::Ptr errors = new Array(); - + if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors)) { - result1->Set("code", 500); + result1->Set("code", 500); result1->Set("status", "Object could not be deleted."); result1->Set("errors", errors); } else { diff --git a/lib/remote/endpoint.cpp b/lib/remote/endpoint.cpp index 29068ac0d..b3cd83ea5 100644 --- a/lib/remote/endpoint.cpp +++ b/lib/remote/endpoint.cpp @@ -47,14 +47,16 @@ void Endpoint::OnAllConfigLoaded(void) if (members.find(this) != members.end()) { if (m_Zone) - BOOST_THROW_EXCEPTION(ScriptError("Endpoint '" + GetName() + "' is in more than one zone.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Endpoint '" + GetName() + + "' is in more than one zone.", GetDebugInfo())); m_Zone = zone; } } if (!m_Zone) - BOOST_THROW_EXCEPTION(ScriptError("Endpoint '" + GetName() + "' does not belong to a zone.", GetDebugInfo())); + BOOST_THROW_EXCEPTION(ScriptError("Endpoint '" + GetName() + + "' does not belong to a zone.", GetDebugInfo())); } void Endpoint::AddClient(const JsonRpcConnection::Ptr& client) diff --git a/lib/remote/filterutility.cpp b/lib/remote/filterutility.cpp index b91d4ce76..a127172a7 100644 --- a/lib/remote/filterutility.cpp +++ b/lib/remote/filterutility.cpp @@ -183,3 +183,4 @@ std::vector FilterUtility::GetFilterTargets(const QueryDescription& qd, c return result; } + diff --git a/lib/remote/httpclientconnection.cpp b/lib/remote/httpclientconnection.cpp index 4837ed3ec..e6688df71 100644 --- a/lib/remote/httpclientconnection.cpp +++ b/lib/remote/httpclientconnection.cpp @@ -56,7 +56,8 @@ void HttpClientConnection::Reconnect(void) m_Stream = new TlsStream(socket, m_Host, RoleClient); else ASSERT(!"Non-TLS HTTP connections not supported."); - //m_Stream = new NetworkStream(socket); -- does not currently work because the NetworkStream class doesn't support async I/O + /* m_Stream = new NetworkStream(socket); + -- does not currently work because the NetworkStream class doesn't support async I/O */ m_Stream->RegisterDataHandler(boost::bind(&HttpClientConnection::DataAvailableHandler, this)); if (m_Stream->IsDataAvailable()) @@ -149,8 +150,10 @@ boost::shared_ptr HttpClientConnection::NewRequest(void) return boost::make_shared(m_Stream); } -void HttpClientConnection::SubmitRequest(const boost::shared_ptr& request, const HttpCompletionCallback& callback) +void HttpClientConnection::SubmitRequest(const boost::shared_ptr& request, + const HttpCompletionCallback& callback) { m_Requests.push_back(std::make_pair(request, callback)); request->Finish(); } + diff --git a/lib/remote/httphandler.cpp b/lib/remote/httphandler.cpp index 0e580fde3..64da5d2ed 100644 --- a/lib/remote/httphandler.cpp +++ b/lib/remote/httphandler.cpp @@ -18,7 +18,9 @@ ******************************************************************************/ #include "remote/httphandler.hpp" +#include "remote/httputility.hpp" #include "base/singleton.hpp" +#include using namespace icinga; @@ -100,10 +102,10 @@ void HttpHandler::ProcessRequest(const ApiUser::Ptr& user, HttpRequest& request, } } if (!processed) { - response.SetStatus(404, "Not found"); - response.AddHeader("Content-Type", "text/html"); - String msg = "

Not found

"; - response.WriteBody(msg.CStr(), msg.GetLength()); + 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."); return; } } + diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index c2c8f116f..d09a0166d 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -34,7 +34,8 @@ HttpRequest::HttpRequest(const Stream::Ptr& stream) ProtocolVersion(HttpVersion11), Headers(new Dictionary()), m_Stream(stream), - m_State(HttpRequestStart) + m_State(HttpRequestStart), + verboseErrors(false) { } bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) @@ -57,8 +58,10 @@ bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) << "line: " << line << ", tokens: " << tokens.size(); if (tokens.size() != 3) BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); + RequestMethod = tokens[0]; RequestUrl = new class Url(tokens[1]); + verboseErrors = (RequestUrl->GetQueryElement("verboseErrors") == "true"); if (tokens[2] == "HTTP/1.0") ProtocolVersion = HttpVersion10; @@ -84,8 +87,8 @@ bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) String::SizeType pos = line.FindFirstOf(":"); if (pos == String::NPos) BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); - String key = line.SubStr(0, pos).ToLower().Trim(); + String key = line.SubStr(0, pos).ToLower().Trim(); String value = line.SubStr(pos + 1).Trim(); Headers->Set(key, value); @@ -230,3 +233,4 @@ void HttpRequest::Finish(void) m_State = HttpRequestEnd; } + diff --git a/lib/remote/httprequest.hpp b/lib/remote/httprequest.hpp index 9eb1a0b21..6542a7028 100644 --- a/lib/remote/httprequest.hpp +++ b/lib/remote/httprequest.hpp @@ -69,11 +69,15 @@ public: void WriteBody(const char *data, size_t count); void Finish(void); + inline bool GetVerboseErrors(void) + { return verboseErrors; } + private: Stream::Ptr m_Stream; boost::shared_ptr m_ChunkContext; HttpRequestState m_State; FIFO::Ptr m_Body; + bool verboseErrors; void FinishHeaders(void); }; diff --git a/lib/remote/httpresponse.cpp b/lib/remote/httpresponse.cpp index edb9eda76..f129f51c7 100644 --- a/lib/remote/httpresponse.cpp +++ b/lib/remote/httpresponse.cpp @@ -64,7 +64,7 @@ void HttpResponse::FinishHeaders(void) if (m_State == HttpResponseHeaders) { if (m_Request.ProtocolVersion == HttpVersion11) AddHeader("Transfer-Encoding", "chunked"); - + AddHeader("Server", "Icinga/" + Application::GetAppVersion()); m_Stream->Write("\r\n", 2); m_State = HttpResponseBody; @@ -232,3 +232,4 @@ size_t HttpResponse::ReadBody(char *data, size_t count) else return m_Body->Read(data, count, true); } + diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index e97d9c2c7..2c7e18902 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -98,7 +98,8 @@ bool HttpServerConnection::ProcessMessage(void) } if (m_CurrentRequest.Complete) { - m_RequestQueue.Enqueue(boost::bind(&HttpServerConnection::ProcessMessageAsync, HttpServerConnection::Ptr(this), m_CurrentRequest)); + m_RequestQueue.Enqueue(boost::bind(&HttpServerConnection::ProcessMessageAsync, + HttpServerConnection::Ptr(this), m_CurrentRequest)); m_Seen = Utility::GetTime(); m_PendingRequests++; @@ -200,3 +201,4 @@ void HttpServerConnection::TimeoutTimerHandler(void) client->CheckLiveness(); } } + diff --git a/lib/remote/httputility.cpp b/lib/remote/httputility.cpp index 50e388463..cdf10213c 100644 --- a/lib/remote/httputility.cpp +++ b/lib/remote/httputility.cpp @@ -70,3 +70,45 @@ Value HttpUtility::GetLastParameter(const Dictionary::Ptr& params, const String& else return arr->Get(arr->GetLength() - 1); } + +void HttpUtility::SendJsonError(HttpResponse& response, const int code, + const String& info, const String& diagnosticInformation) +{ + Dictionary::Ptr result = new Dictionary(); + response.SetStatus(code, HttpUtility::GetErrorNameByCode(code)); + result->Set("error", code); + if (!info.IsEmpty()) + result->Set("status", info); + if (!diagnosticInformation.IsEmpty()) + result->Set("diagnostic information", diagnosticInformation); + HttpUtility::SendJsonBody(response, result); +} + +String HttpUtility::GetErrorNameByCode(const int code) +{ + switch(code) { + case 200: + return "OK"; + case 201: + return "Created"; + case 204: + return "No Content"; + case 304: + return "Not Modified"; + case 400: + return "Bad Request"; + case 401: + return "Unauthorized"; + case 403: + return "Forbidden"; + case 404: + return "Not Found"; + case 409: + return "Conflict"; + case 500: + return "Internal Server Error"; + default: + return "Unknown Error Code"; + } +} + diff --git a/lib/remote/httputility.hpp b/lib/remote/httputility.hpp index 64f84fa9b..f560723a3 100644 --- a/lib/remote/httputility.hpp +++ b/lib/remote/httputility.hpp @@ -39,6 +39,12 @@ public: static Dictionary::Ptr FetchRequestParameters(HttpRequest& request); static void SendJsonBody(HttpResponse& response, const Value& val); static Value GetLastParameter(const Dictionary::Ptr& params, const String& key); + static void SendJsonError(HttpResponse& response, const int code, + const String& verbose="", const String& diagnosticInformation=""); + +private: + static String GetErrorNameByCode(int code); + }; } diff --git a/lib/remote/jsonrpc.cpp b/lib/remote/jsonrpc.cpp index 9447470d9..008b97939 100644 --- a/lib/remote/jsonrpc.cpp +++ b/lib/remote/jsonrpc.cpp @@ -20,7 +20,6 @@ #include "remote/jsonrpc.hpp" #include "base/netstring.hpp" #include "base/json.hpp" -//#include using namespace icinga; @@ -32,7 +31,6 @@ using namespace icinga; void JsonRpc::SendMessage(const Stream::Ptr& stream, const Dictionary::Ptr& message) { String json = JsonEncode(message); - //std::cerr << ">> " << json << std::endl; NetString::WriteStringToStream(stream, json); } @@ -44,7 +42,6 @@ StreamReadStatus JsonRpc::ReadMessage(const Stream::Ptr& stream, Dictionary::Ptr if (srs != StatusNewItem) return srs; - //std::cerr << "<< " << jsonString << std::endl; Value value = JsonDecode(jsonString); if (!value.IsObjectType()) { @@ -56,3 +53,4 @@ StreamReadStatus JsonRpc::ReadMessage(const Stream::Ptr& stream, Dictionary::Ptr return StatusNewItem; } + diff --git a/lib/remote/jsonrpcconnection.cpp b/lib/remote/jsonrpcconnection.cpp index 7a0be8a5d..128dfdc42 100644 --- a/lib/remote/jsonrpcconnection.cpp +++ b/lib/remote/jsonrpcconnection.cpp @@ -38,8 +38,10 @@ REGISTER_APIFUNCTION(RequestCertificate, pki, &RequestCertificateHandler); static boost::once_flag l_JsonRpcConnectionOnceFlag = BOOST_ONCE_INIT; static Timer::Ptr l_JsonRpcConnectionTimeoutTimer; -JsonRpcConnection::JsonRpcConnection(const String& identity, bool authenticated, const TlsStream::Ptr& stream, ConnectionRole role) - : m_Identity(identity), m_Authenticated(authenticated), m_Stream(stream), m_Role(role), m_Seen(Utility::GetTime()), +JsonRpcConnection::JsonRpcConnection(const String& identity, bool authenticated, + const TlsStream::Ptr& stream, ConnectionRole role) + : m_Identity(identity), m_Authenticated(authenticated), m_Stream(stream), + m_Role(role), m_Seen(Utility::GetTime()), m_NextHeartbeat(0), m_HeartbeatTimeout(0) { boost::call_once(l_JsonRpcConnectionOnceFlag, &JsonRpcConnection::StaticInitialize); @@ -174,7 +176,7 @@ bool JsonRpcConnection::ProcessMessage(void) resultMessage->Set("result", afunc->Invoke(origin, message->Get("params"))); } catch (const std::exception& ex) { - //TODO: Add a user readable error message for the remote caller + /* TODO: Add a user readable error message for the remote caller */ resultMessage->Set("error", DiagnosticInformation(ex)); std::ostringstream info; info << "Error while processing message for identity '" << m_Identity << "'"; @@ -200,7 +202,8 @@ void JsonRpcConnection::DataAvailableHandler(void) ; /* empty loop body */ } catch (const std::exception& ex) { Log(LogWarning, "JsonRpcConnection") - << "Error while reading JSON-RPC message for identity '" << m_Identity << "': " << DiagnosticInformation(ex); + << "Error while reading JSON-RPC message for identity '" << m_Identity + << "': " << DiagnosticInformation(ex); Disconnect(); } @@ -286,3 +289,4 @@ void JsonRpcConnection::TimeoutTimerHandler(void) } } } + diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index 5786e4192..9fbeee05a 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -35,7 +35,8 @@ class TypeTargetProvider : public TargetProvider public: DECLARE_PTR_TYPEDEFS(TypeTargetProvider); - virtual void FindTargets(const String& type, const boost::function& addTarget) const override + virtual void FindTargets(const String& type, + const boost::function& addTarget) const override { std::vector targets; diff --git a/lib/remote/url.cpp b/lib/remote/url.cpp index 6f1e2f2e8..52d41793e 100644 --- a/lib/remote/url.cpp +++ b/lib/remote/url.cpp @@ -243,7 +243,7 @@ String Url::Format(bool print_credentials) const param = "?"; else param += "&"; - + String temp; BOOST_FOREACH (const String s, kv.second) { if (!temp.IsEmpty()) @@ -343,7 +343,7 @@ bool Url::ParsePath(const String& path) bool Url::ParseQuery(const String& query) { - //Tokenizer does not like String AT ALL + /* Tokenizer does not like String AT ALL */ std::string queryStr = query; boost::char_separator sep("&"); boost::tokenizer > tokens(queryStr, sep); @@ -354,7 +354,7 @@ bool Url::ParseQuery(const String& query) if (pHelper == 0) // /?foo=bar&=bar == invalid return false; - + String key = token.SubStr(0, pHelper); String value = Empty; @@ -363,7 +363,7 @@ bool Url::ParseQuery(const String& query) if (!ValidateToken(value, ACQUERY)) return false; - + value = Utility::UnescapeString(value); pHelper = key.Find("[]"); @@ -372,7 +372,7 @@ bool Url::ParseQuery(const String& query) return false; key = key.SubStr(0, pHelper); - + if (!ValidateToken(key, ACQUERY)) return false; @@ -385,7 +385,7 @@ bool Url::ParseQuery(const String& query) m_Query[key].push_back(value); } else m_Query[key].push_back(value); - + } return true; @@ -407,3 +407,4 @@ bool Url::ValidateToken(const String& token, const String& symbols) return true; } + diff --git a/lib/remote/zone.cpp b/lib/remote/zone.cpp index f90a75e77..1a307c91d 100644 --- a/lib/remote/zone.cpp +++ b/lib/remote/zone.cpp @@ -97,3 +97,4 @@ Zone::Ptr Zone::GetLocalZone(void) return local->GetZone(); } +