From bc4b852c36ee6d36e12592c37e4e3bfe497f8c68 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Mon, 25 Aug 2014 10:36:46 +0200
Subject: Replace space: add some checks

This patch adds some additional checks if the option for replacing
spaces in user and group names is used.

When replacing space with the replacement character it is checked if the
name already contains the replacement character. If it does the
unmodified name is returned because in this case a revers operation
would not be possible.

For the reverse operation is it checked if the input contains both a
space and the replacement character. If this is true the unmodified name
is returned as well, because we have to assume that it is the original
name because otherwise it wouldn't contain both characters.

Additionally a shortcut if the replacement characters is a space and
tests for the new checks are added. The man page is updated accordingly.

Related to https://fedorahosted.org/sssd/ticket/1854 and
https://fedorahosted.org/sssd/ticket/2397 .

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
(cherry picked from commit 92d19f76449817dfb125da9510d478a30eed37bc)
---
 src/man/sssd.conf.5.xml              |  8 +++++---
 src/tests/cmocka/test_string_utils.c |  4 ++++
 src/util/string_utils.c              | 26 +++++++++++++++++++++++---
 src/util/util.h                      |  2 +-
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 791c38e46..1f9506c35 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -272,9 +272,11 @@
                             </para>
                             <para>
                                 Please note it is a configuration error to use
-                                a replacement character that might be used by
-                                another LDAP object. In that case, result of
-                                a lookup is undefined.
+                                a replacement character that might be used in
+                                user or group names. If a name contains the
+                                replacement character SSSD tries to return the
+                                unmodified name but in general the result of a
+                                lookup is undefined.
                             </para>
                             <para>
                                 Default: not set (spaces will not be replaced)
diff --git a/src/tests/cmocka/test_string_utils.c b/src/tests/cmocka/test_string_utils.c
index 93d7d716f..e446387d6 100644
--- a/src/tests/cmocka/test_string_utils.c
+++ b/src/tests/cmocka/test_string_utils.c
@@ -47,6 +47,7 @@ void test_replace_whitespaces(void **state)
         { "    ", "    ", ' ' },
         { "abcd", "abcd", ' ' },
         { "a b c d", "a b c d", ' ' },
+        { "a b^c d", "a b^c d", '^' },
         { NULL, NULL, '\0' },
     };
 
@@ -92,6 +93,8 @@ void test_reverse_replace_whitespaces(void **state)
         { "abcd", "abcd", '-' },
         { "a-b-c-d", "a b c d", '-' },
         { "-a-b-c-d-", " a b c d ", '-' },
+        { "a b c d", "a b c d", '-' },
+        { " a b c d ", " a b c d ", '-' },
         { "^", " ", '^' },
         { "^^^^", "    ", '^' },
         { "abcd", "abcd", '^' },
@@ -102,6 +105,7 @@ void test_reverse_replace_whitespaces(void **state)
         { "abcd", "abcd", ' ' },
         { "a b c d", "a b c d", ' ' },
         { " a b c d ", " a b c d ", ' ' },
+        { "a b^c d", "a b^c d", '^' },
         { NULL, NULL, '\0' },
     };
 
diff --git a/src/util/string_utils.c b/src/util/string_utils.c
index ec4cc687e..a39b950e8 100644
--- a/src/util/string_utils.c
+++ b/src/util/string_utils.c
@@ -48,18 +48,38 @@ char * sss_replace_space(TALLOC_CTX *mem_ctx,
                          const char *orig_name,
                          const char subst)
 {
-    if (subst == '\0') {
+    if (subst == '\0' || subst == ' ') {
         return talloc_strdup(mem_ctx, orig_name);
     }
+
+    if (strchr(orig_name, subst) != NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              "Input [%s] already contains replacement character [%c].\n",
+              orig_name, subst);
+        sss_log(SSS_LOG_CRIT,
+                "Name [%s] already contains replacement character [%c]. " \
+                "No replacement will be done.\n",
+                orig_name, subst);
+        return talloc_strdup(mem_ctx, orig_name);
+    }
+
     return replace_char(mem_ctx, orig_name, ' ', subst);
 }
 
 char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
-                                 char *orig_name,
+                                 const char *orig_name,
                                  const char subst)
 {
-    if (subst == '\0') {
+    if (subst == '\0' || subst == ' ') {
         return talloc_strdup(mem_ctx, orig_name);
     }
+
+    if (strchr(orig_name, subst) != NULL && strchr(orig_name, ' ') != NULL) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "Input [%s] contains replacement character [%c] and space.\n",
+              orig_name, subst);
+        return talloc_strdup(mem_ctx, orig_name);
+    }
+
     return replace_char(mem_ctx, orig_name, subst, ' ');
 }
diff --git a/src/util/util.h b/src/util/util.h
index 66b3e87db..7a668465e 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -540,7 +540,7 @@ char * sss_replace_space(TALLOC_CTX *mem_ctx,
                          const char *orig_name,
                          const char replace_char);
 char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
-                                 char *orig_name,
+                                 const char *orig_name,
                                  const char replace_char);
 
 #endif /* __SSSD_UTIL_H__ */
-- 
cgit