On 01/14/2015 04:24 PM, Jakub Hrozek wrote:
On Wed, Dec 17, 2014 at 10:32:55AM +0100, Pavel Březina wrote:
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.
I'm sorry about the delay in review -- I was about to proceed with
reviewing these patches, but they don't apply anymore, can you rebase?
Rebased patches attached. This is the only patch set from the sbus
patches that required rebase. It depends on the first set "sbus: support
multiple interfaces on single object path"
>From c5c5986fd65a52a6b8baa5dc236c60b40c5c164e 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/5] 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 28d8e0dc04ca3c50b1d53a60892566c228c14c7c..52227483b33d8389419763394ed2ee2204c02b83 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -219,6 +219,7 @@ if HAVE_CMOCKA
test_copy_keytab \
test_child_common \
responder_cache_req-tests \
+ test_sbus_opath \
$(NULL)
if BUILD_IFP
@@ -2203,6 +2204,18 @@ responder_cache_req_tests_LDADD = \
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
noinst_PROGRAMS = pam_test_client
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.9.3
>From 5cff5263f39556da41da26d9448ce25ce96d8d03 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/5] 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.9.3
>From 48f5bbf09e0e654da524b83e501d60b1e2236cf3 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/5] 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.9.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
