[bug-gettext] RFC: move LANGUAGE check out of gettext()

[Daiki Ueno]

Hello,

A while ago Matthias Clasen pointed me to a bug that is caused by a race
condition between a getenv() call in gettext() and a setenv() call in
another thread:
https://bugzilla.gnome.org/show_bug.cgi?id=754951

The direct cause of this bug is that gettext() tries to check LANGUAGE
envvar, while the string content returned by getenv() can be overwritten
by setenv() before being used.

To mitigate this, I was wondering if it would be possible to move the
getenv() call from gettext() to setlocale(), which is typically called
at program startup, and cache the value in a static variable.

The attached patch is an experiment implementing that in libintl, though
in practice it would require a change in glibc's setlocale
implementation.

Comments appreciated.

Regards,
-- 
Daiki Ueno

>From df259036a888e89d3fae7f4b4781b015776852db Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Tue, 10 May 2016 17:53:42 +0900
Subject: [PATCH] intl: Check LANGUAGE envvar in setlocale

Problem reported by Matthias Clasen in:
https://bugzilla.gnome.org/show_bug.cgi?id=754951
* gettext-runtime/intl/dcigettext.c (_nl_default_overriding_languages):
New static variable.
(_nl_overriding_languages): New variable to cache the value of LANGUAGE
envvar.
(_nl_locale_name_override): New function, split from
guess_category_value.  Set _nl_overriding_languages once LANGUAGE envvar
is successfully retrieved.
(guess_category_value): Call _nl_locale_name_override instead of
directly calling getenv.
* gettext-runtime/intl/gettextP.h (_nl_locale_name_override): New
function declaration.
(_nl_overriding_languages): New variable declaration.
* gettext-runtime/intl/setlocale.c (libintl_setlocale)
(libintl_newlocale): Call _nl_locale_name_override to ensure
_nl_overriding_languages.
---
 gettext-runtime/intl/dcigettext.c | 33 +++++++++++++++++++++++++++++++--
 gettext-runtime/intl/gettextP.h   | 12 ++++++++++++
 gettext-runtime/intl/setlocale.c  | 12 ++++++++++--
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/gettext-runtime/intl/dcigettext.c 
b/gettext-runtime/intl/dcigettext.c
index 83bd775..318c27b 100644
--- a/gettext-runtime/intl/dcigettext.c
+++ b/gettext-runtime/intl/dcigettext.c
@@ -342,6 +342,12 @@ libc_hidden_data_def (_nl_default_dirname)
 struct binding *_nl_domain_bindings;
 #endif
 
+/* Default value of the 'LANGUAGE' environment variable used by
+   gettext(3) prior any call to setlocale(3) or gettext(3).  The
+   default value for this is "".  */
+static const char _nl_default_overriding_languages[] = "";
+const char *_nl_overriding_languages = _nl_default_overriding_languages;
+
 /* Prototypes for local functions.  */
 static char *plural_lookup (struct loaded_l10nfile *domain,
                            unsigned long int n,
@@ -1510,6 +1516,28 @@ category_to_name (int category)
 }
 #endif
 
+const char *
+internal_function
+_nl_locale_name_override (void)
+{
+  const char *language;
+
+  /* The value is stored in _nl_overriding_languages to avoid race
+     when setenv and gettext are called in separate threads. */
+  if (_nl_overriding_languages != _nl_default_overriding_languages)
+    return _nl_overriding_languages;
+
+  language = getenv ("LANGUAGE");
+
+  if (language != NULL && language[0] != '\0')
+    _nl_overriding_languages = strdup (language);
+  else
+    _nl_overriding_languages = NULL;
+  language = _nl_overriding_languages;
+
+  return language;
+}
+
 /* Guess value of current locale from value of the environment variables
    or system-dependent defaults.  */
 static const char *
@@ -1584,9 +1612,10 @@ guess_category_value (int category, const char 
*categoryname)
 
   /* The highest priority value is the value of the 'LANGUAGE' environment
      variable.  */
-  language = getenv ("LANGUAGE");
-  if (language != NULL && language[0] != '\0')
+  language = _nl_locale_name_override ();
+  if (language != NULL)
     return language;
+
 #if !defined IN_LIBGLOCALE && !defined _LIBC
   /* The next priority value is the locale name, if not defaulted.  */
   if (locale_defaulted)
diff --git a/gettext-runtime/intl/gettextP.h b/gettext-runtime/intl/gettextP.h
index 52be2fc..59c6cf1 100644
--- a/gettext-runtime/intl/gettextP.h
+++ b/gettext-runtime/intl/gettextP.h
@@ -254,6 +254,7 @@ extern const char *_nl_locale_name_default (void);
 # define gl_locale_name _nl_locale_name
 /* extern const char *_nl_locale_name (int category,
                                       const char *categoryname); */
+extern const char *_nl_locale_name_override (void);
 #endif
 
 struct loaded_l10nfile *_nl_find_domain (const char *__dirname, char *__locale,
@@ -308,6 +309,17 @@ extern const char _nl_default_default_domain[] 
attribute_hidden;
 /* Default text domain in which entries for gettext(3) are to be found.  */
 extern const char *_nl_current_default_domain attribute_hidden;
 
+/* The internal variables in the standalone libintl.a must have different
+   names than the internal variables in GNU libc, otherwise programs
+   using libintl.a cannot be linked statically.  */
+#if !defined _LIBC
+# define _nl_overriding_languages libintl_nl_overriding_languages
+#endif
+
+/* Default list of languages which override locale environment
+   variables.  */
+extern const char *_nl_overriding_languages attribute_hidden;
+
 /* @@ begin of epilog @@ */
 
 #endif /* gettextP.h  */
diff --git a/gettext-runtime/intl/setlocale.c b/gettext-runtime/intl/setlocale.c
index 07c2305..3e0fea8 100644
--- a/gettext-runtime/intl/setlocale.c
+++ b/gettext-runtime/intl/setlocale.c
@@ -884,6 +884,7 @@ libintl_setlocale (int category, const char *locale)
             }
 
           /* All steps were successful.  */
+          (void) _nl_locale_name_override ();
           ++_nl_msg_cat_cntr;
           free (saved_locale);
           return setlocale (LC_ALL, NULL);
@@ -904,7 +905,10 @@ libintl_setlocale (int category, const char *locale)
 
           result = setlocale_single (category, name);
           if (result != NULL)
-            ++_nl_msg_cat_cntr;
+            {
+              (void) _nl_locale_name_override ();
+              ++_nl_msg_cat_cntr;
+            }
           return result;
         }
     }
@@ -941,6 +945,7 @@ libintl_setlocale (int category, const char *locale)
             }
 
           /* It was really successful.  */
+          (void) _nl_locale_name_override ();
           ++_nl_msg_cat_cntr;
           free (saved_locale);
           return setlocale (LC_ALL, NULL);
@@ -950,7 +955,10 @@ libintl_setlocale (int category, const char *locale)
         {
           char *result = setlocale_single (category, locale);
           if (result != NULL)
-            ++_nl_msg_cat_cntr;
+            {
+              (void) _nl_locale_name_override ();
+              ++_nl_msg_cat_cntr;
+            }
           return result;
         }
     }
-- 
2.5.5