On 10/19/2015 04:21 PM, Lukas Slebodnik wrote:
On (19/10/15 16:03), Michal Židek wrote:
Hi,
attached is patch to fix ticket:
https://fedorahosted.org/sssd/ticket/2785
And one additional patch to add DEBUG message.
Michal
From 3227027c3680b4503477135608969ca904e491c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Mon, 19 Oct 2015 15:38:08 +0200
Subject: [PATCH 1/2] util: Continue if setlocale fails
Fixes:
https://fedorahosted.org/sssd/ticket/2785
setlocale needs some environment variables
to be set in order to work. These variables
are not present in some special cases. We
should not fail completely in these cases
but continue with the compatible C locale.
---
src/sss_client/ssh/sss_ssh_client.c | 4 +++-
src/tools/tools_util.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_client.c
b/src/sss_client/ssh/sss_ssh_client.c
index 0d206ef..ea7a1cc 100644
--- a/src/sss_client/ssh/sss_ssh_client.c
+++ b/src/sss_client/ssh/sss_ssh_client.c
@@ -50,7 +50,9 @@ int set_locale(void)
c = setlocale(LC_ALL, "");
if (c == NULL) {
- return EIO;
+ /* If setlocale fails, continue with the compatible
+ * C locale. */
I'm not sure wehter "compatible" is the best explanation.
I think that "continue with the default" would suit better.
BTW: man setlocale(3) says:
On startup of the main program, the portable "C" locale is selected as
default. A program may be made portable to all locales by calling:
setlocale(LC_ALL, "");
Would you also mind to write integration test?
I can test on command line with:
[root@host ~]# LC_ALL="adasd" sss_cache -E
Error setting the locale
[root@host ~]# echo $?
5
LS
Thank you Lukas!
I added the integration test for this. I added
new file for tests that require LOCAL domain
only (no LDAP). I plan to add more tests here
later.
As we already discussed offline. I temporarily
worked around the issue with failing memcache
test by not removing the memcache files.
Nick asked me to provide some explanation on the
issue so I put it here, please correct me if I
say something wrong.
The problem seems to be that the nss client code only
checks validity of the memcache when the
memcache context is created. Long living
application only check the validity once and
then use file descriptor to manipulate the
cache until they are finished. Pytest is
one such application. If we use the python
pwd and grp wrappers, we initialize memcache
context in Pytest. If we later remove
the memcache files as part of teardown
and create new memcache files for new
tests, Pytest still uses the old file descriptors
so calls to pwd and grp wrappers will
work with the deleted memcache files.
See the attached patches.
Thanks!
Michal
>From 63bc99593d2c6f1d5c40d66c578b6e867cfb45ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Mon, 19 Oct 2015 15:38:08 +0200
Subject: [PATCH 1/5] util: Continue if setlocale fails
Fixes:
https://fedorahosted.org/sssd/ticket/2785
setlocale needs some environment variables
to be set in order to work. These variables
are not present in some special cases. We
should not fail completely in these cases
but continue with the compatible C locale.
---
src/sss_client/ssh/sss_ssh_client.c | 4 +++-
src/tools/tools_util.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_client.c b/src/sss_client/ssh/sss_ssh_client.c
index 0d206ef..a198039 100644
--- a/src/sss_client/ssh/sss_ssh_client.c
+++ b/src/sss_client/ssh/sss_ssh_client.c
@@ -50,7 +50,9 @@ int set_locale(void)
c = setlocale(LC_ALL, "");
if (c == NULL) {
- return EIO;
+ /* If setlocale fails, continue with the default
+ * locale. */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
}
errno = 0;
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 68f6588..f9dca72 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -259,7 +259,9 @@ int set_locale(void)
c = setlocale(LC_ALL, "");
if (c == NULL) {
- return EIO;
+ /* If setlocale fails, continue with the default
+ * locale. */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
}
errno = 0;
--
2.1.0
>From 16aae8eba2fbbce128a26cefc7e2a04763d12907 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Mon, 19 Oct 2015 15:49:02 +0200
Subject: [PATCH 2/5] server_setup: Log failed attempt to set locale
Failed setlocale call could cause unexpected
behaviour. It is better to generate DEBUG
message if this happens.
---
src/util/server.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/util/server.c b/src/util/server.c
index 036dace..03796be 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -458,6 +458,7 @@ int server_setup(const char *name, int flags,
bool dm;
struct tevent_signal *tes;
struct logrotate_ctx *lctx;
+ char *locale;
ret = chown_debug_file(NULL, uid, gid);
if (ret != EOK) {
@@ -508,7 +509,12 @@ int server_setup(const char *name, int flags,
}
/* Set up locale */
- setlocale(LC_ALL, "");
+ locale = setlocale(LC_ALL, "");
+ if (locale == NULL) {
+ /* Just print debug message and continue */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
+ }
+
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
--
2.1.0
>From 58b7bc6fcc8ad4cb3723ba2fd3dcf1882a3ca4ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Tue, 20 Oct 2015 18:18:01 +0200
Subject: [PATCH 3/5] tests: Run intgcheck without libsemanage
For now the libsemanage can not be used inside
intgcheck tests.
---
Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am
index 15d99ce..77a325a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2647,6 +2647,7 @@ intgcheck:
--prefix="$$prefix" \
--with-ldb-lib-dir="$$prefix"/lib/ldb \
--enable-intgcheck-reqs \
+ --without-semanage \
$(INTGCHECK_CONFIGURE_FLAGS); \
$(MAKE) $(AM_MAKEFLAGS); \
: Force single-thread install to workaround concurrency issues; \
--
2.1.0
>From 01a00d79eee83c0c8ced562af8e347e91397f401 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Tue, 20 Oct 2015 15:03:22 +0200
Subject: [PATCH 4/5] tests: Regression test with wrong LC_ALL
Ticket:
https://fedorahosted.org/sssd/ticket/2785
Test local domain tool with wrong LC_ALL
environment variable value.
NOTE: The memory cache files are not deleted
properly in the test teardown to work around the
problem described in ticket
https://fedorahosted.org/sssd/ticket/2726
Once the ticket above is solved, the teardown
will be updated to remove the memory cache
files.
---
src/tests/intg/test_local_domain.py | 116 ++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
create mode 100644 src/tests/intg/test_local_domain.py
diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
new file mode 100644
index 0000000..56133c0
--- /dev/null
+++ b/src/tests/intg/test_local_domain.py
@@ -0,0 +1,116 @@
+#
+# SSSD LOCAL domain tests
+#
+# Copyright (c) 2015 Red Hat, Inc.
+# Author: Michal Zidek <mzi...@redhat.com>
+#
+# This 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; version 2 only
+#
+# 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/>.
+#
+import os
+import stat
+import ent
+import grp
+import pwd
+import time
+import config
+import signal
+import subprocess
+import pytest
+import sssd_id
+from util import unindent
+
+LDAP_BASE_DN = "dc=example,dc=com"
+
+
+def stop_sssd():
+ pid_file = open(config.PIDFILE_PATH, "r")
+ pid = int(pid_file.read())
+ os.kill(pid, signal.SIGTERM)
+ while True:
+ try:
+ os.kill(pid, signal.SIGCONT)
+ except:
+ break
+ time.sleep(1)
+
+
+def create_conf_fixture(request, contents):
+ """Generate sssd.conf and add teardown for removing it"""
+ conf = open(config.CONF_PATH, "w")
+ conf.write(contents)
+ conf.close()
+ os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+ request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
+
+
+def create_sssd_fixture(request):
+ """Start sssd and add teardown for stopping it and removing state"""
+ if subprocess.call(["sssd", "-D", "-f"]) != 0:
+ raise Exception("sssd start failed")
+
+ def teardown():
+ try:
+ stop_sssd()
+ except:
+ pass
+ subprocess.call(["sss_cache", "-E"])
+ for path in os.listdir(config.DB_PATH):
+ os.unlink(config.DB_PATH + "/" + path)
+ # FIXME: Uncomment this when ticket #2726 is solved
+ # https://fedorahosted.org/sssd/ticket/2726
+ #for path in os.listdir(config.MCACHE_PATH):
+ # os.unlink(config.MCACHE_PATH + "/" + path)
+ request.addfinalizer(teardown)
+
+@pytest.fixture
+def local_domain_only(request):
+ conf = unindent("""\
+ [sssd]
+ domains = LOCAL
+ services = nss
+
+ [nss]
+ memcache_timeout = 0
+
+ [domain/LOCAL]
+ id_provider = local
+ min_id = 10000
+ max_id = 20000
+ """).format(**locals())
+ create_conf_fixture(request, conf)
+ create_sssd_fixture(request)
+ return None
+
+
+def assert_nonexistent_user(name):
+ with pytest.raises(KeyError):
+ pwd.getpwnam(name)
+
+
+def test_wrong_LC_ALL(local_domain_only):
+ """
+ Regression test for ticket
+ https://fedorahosted.org/sssd/ticket/2785
+
+ """
+ subprocess.call(["sss_useradd", "foo", "-M"])
+ pwd.getpwnam("foo")
+
+ # Change the LC_ALL variable to nonexistent locale
+ oldvalue = os.environ.get("LC_ALL", "")
+ os.environ["LC_ALL"] = "nonexistent_locale"
+
+ # sss_userdel must remove the user despite wrong LC_ALL
+ ret = subprocess.call(["sss_userdel", "foo", "-R"])
+ assert_nonexistent_user("foo")
+ os.environ["LC_LOCAL"] = oldvalue
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
