The bug that was reported showed that if the Resources invoke method was
executed with an empty argument that should be required, it could start
up a recurring action that would fail every time and could never be
cancelled.
Address this by adding some sanity checking on the arguments in the
core services API. If a parameter is determined to be invalid, don't
attempt the action at all and return an error back to the agent code.
In the QMF agent, if this happens, raise an exception that indicates
that the arguments were invalid.
Signed-off-by: Russell Bryant <rbryant(a)redhat.com>
---
src/include/matahari/agent.h | 1 +
src/include/matahari/services.h | 17 ++++++++++++++++-
src/lib/services.c | 34 ++++++++++++++++++++++++++++++++++
src/service/service-qmf.cpp | 19 +++++++++++++++++--
4 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/src/include/matahari/agent.h b/src/include/matahari/agent.h
index 84e9aaf..8345841 100644
--- a/src/include/matahari/agent.h
+++ b/src/include/matahari/agent.h
@@ -45,6 +45,7 @@ int required_argument = 1;
}
#define MH_NOT_IMPLEMENTED "Not implemented"
+#define MH_INVALID_ARGS "Invalid Arguments"
#ifndef NS_MAXDNAME
# define NS_MAXDNAME 1025
diff --git a/src/include/matahari/services.h b/src/include/matahari/services.h
index 86341b2..54d826b 100644
--- a/src/include/matahari/services.h
+++ b/src/include/matahari/services.h
@@ -218,4 +218,19 @@ services_get_ocf_exitcode(char *action, int lsb_exitcode)
* for rc <= 7 */
return (enum ocf_exitcode)lsb_exitcode;
}
-#endif
+
+/**
+ * Check if a string is empty.
+ *
+ * \param[in] s the string to check
+ *
+ * \retval 0 not empty
+ * \retval non-zero empty
+ */
+static inline int
+mh_strlen_zero(const char *s)
+{
+ return !s || *s == '\0';
+}
+
+#endif /* __MH_SERVICES__ */
diff --git a/src/lib/services.c b/src/lib/services.c
index 5768a5e..a62fff2 100644
--- a/src/lib/services.c
+++ b/src/lib/services.c
@@ -49,6 +49,40 @@ svc_action_t *resources_action_create(
{
svc_action_t *op;
+ /*
+ * Do some up front sanity checks before we go off and
+ * build the svc_action_t instance.
+ */
+
+ if (mh_strlen_zero(name)) {
+ mh_err("A service or resource action must have a name.");
+ return NULL;
+ }
+
+ if (mh_strlen_zero(standard)) {
+ mh_err("A service action must have a valid standard.");
+ return NULL;
+ }
+
+ if (!strcasecmp(standard, "ocf") && mh_strlen_zero(provider)) {
+ mh_err("An OCF resource action must have a provider.");
+ return NULL;
+ }
+
+ if (mh_strlen_zero(agent)) {
+ mh_err("A service or resource action must have an agent.");
+ return NULL;
+ }
+
+ if (mh_strlen_zero(action)) {
+ mh_err("A service or resource action must specify an action.");
+ return NULL;
+ }
+
+ /*
+ * Sanity checks passed, proceed!
+ */
+
op = calloc(1, sizeof(svc_action_t));
op->opaque = calloc(1, sizeof(svc_action_private_t));
op->rsc = strdup(name);
diff --git a/src/service/service-qmf.cpp b/src/service/service-qmf.cpp
index a2df2e3..2ad2a75 100644
--- a/src/service/service-qmf.cpp
+++ b/src/service/service-qmf.cpp
@@ -305,7 +305,12 @@ SrvAgent::invoke_services(qmf::AgentSession session, qmf::AgentEvent
event,
args["name"].asString().c_str(), methodName.c_str(), 0,
default_timeout_ms);
- action_async(SRV_SERVICES, session, event, op, true);
+ if (!op) {
+ session.raiseException(event, MH_INVALID_ARGS);
+ } else {
+ action_async(SRV_SERVICES, session, event, op, true);
+ }
+
return TRUE;
} else if (methodName == "start"
@@ -314,7 +319,12 @@ SrvAgent::invoke_services(qmf::AgentSession session, qmf::AgentEvent
event,
svc_action_t *op = services_action_create(
args["name"].asString().c_str(), methodName.c_str(), 0,
args["timeout"].asInt32());
- action_async(SRV_SERVICES, session, event, op, true);
+ if (!op) {
+ session.raiseException(event, MH_INVALID_ARGS);
+ } else {
+ action_async(SRV_SERVICES, session, event, op, true);
+ }
+
return TRUE;
} else {
@@ -432,6 +442,11 @@ SrvAgent::invoke_resources(qmf::AgentSession session, qmf::AgentEvent
event,
args["action"].asString().c_str(),
interval, timeout, params);
+ if (!op) {
+ session.raiseException(event, MH_INVALID_ARGS);
+ return TRUE;
+ }
+
if(args.count("expected-rc") == 1) {
op->expected_rc = args["expected-rc"].asInt32();
}
--
1.7.6