Hi,
Patch 0001:
Moves few object path functions from IFP to sbus.
Patch 0002:
Adds a new function that retrieves unescaped object name.
Patch 0003:
Fixes a potential memory leak.
From 4d43661b4fb65e61407f370a44d7ce3d1d922da0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 16 Dec 2014 15:19:05 +0100
Subject: [PATCH 1/3] sbus: move common opath functions from ifp to sbus code
These functions are quite general thus they may be part
of sbus interface.
---
Makefile.am | 13 +++
src/responder/ifp/ifp_components.c | 14 +--
src/responder/ifp/ifp_domains.c | 14 +--
src/responder/ifp/ifp_private.h | 11 ---
src/responder/ifp/ifpsrv_util.c | 181 ----------------------------------
src/sbus/sssd_dbus.h | 20 ++++
src/sbus/sssd_dbus_interface.c | 194 +++++++++++++++++++++++++++++++++++++
src/tests/cmocka/test_ifp.c | 98 -------------------
src/tests/cmocka/test_sbus_opath.c | 157 ++++++++++++++++++++++++++++++
9 files changed, 398 insertions(+), 304 deletions(-)
create mode 100644 src/tests/cmocka/test_sbus_opath.c
diff --git a/Makefile.am b/Makefile.am
index 2f98d1dc641a1c8620188ef0385db92575247723..d5ce6109f06608eac0b632c92683890e3e3877d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -218,6 +218,7 @@ if HAVE_CMOCKA
test_copy_ccache \
test_copy_keytab \
test_child_common \
+ test_sbus_opath \
$(NULL)
if BUILD_IFP
@@ -2180,6 +2181,18 @@ test_child_common_LDADD = \
libsss_debug.la \
libsss_test_common.la \
$(NULL)
+
+test_sbus_opath_SOURCES = \
+ src/tests/cmocka/test_sbus_opath.c \
+ $(NULL)
+test_sbus_opath_CFLAGS = \
+ $(AM_CFLAGS)
+test_sbus_opath_LDADD = \
+ $(CMOCKA_LIBS) \
+ $(SSSD_LIBS) \
+ $(SSSD_INTERNAL_LTLIBS) \
+ libsss_debug.la \
+ libsss_test_common.la
endif # HAVE_CMOCKA
diff --git a/src/responder/ifp/ifp_components.c b/src/responder/ifp/ifp_components.c
index 2b76a480dcb5f2d7203eda1c193d133da9ff103b..5847a767d220c1305f1cbd54a2361da87ca286b0 100644
--- a/src/responder/ifp/ifp_components.c
+++ b/src/responder/ifp/ifp_components.c
@@ -97,11 +97,11 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
type = COMPONENT_MONITOR;
name = "monitor";
} else {
- name = ifp_path_strip_prefix(path, PATH_RESPONDERS "/");
+ name = sbus_opath_strip_prefix(path, PATH_RESPONDERS "/");
if (name != NULL) {
type = COMPONENT_RESPONDER;
} else {
- name = ifp_path_strip_prefix(path, PATH_BACKENDS "/");
+ name = sbus_opath_strip_prefix(path, PATH_BACKENDS "/");
if (name != NULL) {
type = COMPONENT_BACKEND;
} else {
@@ -116,7 +116,7 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
goto done;
}
- safe_name = ifp_bus_path_unescape(mem_ctx, name);
+ safe_name = sbus_opath_unescape_part(mem_ctx, name);
if (safe_name == NULL) {
ret = ENOMEM;
goto done;
@@ -236,7 +236,7 @@ static errno_t list_responders(TALLOC_CTX *mem_ctx,
}
for (i = 0; i < num; i++) {
- list[i] = ifp_reply_objpath(list, PATH_RESPONDERS, svc[i]);
+ list[i] = sbus_opath_compose(list, PATH_RESPONDERS, svc[i]);
if (list[i] == NULL) {
ret = ENOMEM;
goto done;
@@ -286,7 +286,7 @@ static errno_t list_backends(TALLOC_CTX *mem_ctx,
}
for (i = 0; i < num; i++) {
- list[i] = ifp_reply_objpath(list, PATH_BACKENDS, names[i]);
+ list[i] = sbus_opath_compose(list, PATH_BACKENDS, names[i]);
if (list[i] == NULL) {
ret = ENOMEM;
goto done;
@@ -418,7 +418,7 @@ int ifp_find_responder_by_name(struct sbus_request *dbus_req,
const char *result = NULL;
if (responder_exists(arg_name)) {
- result = ifp_reply_objpath(dbus_req, PATH_RESPONDERS, arg_name);
+ result = sbus_opath_compose(dbus_req, PATH_RESPONDERS, arg_name);
if (result == NULL) {
return sbus_request_fail_and_finish(dbus_req, NULL);
}
@@ -448,7 +448,7 @@ int ifp_find_backend_by_name(struct sbus_request *dbus_req,
}
if (backend_exists(ctx->rctx->cdb, arg_name)) {
- result = ifp_reply_objpath(dbus_req, PATH_BACKENDS, arg_name);
+ result = sbus_opath_compose(dbus_req, PATH_BACKENDS, arg_name);
if (result == NULL) {
return sbus_request_fail_and_finish(dbus_req, NULL);
}
diff --git a/src/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c
index 1841cbe5857ce159718c6d5e44b02e3bcd6e4a39..0af81af817037c06111eb67b0256c87c6bb4781a 100644
--- a/src/responder/ifp/ifp_domains.c
+++ b/src/responder/ifp/ifp_domains.c
@@ -125,7 +125,7 @@ static void ifp_list_domains_process(struct tevent_req *req)
for (dom = ireq->ifp_ctx->rctx->domains;
dom != NULL;
dom = get_next_domain(dom, true)) {
- p = ifp_reply_objpath(ireq, INFOPIPE_DOMAIN_PATH_PFX, dom->name);
+ p = sbus_opath_compose(ireq, INFOPIPE_DOMAIN_PATH_PFX, dom->name);
if (p == NULL) {
DEBUG(SSSDBG_MINOR_FAILURE,
"Could not create path for dom %s, skipping\n", dom->name);
@@ -234,7 +234,7 @@ static void ifp_find_domain_by_name_process(struct tevent_req *req)
return;
}
- path = ifp_reply_objpath(ireq, INFOPIPE_DOMAIN_PATH_PFX, iter->name);
+ path = sbus_opath_compose(ireq, INFOPIPE_DOMAIN_PATH_PFX, iter->name);
if (path == NULL) {
DEBUG(SSSDBG_MINOR_FAILURE,
"Could not create path for domain %s, skipping\n", iter->name);
@@ -263,13 +263,13 @@ get_domain_info_from_req(struct sbus_request *dbus_req, void *data)
return NULL;
}
- raw_name = ifp_path_strip_prefix(dbus_req->path,
- INFOPIPE_DOMAIN_PATH_PFX "/");
+ raw_name = sbus_opath_strip_prefix(dbus_req->path,
+ INFOPIPE_DOMAIN_PATH_PFX "/");
if (raw_name == NULL) {
return NULL;
}
- name = ifp_bus_path_unescape(dbus_req, raw_name);
+ name = sbus_opath_unescape_part(dbus_req, raw_name);
if (name == NULL) {
return NULL;
}
@@ -536,6 +536,6 @@ void ifp_dom_get_parent_domain(struct sbus_request *dbus_req,
return;
}
- *_out = ifp_reply_objpath(dbus_req, INFOPIPE_DOMAIN_PATH_PFX,
- dom->parent->name);
+ *_out = sbus_opath_compose(dbus_req, INFOPIPE_DOMAIN_PATH_PFX,
+ dom->parent->name);
}
diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
index fb1639c8dfd00f547a666dce67ee7a94c3143618..676544fadfab136707f01b752c9526d64ee6fa3f 100644
--- a/src/responder/ifp/ifp_private.h
+++ b/src/responder/ifp/ifp_private.h
@@ -68,17 +68,6 @@ errno_t ifp_req_create(struct sbus_request *dbus_req,
/* Returns an appropriate DBus error for specific ifp_req_create failures */
int ifp_req_create_handle_failure(struct sbus_request *dbus_req, errno_t err);
-const char *ifp_path_strip_prefix(const char *path, const char *prefix);
-
-char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path);
-char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path);
-
-char *_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
- const char *part, ...);
-
-#define ifp_reply_objpath(mem_ctx, base, ...) \
- _ifp_reply_objpath(mem_ctx, base, ##__VA_ARGS__, NULL)
-
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el);
const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str);
diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c
index 909bc54870d6442ea9daf8e86d2dc97acfe4b93d..b6a0b168b35f3b3354a837cf9a2c1bc57d41324e 100644
--- a/src/responder/ifp/ifpsrv_util.c
+++ b/src/responder/ifp/ifpsrv_util.c
@@ -97,187 +97,6 @@ int ifp_req_create_handle_failure(struct sbus_request *dbus_req, errno_t err)
"Cannot create IFP request\n"));
}
-const char *ifp_path_strip_prefix(const char *path, const char *prefix)
-{
- if (strncmp(path, prefix, strlen(prefix)) == 0) {
- return path + strlen(prefix);
- }
-
- return NULL;
-}
-
-/* The following path related functions are based on similar code in
- * storaged, just tailored to use talloc instead of glib
- */
-char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path)
-{
- size_t n;
- char *safe_path = NULL;
- TALLOC_CTX *tmp_ctx = NULL;
-
- /* The path must be valid */
- if (path == NULL) {
- return NULL;
- }
-
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
- return NULL;
- }
-
- safe_path = talloc_strdup(tmp_ctx, "");
- if (safe_path == NULL) {
- goto done;
- }
-
- /* Special case for an empty string */
- if (strcmp(path, "") == 0) {
- /* the for loop would just fall through */
- safe_path = talloc_asprintf_append_buffer(safe_path, "_");
- }
-
- for (n = 0; path[n]; n++) {
- int c = path[n];
- /* D-Bus spec says:
- * *
- * * Each element must only contain the ASCII characters
- * "[A-Z][a-z][0-9]_"
- * */
- if ((c >= 'A' && c <= 'Z')
- || (c >= 'a' && c <= 'z')
- || (c >= '0' && c <= '9')) {
- safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
- if (safe_path == NULL) {
- goto done;
- }
- } else {
- safe_path = talloc_asprintf_append_buffer(safe_path, "_%02x", c);
- if (safe_path == NULL) {
- goto done;
- }
- }
- }
-
- safe_path = talloc_steal(mem_ctx, safe_path);
-done:
- talloc_free(tmp_ctx);
- return safe_path;
-}
-
-static inline int unhexchar(char c)
-{
- if (c >= '0' && c <= '9') {
- return c - '0';
- }
-
- if (c >= 'a' && c <= 'f') {
- return c - 'a' + 10;
- }
-
- if (c >= 'A' && c <= 'F') {
- return c - 'A' + 10;
- }
-
- return -1;
-}
-
-char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path)
-{
- char *safe_path;
- const char *p;
- int a, b, c;
- TALLOC_CTX *tmp_ctx = NULL;
-
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
- return NULL;
- }
-
- safe_path = talloc_strdup(tmp_ctx, "");
- if (safe_path == NULL) {
- goto done;
- }
-
- /* Special case for the empty string */
- if (strcmp(path, "_") == 0) {
- safe_path = talloc_steal(mem_ctx, safe_path);
- goto done;
- }
-
- for (p = path; *p; p++) {
- if (*p == '_') {
- /* There must be at least two more chars after underscore */
- if (p[1] == '\0' || p[2] == '\0') {
- safe_path = NULL;
- goto done;
- }
-
- if ((a = unhexchar(p[1])) < 0
- || (b = unhexchar(p[2])) < 0) {
- /* Invalid escape code, let's take it literal then */
- c = '_';
- } else {
- c = ((a << 4) | b);
- p += 2;
- }
- } else {
- c = *p;
- }
-
- safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
- if (safe_path == NULL) {
- goto done;
- }
- }
-
- safe_path = talloc_steal(mem_ctx, safe_path);
-done:
- talloc_free(tmp_ctx);
- return safe_path;
-}
-
-char *
-_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
- const char *part, ...)
-{
- char *safe_part;
- char *path = NULL;
- va_list va;
-
- if (base == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "Wrong object path base!\n");
- return NULL;
- }
-
- path = talloc_strdup(mem_ctx, base);
- if (path == NULL) return NULL;
-
- va_start(va, part);
- while (part != NULL) {
- safe_part = ifp_bus_path_escape(mem_ctx, part);
- if (safe_part == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "Could not add [%s] to objpath\n", part);
- goto fail;
- }
-
- path = talloc_asprintf_append(path, "/%s", safe_part);
- talloc_free(safe_part);
- if (path == NULL) {
- goto fail;
- }
-
- part = va_arg(va, const char *);
- }
- va_end(va);
-
- return path;
-
-fail:
- va_end(va);
- talloc_free(path);
- return NULL;
-}
-
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el)
{
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index 0d021f8fd601ebdacfeb786036144bb627a6389f..d76fcc28e301f350b5ccddcc0ed079a3dab5740f 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -173,6 +173,26 @@ int sbus_conn_register_iface(struct sbus_connection *conn,
errno_t
sbus_conn_reregister_paths(struct sbus_connection *conn);
+char *
+sbus_opath_escape_part(TALLOC_CTX *mem_ctx,
+ const char *object_path_part);
+
+char *
+sbus_opath_unescape_part(TALLOC_CTX *mem_ctx,
+ const char *object_path_part);
+
+char *
+_sbus_opath_compose(TALLOC_CTX *mem_ctx,
+ const char *base,
+ const char *part, ...);
+
+#define sbus_opath_compose(mem_ctx, base, ...) \
+ _sbus_opath_compose(mem_ctx, base, ##__VA_ARGS__, NULL)
+
+const char *
+sbus_opath_strip_prefix(const char *object_path,
+ const char *prefix);
+
bool sbus_conn_disconnecting(struct sbus_connection *conn);
/* max_retries < 0: retry forever
diff --git a/src/sbus/sssd_dbus_interface.c b/src/sbus/sssd_dbus_interface.c
index 16919c967b2d3e11036a78f9328b358da0503db7..3f509c96e1e59a7bc005497fb5ab63ebc9932e34 100644
--- a/src/sbus/sssd_dbus_interface.c
+++ b/src/sbus/sssd_dbus_interface.c
@@ -171,6 +171,200 @@ static char *sbus_opath_march_up(TALLOC_CTX *mem_ctx,
return subtree;
}
+/**
+ * The following path related functions are based on similar code in
+ * storaged, just tailored to use talloc instead of glib
+ */
+char *
+sbus_opath_escape_part(TALLOC_CTX *mem_ctx,
+ const char *object_path_part)
+{
+ size_t n;
+ char *safe_path = NULL;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ /* The path must be valid */
+ if (object_path_part == NULL) {
+ return NULL;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return NULL;
+ }
+
+ safe_path = talloc_strdup(tmp_ctx, "");
+ if (safe_path == NULL) {
+ goto done;
+ }
+
+ /* Special case for an empty string */
+ if (strcmp(object_path_part, "") == 0) {
+ /* the for loop would just fall through */
+ safe_path = talloc_asprintf_append_buffer(safe_path, "_");
+ if (safe_path == NULL) {
+ goto done;
+ }
+ }
+
+ for (n = 0; object_path_part[n]; n++) {
+ int c = object_path_part[n];
+ /* D-Bus spec says:
+ * *
+ * * Each element must only contain the ASCII characters
+ * "[A-Z][a-z][0-9]_"
+ * */
+ if ((c >= 'A' && c <= 'Z')
+ || (c >= 'a' && c <= 'z')
+ || (c >= '0' && c <= '9')) {
+ safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ } else {
+ safe_path = talloc_asprintf_append_buffer(safe_path, "_%02x", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ }
+ }
+
+ safe_path = talloc_steal(mem_ctx, safe_path);
+
+done:
+ talloc_free(tmp_ctx);
+ return safe_path;
+}
+
+static inline int unhexchar(char c)
+{
+ if (c >= '0' && c <= '9') {
+ return c - '0';
+ }
+
+ if (c >= 'a' && c <= 'f') {
+ return c - 'a' + 10;
+ }
+
+ if (c >= 'A' && c <= 'F') {
+ return c - 'A' + 10;
+ }
+
+ return -1;
+}
+
+char *
+sbus_opath_unescape_part(TALLOC_CTX *mem_ctx,
+ const char *object_path_part)
+{
+ char *safe_path;
+ const char *p;
+ int a, b, c;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return NULL;
+ }
+
+ safe_path = talloc_strdup(tmp_ctx, "");
+ if (safe_path == NULL) {
+ goto done;
+ }
+
+ /* Special case for the empty string */
+ if (strcmp(object_path_part, "_") == 0) {
+ safe_path = talloc_steal(mem_ctx, safe_path);
+ goto done;
+ }
+
+ for (p = object_path_part; *p; p++) {
+ if (*p == '_') {
+ /* There must be at least two more chars after underscore */
+ if (p[1] == '\0' || p[2] == '\0') {
+ safe_path = NULL;
+ goto done;
+ }
+
+ if ((a = unhexchar(p[1])) < 0
+ || (b = unhexchar(p[2])) < 0) {
+ /* Invalid escape code, let's take it literal then */
+ c = '_';
+ } else {
+ c = ((a << 4) | b);
+ p += 2;
+ }
+ } else {
+ c = *p;
+ }
+
+ safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ }
+
+ safe_path = talloc_steal(mem_ctx, safe_path);
+
+done:
+ talloc_free(tmp_ctx);
+ return safe_path;
+}
+
+char *
+_sbus_opath_compose(TALLOC_CTX *mem_ctx,
+ const char *base,
+ const char *part, ...)
+{
+ char *safe_part;
+ char *path = NULL;
+ va_list va;
+
+ if (base == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Wrong object path base!\n");
+ return NULL;
+ }
+
+ path = talloc_strdup(mem_ctx, base);
+ if (path == NULL) return NULL;
+
+ va_start(va, part);
+ while (part != NULL) {
+ safe_part = sbus_opath_escape_part(mem_ctx, part);
+ if (safe_part == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Could not add [%s] to objpath\n", part);
+ goto fail;
+ }
+
+ path = talloc_asprintf_append(path, "/%s", safe_part);
+ talloc_free(safe_part);
+ if (path == NULL) {
+ goto fail;
+ }
+
+ part = va_arg(va, const char *);
+ }
+ va_end(va);
+
+ return path;
+
+fail:
+ va_end(va);
+ talloc_free(path);
+ return NULL;
+}
+
+const char *
+sbus_opath_strip_prefix(const char *object_path,
+ const char *prefix)
+{
+ if (strncmp(object_path, prefix, strlen(prefix)) == 0) {
+ return object_path + strlen(prefix);
+ }
+
+ return NULL;
+}
+
static void
sbus_opath_hash_delete_cb(hash_entry_t *item,
hash_destroy_enum deltype,
diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c
index 5793f91911c1e15c2c241aaa51ffcd4196daea63..484ab85689ec1e0f6cc3882d6170144cbd9a9603 100644
--- a/src/tests/cmocka/test_ifp.c
+++ b/src/tests/cmocka/test_ifp.c
@@ -134,14 +134,6 @@ void ifp_test_req_wrong_uid(void **state)
assert_true(leak_check_teardown());
}
-void test_path_prefix(void **state)
-{
- const char *prefix = "foo";
-
- assert_non_null(ifp_path_strip_prefix("foobar", prefix));
- assert_null(ifp_path_strip_prefix("notfoo", prefix));
-}
-
void test_el_to_dict(void **state)
{
static struct sbus_request *sr;
@@ -362,64 +354,6 @@ void test_attr_allowed(void **state)
assert_false(ifp_attr_allowed(NULL, "name"));
}
-void test_path_escape_unescape(void **state)
-{
- char *escaped;
- char *raw;
- TALLOC_CTX *mem_ctx;
-
- assert_true(leak_check_setup());
- mem_ctx = talloc_new(global_talloc_context);
-
- escaped = ifp_bus_path_escape(mem_ctx, "noescape");
- assert_non_null(escaped);
- assert_string_equal(escaped, "noescape");
- raw = ifp_bus_path_unescape(mem_ctx, escaped);
- talloc_free(escaped);
- assert_non_null(raw);
- assert_string_equal(raw, "noescape");
- talloc_free(raw);
-
- escaped = ifp_bus_path_escape(mem_ctx, "redhat.com");
- assert_non_null(escaped);
- assert_string_equal(escaped, "redhat_2ecom"); /* dot is 0x2E in ASCII */
- raw = ifp_bus_path_unescape(mem_ctx, escaped);
- talloc_free(escaped);
- assert_non_null(raw);
- assert_string_equal(raw, "redhat.com");
- talloc_free(raw);
-
- escaped = ifp_bus_path_escape(mem_ctx, "path_with_underscore");
- assert_non_null(escaped);
- /* underscore is 0x5F in ascii */
- assert_string_equal(escaped, "path_5fwith_5funderscore");
- raw = ifp_bus_path_unescape(mem_ctx, escaped);
- talloc_free(escaped);
- assert_non_null(raw);
- assert_string_equal(raw, "path_with_underscore");
- talloc_free(raw);
-
- /* empty string */
- escaped = ifp_bus_path_escape(mem_ctx, "");
- assert_non_null(escaped);
- assert_string_equal(escaped, "_");
- raw = ifp_bus_path_unescape(mem_ctx, escaped);
- talloc_free(escaped);
- assert_non_null(raw);
- assert_string_equal(raw, "");
- talloc_free(raw);
-
- /* negative tests */
- escaped = ifp_bus_path_escape(mem_ctx, NULL);
- assert_null(escaped);
- raw = ifp_bus_path_unescape(mem_ctx, "wrongpath_");
- assert_null(raw);
-
- assert_true(leak_check_teardown());
-}
-
-#define PATH_BASE "/some/path"
-
struct ifp_test_req_ctx {
struct ifp_req *ireq;
struct sbus_request *sr;
@@ -462,32 +396,6 @@ void ifp_test_req_teardown(void **state)
assert_true(leak_check_teardown());
}
-void test_reply_path(void **state)
-{
- struct ifp_test_req_ctx *test_ctx = talloc_get_type_abort(*state,
- struct ifp_test_req_ctx);
- char *path;
-
- /* Doesn't need escaping */
- path = ifp_reply_objpath(test_ctx->ireq, PATH_BASE, "domname", NULL);
- assert_non_null(path);
- assert_string_equal(path, PATH_BASE"/domname");
- talloc_free(path);
-}
-
-void test_reply_path_escape(void **state)
-{
- struct ifp_test_req_ctx *test_ctx = talloc_get_type_abort(*state,
- struct ifp_test_req_ctx);
- char *path;
-
- /* A dot needs escaping */
- path = ifp_reply_objpath(test_ctx->ireq, PATH_BASE, "redhat.com", NULL);
- assert_non_null(path);
- assert_string_equal(path, PATH_BASE"/redhat_2ecom");
- talloc_free(path);
-}
-
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -501,17 +409,11 @@ int main(int argc, const char *argv[])
const UnitTest tests[] = {
unit_test(ifp_test_req_create),
unit_test(ifp_test_req_wrong_uid),
- unit_test(test_path_prefix),
unit_test_setup_teardown(test_el_to_dict,
ifp_test_req_setup, ifp_test_req_teardown),
unit_test(test_attr_acl),
unit_test(test_attr_acl_ex),
unit_test(test_attr_allowed),
- unit_test(test_path_escape_unescape),
- unit_test_setup_teardown(test_reply_path,
- ifp_test_req_setup, ifp_test_req_teardown),
- unit_test_setup_teardown(test_reply_path_escape,
- ifp_test_req_setup, ifp_test_req_teardown),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
diff --git a/src/tests/cmocka/test_sbus_opath.c b/src/tests/cmocka/test_sbus_opath.c
new file mode 100644
index 0000000000000000000000000000000000000000..b526c88734c626c9c1d4c69d89e05fdff9f32cb1
--- /dev/null
+++ b/src/tests/cmocka/test_sbus_opath.c
@@ -0,0 +1,157 @@
+/*
+ Authors:
+ Jakub Hrozek <jhro...@redhat.com>
+ Pavel BÅezina <pbrez...@redhat.com>
+
+ Copyright (C) 2014 Red Hat
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+#include <errno.h>
+#include <popt.h>
+
+#include "sbus/sssd_dbus.h"
+#include "tests/cmocka/common_mock.h"
+#include "tests/common.h"
+
+void test_sbus_opath_strip_prefix(void **state)
+{
+ const char *prefix = "/org/freedesktop/sssd/";
+ const char *path = "/org/freedesktop/sssd/infopipe";
+ const char *strip;
+
+ strip = sbus_opath_strip_prefix(path, prefix);
+ assert_non_null(prefix);
+ assert_string_equal(strip, "infopipe");
+
+ strip = sbus_opath_strip_prefix("/other/path", prefix);
+ assert_null(strip);
+}
+
+void test_sbus_opath_escape_unescape(void **state)
+{
+ char *escaped;
+ char *raw;
+ TALLOC_CTX *mem_ctx;
+
+ assert_true(leak_check_setup());
+ mem_ctx = talloc_new(global_talloc_context);
+
+ escaped = sbus_opath_escape_part(mem_ctx, "noescape");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "noescape");
+ raw = sbus_opath_unescape_part(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "noescape");
+ talloc_free(raw);
+
+ escaped = sbus_opath_escape_part(mem_ctx, "redhat.com");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "redhat_2ecom"); /* dot is 0x2E in ASCII */
+ raw = sbus_opath_unescape_part(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "redhat.com");
+ talloc_free(raw);
+
+ escaped = sbus_opath_escape_part(mem_ctx, "path_with_underscore");
+ assert_non_null(escaped);
+ /* underscore is 0x5F in ascii */
+ assert_string_equal(escaped, "path_5fwith_5funderscore");
+ raw = sbus_opath_unescape_part(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "path_with_underscore");
+ talloc_free(raw);
+
+ /* empty string */
+ escaped = sbus_opath_escape_part(mem_ctx, "");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "_");
+ raw = sbus_opath_unescape_part(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "");
+ talloc_free(raw);
+
+ /* negative tests */
+ escaped = sbus_opath_escape_part(mem_ctx, NULL);
+ assert_null(escaped);
+ raw = sbus_opath_unescape_part(mem_ctx, "wrongpath_");
+ assert_null(raw);
+
+ assert_true(leak_check_teardown());
+}
+
+void test_sbus_opath_compose(void **state)
+{
+ char *path;
+
+ /* Doesn't need escaping */
+ path = sbus_opath_compose(NULL, "/base/path", "domname");
+ assert_non_null(path);
+ assert_string_equal(path, "/base/path/domname");
+ talloc_free(path);
+}
+
+void test_sbus_opath_compose_escape(void **state)
+{
+ char *path;
+
+ /* A dot needs escaping */
+ path = sbus_opath_compose(NULL, "/base/path", "redhat.com", NULL);
+ assert_non_null(path);
+ assert_string_equal(path, "/base/path/redhat_2ecom");
+ talloc_free(path);
+}
+
+int main(int argc, const char *argv[])
+{
+ poptContext pc;
+ int opt;
+ struct poptOption long_options[] = {
+ POPT_AUTOHELP
+ SSSD_DEBUG_OPTS
+ POPT_TABLEEND
+ };
+
+ const UnitTest tests[] = {
+ unit_test(test_sbus_opath_strip_prefix),
+ unit_test(test_sbus_opath_escape_unescape),
+ unit_test(test_sbus_opath_compose),
+ unit_test(test_sbus_opath_compose_escape),
+ };
+
+ /* Set debug level to invalid value so we can deside if -d 0 was used. */
+ debug_level = SSSDBG_INVALID;
+
+ pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+ while((opt = poptGetNextOpt(pc)) != -1) {
+ switch(opt) {
+ default:
+ fprintf(stderr, "\nInvalid option %s: %s\n\n",
+ poptBadOption(pc, 0), poptStrerror(opt));
+ poptPrintUsage(pc, stderr, 0);
+ return 1;
+ }
+ }
+ poptFreeContext(pc);
+
+ DEBUG_CLI_INIT(debug_level);
+
+ return run_tests(tests);
+}
--
1.7.11.7
From c616b4710dd428746b8625647355fed6cc0e1076 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 16 Dec 2014 15:25:38 +0100
Subject: [PATCH 2/3] sbus: add sbus_opath_get_object_name()
This function assumes that the last component of the object path
is an object name. It will return the part unescaped.
---
src/responder/ifp/ifp_components.c | 25 +++++++++++--------------
src/responder/ifp/ifp_domains.c | 10 ++--------
src/sbus/sssd_dbus.h | 5 +++++
src/sbus/sssd_dbus_interface.c | 20 ++++++++++++++++++++
src/tests/cmocka/test_sbus_opath.c | 33 +++++++++++++++++++++++++++++----
5 files changed, 67 insertions(+), 26 deletions(-)
diff --git a/src/responder/ifp/ifp_components.c b/src/responder/ifp/ifp_components.c
index 5847a767d220c1305f1cbd54a2361da87ca286b0..fbccc7e4be2e11699ffb5ba267db05be7d01e00d 100644
--- a/src/responder/ifp/ifp_components.c
+++ b/src/responder/ifp/ifp_components.c
@@ -85,8 +85,7 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
char **_name)
{
enum component_type type;
- const char *name = NULL;
- char *safe_name = NULL;
+ char *name = NULL;
errno_t ret;
if (confdb == NULL || path == NULL) {
@@ -95,13 +94,17 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
if (strcmp(path, PATH_MONITOR) == 0) {
type = COMPONENT_MONITOR;
- name = "monitor";
+ name = talloc_strdup(mem_ctx, "monitor");
+ if (name == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
} else {
- name = sbus_opath_strip_prefix(path, PATH_RESPONDERS "/");
+ name = sbus_opath_get_object_name(mem_ctx, path, PATH_RESPONDERS);
if (name != NULL) {
type = COMPONENT_RESPONDER;
} else {
- name = sbus_opath_strip_prefix(path, PATH_BACKENDS "/");
+ name = sbus_opath_get_object_name(mem_ctx, path, PATH_BACKENDS);
if (name != NULL) {
type = COMPONENT_BACKEND;
} else {
@@ -116,24 +119,18 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
goto done;
}
- safe_name = sbus_opath_unescape_part(mem_ctx, name);
- if (safe_name == NULL) {
- ret = ENOMEM;
- goto done;
- }
-
switch (type) {
case COMPONENT_MONITOR:
/* noop */
break;
case COMPONENT_RESPONDER:
- if (!responder_exists(safe_name)) {
+ if (!responder_exists(name)) {
ret = ENOENT;
goto done;
}
break;
case COMPONENT_BACKEND:
- if (!backend_exists(confdb, safe_name)) {
+ if (!backend_exists(confdb, name)) {
ret = ENOENT;
goto done;
}
@@ -145,7 +142,7 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
}
if (_name != NULL) {
- *_name = safe_name;
+ *_name = name;
}
ret = EOK;
diff --git a/src/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c
index 0af81af817037c06111eb67b0256c87c6bb4781a..7cddb2ebcb864f0e07cad8688d69ae85942bac24 100644
--- a/src/responder/ifp/ifp_domains.c
+++ b/src/responder/ifp/ifp_domains.c
@@ -254,7 +254,6 @@ get_domain_info_from_req(struct sbus_request *dbus_req, void *data)
struct ifp_ctx *ctx = NULL;
struct sss_domain_info *domains = NULL;
struct sss_domain_info *iter = NULL;
- const char *raw_name = NULL;
char *name = NULL;
ctx = talloc_get_type(data, struct ifp_ctx);
@@ -263,13 +262,8 @@ get_domain_info_from_req(struct sbus_request *dbus_req, void *data)
return NULL;
}
- raw_name = sbus_opath_strip_prefix(dbus_req->path,
- INFOPIPE_DOMAIN_PATH_PFX "/");
- if (raw_name == NULL) {
- return NULL;
- }
-
- name = sbus_opath_unescape_part(dbus_req, raw_name);
+ name = sbus_opath_get_object_name(dbus_req, dbus_req->path,
+ INFOPIPE_DOMAIN_PATH_PFX);
if (name == NULL) {
return NULL;
}
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index d76fcc28e301f350b5ccddcc0ed079a3dab5740f..c6a69e2065777385af6ff5f5ed753cd8a371c200 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -193,6 +193,11 @@ const char *
sbus_opath_strip_prefix(const char *object_path,
const char *prefix);
+char *
+sbus_opath_get_object_name(TALLOC_CTX *mem_ctx,
+ const char *object_path,
+ const char *base_path);
+
bool sbus_conn_disconnecting(struct sbus_connection *conn);
/* max_retries < 0: retry forever
diff --git a/src/sbus/sssd_dbus_interface.c b/src/sbus/sssd_dbus_interface.c
index 3f509c96e1e59a7bc005497fb5ab63ebc9932e34..40ea4310d452726fc49d2a6c604c57e409a082b1 100644
--- a/src/sbus/sssd_dbus_interface.c
+++ b/src/sbus/sssd_dbus_interface.c
@@ -365,6 +365,26 @@ sbus_opath_strip_prefix(const char *object_path,
return NULL;
}
+char *
+sbus_opath_get_object_name(TALLOC_CTX *mem_ctx,
+ const char *object_path,
+ const char *base_path)
+{
+ const char *name;
+
+ name = sbus_opath_strip_prefix(object_path, base_path);
+ if (name == NULL || name[0] == '\0') {
+ return NULL;
+ }
+
+ /* if base_path did not end with / */
+ if (name[0] == '/') {
+ name = name + 1;
+ }
+
+ return sbus_opath_unescape_part(mem_ctx, name);
+}
+
static void
sbus_opath_hash_delete_cb(hash_entry_t *item,
hash_destroy_enum deltype,
diff --git a/src/tests/cmocka/test_sbus_opath.c b/src/tests/cmocka/test_sbus_opath.c
index b526c88734c626c9c1d4c69d89e05fdff9f32cb1..a91a3e2f65506567c98310520ae1b862d8ae7955 100644
--- a/src/tests/cmocka/test_sbus_opath.c
+++ b/src/tests/cmocka/test_sbus_opath.c
@@ -27,6 +27,8 @@
#include "tests/cmocka/common_mock.h"
#include "tests/common.h"
+#define BASE_PATH "/some/path"
+
void test_sbus_opath_strip_prefix(void **state)
{
const char *prefix = "/org/freedesktop/sssd/";
@@ -102,9 +104,9 @@ void test_sbus_opath_compose(void **state)
char *path;
/* Doesn't need escaping */
- path = sbus_opath_compose(NULL, "/base/path", "domname");
+ path = sbus_opath_compose(NULL, BASE_PATH, "domname");
assert_non_null(path);
- assert_string_equal(path, "/base/path/domname");
+ assert_string_equal(path, BASE_PATH "/domname");
talloc_free(path);
}
@@ -113,12 +115,34 @@ void test_sbus_opath_compose_escape(void **state)
char *path;
/* A dot needs escaping */
- path = sbus_opath_compose(NULL, "/base/path", "redhat.com", NULL);
+ path = sbus_opath_compose(NULL, BASE_PATH, "redhat.com", NULL);
assert_non_null(path);
- assert_string_equal(path, "/base/path/redhat_2ecom");
+ assert_string_equal(path, BASE_PATH "/redhat_2ecom");
talloc_free(path);
}
+void test_sbus_opath_get_object_name(void **state)
+{
+ const char *path = BASE_PATH "/redhat_2ecom";
+ char *name;
+
+ name = sbus_opath_get_object_name(NULL, path, BASE_PATH);
+ assert_non_null(name);
+ assert_string_equal(name, "redhat.com");
+ talloc_free(name);
+
+ name = sbus_opath_get_object_name(NULL, path, BASE_PATH "/");
+ assert_non_null(name);
+ assert_string_equal(name, "redhat.com");
+ talloc_free(name);
+
+ name = sbus_opath_get_object_name(NULL, BASE_PATH, BASE_PATH);
+ assert_null(name);
+
+ name = sbus_opath_get_object_name(NULL, "invalid", BASE_PATH);
+ assert_null(name);
+}
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -134,6 +158,7 @@ int main(int argc, const char *argv[])
unit_test(test_sbus_opath_escape_unescape),
unit_test(test_sbus_opath_compose),
unit_test(test_sbus_opath_compose_escape),
+ unit_test(test_sbus_opath_get_object_name)
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
--
1.7.11.7
From 4ac2c5a96d713852782edc4abee65a395b4a8e24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 16 Dec 2014 16:43:28 +0100
Subject: [PATCH 3/3] ifp: fix potential memory leak in
check_and_get_component_from_path()
---
src/responder/ifp/ifp_components.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/responder/ifp/ifp_components.c b/src/responder/ifp/ifp_components.c
index fbccc7e4be2e11699ffb5ba267db05be7d01e00d..fa476c76a50fd4e250a6ef44c978d85fe2a736d4 100644
--- a/src/responder/ifp/ifp_components.c
+++ b/src/responder/ifp/ifp_components.c
@@ -148,6 +148,10 @@ static errno_t check_and_get_component_from_path(TALLOC_CTX *mem_ctx,
ret = EOK;
done:
+ if (ret != EOK) {
+ talloc_free(name);
+ }
+
return ret;
}
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
