On Fri, 2011-12-02 at 15:16 -0500, Simo Sorce wrote: > ----- Original Message ----- > > Currently, SSSD only supports using libunistring to manage unicode > > strings. There are some platforms out there (such as RHEL 5) that do > > not > > have libunistring available. With this patch, we add an optional flag > > to > > autoconf to allow SSSD to link against Glib and use its unicode > > functionality. > > Two quick comments. > 1. Can you use a shorter function name than > sss_utf8_case_insensitive_equality ? > I am all for clarity, but when the function name is so long arguments go on > the next line w/o any nesting it is bad :-) > > 2. given result returns just a boolean I would prefer to not use it and just > use ret. > 0 is 'matches' and some other error code means it didn't match, something > like ERANGE could be used. or if you feel strongly about having your own > value define a value like -1 as ENOMATCH earlier in the code. It makes for a > more usable interface if you do not have to care about 2 return variables IMO.
Thanks for the review. Revised patch is attached. I opted to use ENOTUNIQ for the "not matched" response, since it should be impossible to receive that error from any of the underlying functions within the comparison.
From 2a7770be95f089f7f4ab0bf245e2ca0edba40da8 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Fri, 2 Dec 2011 11:59:20 -0500
Subject: [PATCH] Allow using Glib for UTF8 support
---
Makefile.am | 19 ++++--
configure.ac | 14 ++++-
src/conf_macros.m4 | 21 ++++++
src/external/glib.m4 | 11 +++
src/providers/ipa/hbac_evaluator.c | 44 +++---------
src/responder/common/responder_common.c | 10 +---
src/util/sss_utf8.c | 111 +++++++++++++++++++++++++++++++
src/util/sss_utf8.h | 39 +++++++++++
8 files changed, 219 insertions(+), 50 deletions(-)
create mode 100644 src/external/glib.m4
create mode 100644 src/util/sss_utf8.c
create mode 100644 src/util/sss_utf8.h
diff --git a/Makefile.am b/Makefile.am
index a89b32302b6b0bc57f78c981b6bf7869e02e2dbe..8d75b8767bd30532f9a241ca1518545d602beffb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -40,6 +40,8 @@ pubconfpath = @pubconfpath@
pkgconfigdir = $(libdir)/pkgconfig
krb5rcachedir = @krb5rcachedir@
+UNICODE_LIBS=@UNICODE_LIBS@
+
AM_CFLAGS =
if WANT_AUX_INFO
AM_CFLAGS += -aux-info $@.X
@@ -197,6 +199,7 @@ AM_CPPFLAGS = \
$(DHASH_CFLAGS) \
$(LIBNL_CFLAGS) \
$(OPENLDAP_CFLAGS) \
+ $(GLIB2_CFLAGS) \
-DLIBDIR=\"$(libdir)\" \
-DVARDIR=\"$(localstatedir)\" \
-DSHLIBEXT=\"$(SHLIBEXT)\" \
@@ -293,6 +296,7 @@ dist_noinst_HEADERS = \
src/util/sss_ldap.h \
src/util/sss_python.h \
src/util/sss_krb5.h \
+ src/util/sss_utf8.h \
src/util/refcount.h \
src/util/find_uid.h \
src/util/user_info_msg.h \
@@ -377,19 +381,22 @@ libsss_util_la_SOURCES = \
src/util/backup_file.c \
src/util/strtonum.c \
src/util/check_and_open.c \
- src/util/refcount.c
+ src/util/refcount.c \
+ src/util/sss_utf8.c
libsss_util_la_LIBADD = \
$(SSSD_LIBS) \
+ $(UNICODE_LIBS) \
libsss_crypt.la \
libsss_debug.la
lib_LTLIBRARIES = libipa_hbac.la
dist_pkgconfig_DATA += src/providers/ipa/ipa_hbac.pc
libipa_hbac_la_SOURCES = \
- src/providers/ipa/hbac_evaluator.c
+ src/providers/ipa/hbac_evaluator.c \
+ src/util/sss_utf8.c
libipa_hbac_la_LDFLAGS = \
-version 1:0:1 \
- -lunistring
+ $(UNICODE_LIBS)
include_HEADERS = \
src/providers/ipa/ipa_hbac.h
@@ -415,8 +422,7 @@ sssd_nss_SOURCES = \
sssd_nss_LDADD = \
$(TDB_LIBS) \
$(SSSD_LIBS) \
- libsss_util.la \
- -lunistring
+ libsss_util.la
sssd_pam_SOURCES = \
src/responder/pam/pam_LOCAL_domain.c \
@@ -427,8 +433,7 @@ sssd_pam_SOURCES = \
sssd_pam_LDADD = \
$(TDB_LIBS) \
$(SSSD_LIBS) \
- libsss_util.la \
- -lunistring
+ libsss_util.la
sssd_be_SOURCES = \
src/providers/data_provider_be.c \
diff --git a/configure.ac b/configure.ac
index 21484ca8d4c6c466bf79d17ac9daa53efabf195c..1fa6c583f4c2038e70fca8954e9ee6bdc3c92c59 100644
--- a/configure.ac
+++ b/configure.ac
@@ -115,7 +115,19 @@ m4_include([src/external/libkeyutils.m4])
m4_include([src/external/libnl.m4])
m4_include([src/external/systemd.m4])
m4_include([src/util/signal.m4])
-m4_include([src/external/libunistring.m4])
+
+WITH_UNICODE_LIB
+if test x$unicode_lib = xlibunistring; then
+ m4_include([src/external/libunistring.m4])
+ AC_DEFINE_UNQUOTED(HAVE_LIBUNISTRING, 1, [Using libunistring for unicode])
+ UNICODE_LIBS=-lunistring
+ AC_SUBST(UNICODE_LIBS)
+else
+ m4_include([src/external/glib.m4])
+ AC_DEFINE_UNQUOTED(HAVE_GLIB2, 1, [Using libunistring for unicode])
+ UNICODE_LIBS=$GLIB2_LIBS
+ AC_SUBST(UNICODE_LIBS)
+fi
WITH_INITSCRIPT
if test x$initscript = xsystemd; then
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index bd661ba3c691ed63a523833c134c9af1e5d6c225..45d54841d5a971df0b62fff33121c9d184f5a908 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -352,3 +352,24 @@ AC_ARG_ENABLE([all-experimental-features],
[build all experimental features])],
[build_all_experimental_features=$enableval],
[build_all_experimental_features=no])
+
+
+AC_DEFUN([WITH_UNICODE_LIB],
+ [ AC_ARG_WITH([unicode-lib],
+ [AC_HELP_STRING([--with-unicode-lib=<library>],
+ [Which library to use for unicode processing (libunistring, glib2) [libunistring]]
+ )
+ ]
+ )
+ unicode_lib="libunistring"
+ if test x"$with_unicode_lib" != x; then
+ unicode_lib=$with_unicode_lib
+ fi
+
+ if test x"$unicode_lib" != x"libunistring" -a x"$unicode_lib" != x"glib2"; then
+ AC_MSG_ERROR([Unsupported unicode library])
+ fi
+
+ AM_CONDITIONAL([WITH_LIBUNISTRING], test x"$unicode_lib" = x"libunistring")
+ AM_CONDITIONAL([WITH_GLIB], test x"$unicode_lib" = x"glib2")
+ ])
\ No newline at end of file
diff --git a/src/external/glib.m4 b/src/external/glib.m4
new file mode 100644
index 0000000000000000000000000000000000000000..8cec763263cd86d4ff1c45a7a3e2afe12954fe1c
--- /dev/null
+++ b/src/external/glib.m4
@@ -0,0 +1,11 @@
+PKG_CHECK_MODULES([GLIB2],[glib-2.0])
+
+if test x$has_glib2 != xno; then
+ SAFE_LIBS="$LIBS"
+ LIBS="$GLIB2_LIBS"
+
+ AC_CHECK_FUNC([g_utf8_validate],
+ AC_DEFINE([HAVE_G_UTF8_VALIDATE], [1],
+ [Define if g_utf8_validate exists]))
+ LIBS="$SAFE_LIBS"
+fi
\ No newline at end of file
diff --git a/src/providers/ipa/hbac_evaluator.c b/src/providers/ipa/hbac_evaluator.c
index 476ad6482a697a070102a60c1109a46f2f350af8..3fa7ec6ea7b1a590f9f06aac6f60b4766ef84df4 100644
--- a/src/providers/ipa/hbac_evaluator.c
+++ b/src/providers/ipa/hbac_evaluator.c
@@ -25,10 +25,9 @@
#include <stdlib.h>
#include <string.h>
-#include <unistr.h>
-#include <unicase.h>
#include <errno.h>
#include "providers/ipa/ipa_hbac.h"
+#include "util/sss_utf8.h"
#ifndef HAVE_ERRNO_T
#define HAVE_ERRNO_T
@@ -240,7 +239,6 @@ static errno_t hbac_evaluate_element(struct hbac_rule_element *rule_el,
size_t i, j;
const uint8_t *rule_name;
const uint8_t *req_name;
- int result;
int ret;
if (rule_el->category & HBAC_CATEGORY_ALL) {
@@ -255,21 +253,11 @@ static errno_t hbac_evaluate_element(struct hbac_rule_element *rule_el,
rule_name = (const uint8_t *) rule_el->names[i];
req_name = (const uint8_t *) req_el->name;
- /* Do a case-insensitive comparison.
- * The input must be encoded in UTF8.
- * We have no way of knowing the language,
- * so we'll pass NULL for the language and
- * hope for the best.
- */
- errno = 0;
- ret = u8_casecmp(rule_name, u8_strlen(rule_name),
- req_name, u8_strlen(req_name),
- NULL, NULL, &result);
- if (ret < 0) {
- return errno;
- }
-
- if (result == 0) {
+ /* Do a case-insensitive comparison. */
+ ret = sss_utf8_case_eq(rule_name, req_name);
+ if (ret != EOK && ret != ENOTUNIQ) {
+ return ret;
+ } else if (ret == EOK) {
*matched = true;
return EOK;
}
@@ -287,21 +275,11 @@ static errno_t hbac_evaluate_element(struct hbac_rule_element *rule_el,
for (j = 0; req_el->groups[j]; j++) {
req_name = (const uint8_t *) req_el->groups[j];
- /* Do a case-insensitive comparison.
- * The input must be encoded in UTF8.
- * We have no way of knowing the language,
- * so we'll pass NULL for the language and
- * hope for the best.
- */
- errno = 0;
- ret = u8_casecmp(rule_name, u8_strlen(rule_name),
- req_name, u8_strlen(req_name),
- NULL, NULL, &result);
- if (ret < 0) {
- return errno;
- }
-
- if (result == 0) {
+ /* Do a case-insensitive comparison. */
+ ret = sss_utf8_case_eq(rule_name, req_name);
+ if (ret != EOK && ret != ENOTUNIQ) {
+ return ret;
+ } else if (ret == EOK) {
*matched = true;
return EOK;
}
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 24f9125b7ca32a996da808c77a20df0686874bc0..1b28a92ef6c6569a52aecf37a4e37a1c7af95de1 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -32,8 +32,8 @@
#include <sys/time.h>
#include <errno.h>
#include <popt.h>
-#include <unistr.h>
#include "util/util.h"
+#include "util/sss_utf8.h"
#include "db/sysdb.h"
#include "confdb/confdb.h"
#include "dbus/dbus.h"
@@ -636,11 +636,3 @@ int responder_logrotate(DBusMessage *message,
return monitor_common_pong(message, conn);
}
-
-bool sss_utf8_check(const uint8_t *s, size_t n)
-{
- if (u8_check(s, n) == NULL) {
- return true;
- }
- return false;
-}
diff --git a/src/util/sss_utf8.c b/src/util/sss_utf8.c
new file mode 100644
index 0000000000000000000000000000000000000000..69917094633b4bafa79e285a1278bc5d69e05bd5
--- /dev/null
+++ b/src/util/sss_utf8.c
@@ -0,0 +1,111 @@
+/*
+ SSSD
+
+ Authors:
+ Stephen Gallagher <sgall...@redhat.com>
+
+ Copyright (C) 2011 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 "util/util.h"
+#include "sss_utf8.h"
+
+bool sss_utf8_check(const uint8_t *s, size_t n)
+{
+#ifdef HAVE_LIBUNISTRING
+ if (u8_check(s, n) == NULL) {
+ return true;
+ }
+ return false;
+#elif HAVE_GLIB2
+ return g_utf8_validate((const gchar *)s, n, NULL);
+#else
+#error No unicode library
+#endif
+}
+
+/* Returns EOK on match, ENOTUNIQ if comparison succeeds but
+ * does not match.
+ * May return other errno error codes on failure
+ */
+errno_t sss_utf8_case_eq(const uint8_t *s1, const uint8_t *s2)
+{
+
+ /* Do a case-insensitive comparison.
+ * The input must be encoded in UTF8.
+ * We have no way of knowing the language,
+ * so we'll pass NULL for the language and
+ * hope for the best.
+ */
+#ifdef HAVE_LIBUNISTRING
+ int ret;
+ int resultp;
+ size_t n1, n2;
+ errno = 0;
+
+ n1 = u8_strlen(s1);
+ n2 = u8_strlen(s2);
+
+ ret = u8_casecmp(s1, n1,
+ s2, n2,
+ NULL, NULL,
+ &resultp);
+ if (ret < 0) {
+ /* An error occurred */
+ return errno;
+ }
+
+ if (resultp == 0) {
+ return EOK;
+ }
+ return ENOTUNIQ;
+
+#elif HAVE_GLIB2
+ gchar *gs1;
+ gchar *gs2;
+ gssize n1, n2;
+ gint gret;
+ errno_t ret;
+
+ n1 = g_utf8_strlen((const gchar *)s1, -1);
+ n2 = g_utf8_strlen((const gchar *)s2, -1);
+
+ gs1 = g_utf8_casefold((const gchar *)s1, n1);
+ if (gs1 == NULL) {
+ return ENOMEM;
+ }
+
+ gs2 = g_utf8_casefold((const gchar *)s2, n2);
+ if (gs2 == NULL) {
+ return ENOMEM;
+ }
+
+ gret = g_utf8_collate(gs1, gs2);
+ if (gret == 0) {
+ ret = EOK;
+ } else {
+ ret = ENOTUNIQ;
+ }
+
+ g_free(gs1);
+ g_free(gs2);
+
+ return ret;
+
+#else
+#error No unicode library
+#endif
+}
diff --git a/src/util/sss_utf8.h b/src/util/sss_utf8.h
new file mode 100644
index 0000000000000000000000000000000000000000..9ddeba9e350f8b3a94abc39b5d91c9e317ad7a34
--- /dev/null
+++ b/src/util/sss_utf8.h
@@ -0,0 +1,39 @@
+/*
+ SSSD
+
+ Authors:
+ Stephen Gallagher <sgall...@redhat.com>
+
+ Copyright (C) 2011 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/>.
+*/
+
+#ifndef SSS_UTF8_H_
+#define SSS_UTF8_H_
+
+#ifdef HAVE_LIBUNISTRING
+#include <unistr.h>
+#include <unicase.h>
+#elif HAVE_GLIB2
+#include <glib.h>
+#endif
+#include "util/util.h"
+
+bool sss_utf8_check(const uint8_t *s, size_t n);
+
+errno_t sss_utf8_case_eq(const uint8_t *s1, const uint8_t *s2);
+
+
+#endif /* SSS_UTF8_H_ */
--
1.7.7.3
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
