[Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context
Sumit Bose
sbose at redhat.com
Wed Mar 4 17:35:22 UTC 2015
Hi,
this patch series improves error reporting of the extdom plugin
especially on the client side. Currently there is only SSSD ticket
https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
corresponding FreeIPA ticket as well?
In the third patch I already added a handful of new error messages.
Suggestions for more messages are welcome.
bye,
Sumit
-------------- next part --------------
From 2e8e4abb7e79d44f0ce0560daeb7696d9641a684 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 2 Feb 2015 00:52:10 +0100
Subject: [PATCH 137/139] extdom: add err_msg member to request context
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h | 1 +
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 1 +
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 5 ++++-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index d4c851169ddadc869a59c53075f9fc7f33321085..421f6c6ea625aba2db7e9ffc84115b3647673699 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -116,6 +116,7 @@ struct extdom_req {
gid_t gid;
} posix_gid;
} data;
+ char *err_msg;
};
struct extdom_res {
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 448710993f551298d3a4cdcc19371b8432773478..27c1313cb1f6f614b0c74992d4a768c3051a86ae 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -360,6 +360,7 @@ void free_req_data(struct extdom_req *req)
break;
}
+ free(req->err_msg);
free(req);
}
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index e53f968db040a37fbd6a193f87b3671eeabda89d..a70ed20f1816a7e00385edae8a81dd5dad9e9362 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -145,11 +145,14 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
rc = LDAP_SUCCESS;
done:
- free_req_data(req);
+ if (req->err_msg != NULL) {
+ err_msg = req->err_msg;
+ }
if (err_msg != NULL) {
LOG("%s", err_msg);
}
slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+ free_req_data(req);
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
--
2.1.0
-------------- next part --------------
From 80406c884eddeb2ebf35bd12a7ed1a8ddd4af2fe Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 2 Feb 2015 00:53:06 +0100
Subject: [PATCH 138/139] extdom: add add_err_msg() with test
---
.../ipa-extdom-extop/ipa_extdom.h | 1 +
.../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 43 ++++++++++++++++++++++
.../ipa-extdom-extop/ipa_extdom_common.c | 23 ++++++++++++
3 files changed, 67 insertions(+)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 421f6c6ea625aba2db7e9ffc84115b3647673699..0d5d55d2fb0ece95466b0225b145a4edeef18efa 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -185,4 +185,5 @@ int getgrnam_r_wrapper(size_t buf_max, const char *name,
struct group *grp, char **_buf, size_t *_buf_len);
int getgrgid_r_wrapper(size_t buf_max, gid_t gid,
struct group *grp, char **_buf, size_t *_buf_len);
+void set_err_msg(struct extdom_req *req, const char *format, ...);
#endif /* _IPA_EXTDOM_H_ */
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index be736dd9c5af4d0b632f1dbc55033fdf738bad46..0ca67030bf667ecd559443842cda166354ce54ce 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -213,6 +213,47 @@ void test_getgrgid_r_wrapper(void **state)
free(buf);
}
+void extdom_req_setup(void **state)
+{
+ struct extdom_req *req;
+
+ req = calloc(sizeof(struct extdom_req), 1);
+ assert_non_null(req);
+
+ *state = req;
+}
+
+void extdom_req_teardown(void **state)
+{
+ struct extdom_req *req;
+
+ req = (struct extdom_req *) *state;
+
+ free_req_data(req);
+}
+
+void test_set_err_msg(void **state)
+{
+ struct extdom_req *req;
+
+ req = (struct extdom_req *) *state;
+ assert_null(req->err_msg);
+
+ set_err_msg(NULL, NULL);
+ assert_null(req->err_msg);
+
+ set_err_msg(req, NULL);
+ assert_null(req->err_msg);
+
+ set_err_msg(req, "Test [%s][%d].", "ABCD", 1234);
+ assert_non_null(req->err_msg);
+ assert_string_equal(req->err_msg, "Test [ABCD][1234].");
+
+ set_err_msg(req, "2nd Test [%s][%d].", "ABCD", 1234);
+ assert_non_null(req->err_msg);
+ assert_string_equal(req->err_msg, "Test [ABCD][1234].");
+}
+
int main(int argc, const char *argv[])
{
const UnitTest tests[] = {
@@ -220,6 +261,8 @@ int main(int argc, const char *argv[])
unit_test(test_getpwuid_r_wrapper),
unit_test(test_getgrnam_r_wrapper),
unit_test(test_getgrgid_r_wrapper),
+ unit_test_setup_teardown(test_set_err_msg,
+ extdom_req_setup, extdom_req_teardown),
};
return run_tests(tests);
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 27c1313cb1f6f614b0c74992d4a768c3051a86ae..ce3884805bd3f8276d29d066c2e54897561d0f99 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -233,6 +233,29 @@ done:
return ret;
}
+void set_err_msg(struct extdom_req *req, const char *format, ...)
+{
+ int ret;
+ va_list ap;
+
+ if (req == NULL) {
+ return;
+ }
+
+ if (format == NULL || req->err_msg != NULL) {
+ /* Do not override an existing error message. */
+ return;
+ }
+ va_start(ap, format);
+
+ ret = vasprintf(&req->err_msg, format, ap);
+ if (ret == -1) {
+ req->err_msg = strdup("vasprintf failed.\n");
+ }
+
+ va_end(ap);
+}
+
int parse_request_data(struct berval *req_val, struct extdom_req **_req)
{
BerElement *ber = NULL;
--
2.1.0
-------------- next part --------------
From 131d5fe646d149057220abf6e0480059af8bf427 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 4 Mar 2015 13:37:50 +0100
Subject: [PATCH 139/139] extdom: add selected error messages
---
.../ipa-extdom-extop/ipa_extdom_common.c | 51 ++++++++++++++++------
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index ce3884805bd3f8276d29d066c2e54897561d0f99..214d6fb23bf21cddac648c0f6b804858d518dcf0 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -304,26 +304,34 @@ int parse_request_data(struct berval *req_val, struct extdom_req **_req)
* }
*/
+ req = calloc(sizeof(struct extdom_req), 1);
+ if (req == NULL) {
+ /* Since we return req even in the case of an error we make sure is is
+ * always safe to call free_req_data() on the returned data. */
+ *_req = NULL;
+ return LDAP_OPERATIONS_ERROR;
+ }
+
+ *_req = req;
+
if (req_val == NULL || req_val->bv_val == NULL || req_val->bv_len == 0) {
+ set_err_msg(req, "Missing request data");
return LDAP_PROTOCOL_ERROR;
}
ber = ber_init(req_val);
if (ber == NULL) {
+ set_err_msg(req, "Cannot initialize BER struct");
return LDAP_PROTOCOL_ERROR;
}
tag = ber_scanf(ber, "{ee", &input_type, &request_type);
if (tag == LBER_ERROR) {
ber_free(ber, 1);
+ set_err_msg(req, "Cannot read input and request type");
return LDAP_PROTOCOL_ERROR;
}
- req = calloc(sizeof(struct extdom_req), 1);
- if (req == NULL) {
- return LDAP_OPERATIONS_ERROR;
- }
-
req->input_type = input_type;
req->request_type = request_type;
@@ -347,17 +355,15 @@ int parse_request_data(struct berval *req_val, struct extdom_req **_req)
break;
default:
ber_free(ber, 1);
- free(req);
+ set_err_msg(req, "Unknown input type");
return LDAP_PROTOCOL_ERROR;
}
ber_free(ber, 1);
if (tag == LBER_ERROR) {
- free(req);
+ set_err_msg(req, "Failed to decode BER data");
return LDAP_PROTOCOL_ERROR;
}
- *_req = req;
-
return LDAP_SUCCESS;
}
@@ -715,6 +721,7 @@ static int pack_ber_name(const char *domain_name, const char *name,
}
static int handle_uid_request(struct ipa_extdom_ctx *ctx,
+ struct extdom_req *req,
enum request_types request_type, uid_t uid,
const char *domain_name, struct berval **berval)
{
@@ -738,6 +745,7 @@ static int handle_uid_request(struct ipa_extdom_ctx *ctx,
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
+ set_err_msg(req, "Failed to lookup SID by UID");
ret = LDAP_OPERATIONS_ERROR;
}
goto done;
@@ -756,6 +764,7 @@ static int handle_uid_request(struct ipa_extdom_ctx *ctx,
ret = sss_nss_getorigbyname(pwd.pw_name, &kv_list, &id_type);
if (ret != 0 || !(id_type == SSS_ID_TYPE_UID
|| id_type == SSS_ID_TYPE_BOTH)) {
+ set_err_msg(req, "Failed to read original data");
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
@@ -781,6 +790,7 @@ done:
}
static int handle_gid_request(struct ipa_extdom_ctx *ctx,
+ struct extdom_req *req,
enum request_types request_type, gid_t gid,
const char *domain_name, struct berval **berval)
{
@@ -803,6 +813,7 @@ static int handle_gid_request(struct ipa_extdom_ctx *ctx,
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
+ set_err_msg(req, "Failed to lookup SID by GID");
ret = LDAP_OPERATIONS_ERROR;
}
goto done;
@@ -821,6 +832,7 @@ static int handle_gid_request(struct ipa_extdom_ctx *ctx,
ret = sss_nss_getorigbyname(grp.gr_name, &kv_list, &id_type);
if (ret != 0 || !(id_type == SSS_ID_TYPE_GID
|| id_type == SSS_ID_TYPE_BOTH)) {
+ set_err_msg(req, "Failed to read original data");
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
@@ -844,6 +856,7 @@ done:
}
static int handle_sid_request(struct ipa_extdom_ctx *ctx,
+ struct extdom_req *req,
enum request_types request_type, const char *sid,
struct berval **berval)
{
@@ -864,6 +877,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
+ set_err_msg(req, "Failed to lookup name by SID");
ret = LDAP_OPERATIONS_ERROR;
}
goto done;
@@ -871,6 +885,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
sep = strchr(fq_name, SSSD_DOMAIN_SEPARATOR);
if (sep == NULL) {
+ set_err_msg(req, "Failed to split fully qualified name");
ret = LDAP_OPERATIONS_ERROR;
goto done;
}
@@ -878,6 +893,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
object_name = strndup(fq_name, (sep - fq_name));
domain_name = strdup(sep + 1);
if (object_name == NULL || domain_name == NULL) {
+ set_err_msg(req, "Missing name or domain");
ret = LDAP_OPERATIONS_ERROR;
goto done;
}
@@ -906,6 +922,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
ret = sss_nss_getorigbyname(pwd.pw_name, &kv_list, &id_type);
if (ret != 0 || !(id_type == SSS_ID_TYPE_UID
|| id_type == SSS_ID_TYPE_BOTH)) {
+ set_err_msg(req, "Failed ot read original data");
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
@@ -934,6 +951,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
ret = sss_nss_getorigbyname(grp.gr_name, &kv_list, &id_type);
if (ret != 0 || !(id_type == SSS_ID_TYPE_GID
|| id_type == SSS_ID_TYPE_BOTH)) {
+ set_err_msg(req, "Failed to read original data");
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
@@ -964,6 +982,7 @@ done:
}
static int handle_name_request(struct ipa_extdom_ctx *ctx,
+ struct extdom_req *req,
enum request_types request_type,
const char *name, const char *domain_name,
struct berval **berval)
@@ -982,6 +1001,7 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,
domain_name);
if (ret == -1) {
ret = LDAP_OPERATIONS_ERROR;
+ set_err_msg(req, "Failed to create fully qualified name");
fq_name = NULL; /* content is undefined according to
asprintf(3) */
goto done;
@@ -993,6 +1013,7 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
+ set_err_msg(req, "Failed to lookup SID by name");
ret = LDAP_OPERATIONS_ERROR;
}
goto done;
@@ -1012,6 +1033,7 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,
ret = sss_nss_getorigbyname(pwd.pw_name, &kv_list, &id_type);
if (ret != 0 || !(id_type == SSS_ID_TYPE_UID
|| id_type == SSS_ID_TYPE_BOTH)) {
+ set_err_msg(req, "Failed to read original data");
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
@@ -1045,6 +1067,7 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,
if (ret == ENOENT) {
ret = LDAP_NO_SUCH_OBJECT;
} else {
+ set_err_msg(req, "Failed to read original data");
ret = LDAP_OPERATIONS_ERROR;
}
goto done;
@@ -1074,27 +1097,29 @@ int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req,
switch (req->input_type) {
case INP_POSIX_UID:
- ret = handle_uid_request(ctx, req->request_type,
+ ret = handle_uid_request(ctx, req, req->request_type,
req->data.posix_uid.uid,
req->data.posix_uid.domain_name, berval);
break;
case INP_POSIX_GID:
- ret = handle_gid_request(ctx, req->request_type,
+ ret = handle_gid_request(ctx, req, req->request_type,
req->data.posix_gid.gid,
req->data.posix_uid.domain_name, berval);
break;
case INP_SID:
- ret = handle_sid_request(ctx, req->request_type, req->data.sid, berval);
+ ret = handle_sid_request(ctx, req, req->request_type, req->data.sid,
+ berval);
break;
case INP_NAME:
- ret = handle_name_request(ctx, req->request_type,
+ ret = handle_name_request(ctx, req, req->request_type,
req->data.name.object_name,
req->data.name.domain_name, berval);
break;
default:
+ set_err_msg(req, "Unknown input type");
ret = LDAP_PROTOCOL_ERROR;
goto done;
}
--
2.1.0
More information about the Freeipa-devel
mailing list