Merge pull request #3144 from grondo/issue#3124

Fix F58 encoding in non-multibyte locales

src/cmd/flux-job.c

@@ -23,6 +23,7 @@
 #include <string.h>
 #include <ctype.h>
 #include <assert.h>
+#include <locale.h>
 #include <jansson.h>
 #include <argz.h>
 #include <czmq.h>
@@ -474,6 +475,11 @@ int main (int argc, char *argv[])
     int optindex;
     int exitval;
 
+    /*  Initialize locale from environment. Allows unicode character
+     *   prefix in F58 encoded JOBIDs in wide-character capable locales.
+     */
+    setlocale (LC_ALL, "");
+
     log_init ("flux-job");
 
     p = optparse_create ("flux-job");

src/common/libjob/test/job.c

@@ -12,6 +12,8 @@
 #include "config.h"
 #endif
 
+#include <locale.h>
+
 #include <flux/core.h>
 
 #include "src/common/libtap/tap.h"
@@ -362,6 +364,11 @@ void check_jobid_parse_encode (void)
     struct jobid_parse_test *tp = jobid_parse_tests;
     while (tp->type != NULL) {
         memset (buf, 0, sizeof (buf));
+        if (MB_CUR_MAX == 1 && strcmp (tp->type, "f58") == 0) {
+            tap_skip (4, "Skipping F58 encode/decode due to current locale");
+            tp++;
+            continue;
+        }
         ok (flux_job_id_encode (tp->id, tp->type, buf, sizeof (buf)) == 0,
             "flux_job_id_encode (%ju, %s) == 0", (uintmax_t) tp->id, tp->type);
         is (buf, tp->string,
@@ -398,6 +405,9 @@ int main (int argc, char *argv[])
 {
     plan (NO_PLAN);
 
+    /* fluid F58 tests require unicode locale initialization */
+    setlocale (LC_ALL, "en_US.UTF-8");
+
     check_jobkey ();
 
     check_corner_case ();

src/common/libutil/fluid.c

@@ -21,11 +21,11 @@
 #include <stdbool.h>
 #include <errno.h>
 #include <ctype.h>
+#include <locale.h>
 
 #include "fluid.h"
 #include "mnemonic.h"
 
-
 /* fluid: [ts:40 id:14 seq:10] */
 static const int bits_per_ts = 40;
 static const int bits_per_id = 14;
@@ -169,27 +169,43 @@ static int b58revenc (char *buf, int bufsz, fluid_t id)
     return index;
 }
 
+static inline int is_utf8_locale (void)
+{
+    /* Assume if MB_CUR_MAX > 1, this locale can handle wide characters
+     *  and therefore will properly handle UTF-8.
+     */
+    if (MB_CUR_MAX > 1)
+        return 1;
+    return 0;
+}
+
 static int fluid_f58_encode (char *buf, int bufsz, fluid_t id)
 {
     int count;
+    const char *prefix = f58_prefix;
     char b58reverse[13];
     int index = 0;
+
     if (buf == NULL || bufsz <= 0) {
         errno = EINVAL;
         return -1;
     }
-    if (bufsz <= strlen (f58_prefix) + 1) {
+
+    /* Use alternate "f" prefix if locale is not multibyte */
+    if (!is_utf8_locale())
+        prefix = f58_alt_prefix;
+
+    if (bufsz <= strlen (prefix) + 1) {
         errno = EOVERFLOW;
         return -1;
     }
-
     if ((count = b58revenc (b58reverse, sizeof (b58reverse), id)) < 0) {
         errno = EINVAL;
         return -1;
     }
 
     /* Copy prefix to buf and zero remaining bytes */
-    (void) strncpy (buf, f58_prefix, bufsz);
+    (void) strncpy (buf, prefix, bufsz);
     index = strlen (buf);
 
     if (index + count + 1 > bufsz) {

src/common/libutil/test/fluid.c

@@ -9,50 +9,53 @@
 \************************************************************/
 
 #include <errno.h>
+#include <string.h>
+#include <locale.h>
 
 #include "src/common/libtap/tap.h"
 #include "src/common/libutil/fluid.h"
 
 struct f58_test {
     fluid_t id;
     const char *f58;
+    const char *f58_alt;
 };
 
 struct f58_test f58_tests [] = {
-    { 0, "ƒ1" },
-    { 1, "ƒ2" },
-    { 57, "ƒz" },
-    { 1234, "ƒNH" },
-    { 1888, "ƒZZ" },
-    { 3363, "ƒzz" },
-    { 3364, "ƒ211" },
-    { 4369, "ƒ2JL" },
-    { 65535, "ƒLUv" },
-    { 4294967295, "ƒ7YXq9G" },
-    { 633528662, "ƒxyzzy" },
-    { 6731191091817518LL, "ƒuZZybuNNy" },
-    { 18446744073709551614UL, "ƒjpXCZedGfVP" },
-    { 18446744073709551615UL, "ƒjpXCZedGfVQ" },
-    { 0, NULL },
+    { 0, "ƒ1", "f1" },
+    { 1, "ƒ2", "f2" },
+    { 57, "ƒz", "fz" },
+    { 1234, "ƒNH", "fNH" },
+    { 1888, "ƒZZ", "fZZ" },
+    { 3363, "ƒzz", "fzz" },
+    { 3364, "ƒ211", "f211" },
+    { 4369, "ƒ2JL", "f2JL" },
+    { 65535, "ƒLUv", "fLUv" },
+    { 4294967295, "ƒ7YXq9G", "f7YXq9G" },
+    { 633528662, "ƒxyzzy", "fxyzzy" },
+    { 6731191091817518LL, "ƒuZZybuNNy", "fuZZybuNNy" },
+    { 18446744073709551614UL, "ƒjpXCZedGfVP", "fjpXCZedGfVP" },
+    { 18446744073709551615UL, "ƒjpXCZedGfVQ", "fjpXCZedGfVQ" },
+    { 0, NULL, NULL },
 };
 
 struct f58_test f58_alt_tests [] = {
-    { 0, "f1" },
-    { 0, "f111" },
-    { 1, "f2" },
-    { 57, "fz" },
-    { 1234, "fNH" },
-    { 1888, "fZZ" },
-    { 3363, "fzz" },
-    { 3364, "f211" },
-    { 4369, "f2JL" },
-    { 65535, "fLUv" },
-    { 4294967295, "f7YXq9G" },
-    { 633528662, "fxyzzy" },
-    { 6731191091817518LL, "fuZZybuNNy" },
-    { 18446744073709551614UL, "fjpXCZedGfVP" },
-    { 18446744073709551615UL, "fjpXCZedGfVQ" },
-    { 0, NULL },
+    { 0, "f1", NULL },
+    { 0, "f111", NULL },
+    { 1, "f2", NULL },
+    { 57, "fz", NULL },
+    { 1234, "fNH", NULL },
+    { 1888, "fZZ", NULL },
+    { 3363, "fzz", NULL },
+    { 3364, "f211", NULL },
+    { 4369, "f2JL", NULL },
+    { 65535, "fLUv", NULL },
+    { 4294967295, "f7YXq9G", NULL },
+    { 633528662, "fxyzzy", NULL },
+    { 6731191091817518LL, "fuZZybuNNy", NULL },
+    { 18446744073709551614UL, "fjpXCZedGfVP", NULL },
+    { 18446744073709551615UL, "fjpXCZedGfVQ", NULL },
+    { 0, NULL, NULL },
 };
 
 void test_f58 (void)
@@ -64,8 +67,12 @@ void test_f58 (void)
     while (tp->f58 != NULL) {
         ok (fluid_encode (buf, sizeof(buf), tp->id, type) == 0,
             "f58_encode (%ju)", tp->id);
-        is (buf, tp->f58,
-            "f58_encode %ju -> %s", tp->id, buf);
+        if (strcmp (buf, tp->f58) == 0
+            || strcmp (buf, tp->f58_alt) == 0)
+            pass ("f58_encode %ju -> %s", tp->id, buf);
+        else
+            fail ("f58_encode %ju: got %s expected %s",
+                  tp->id, buf, tp->f58);
         ok (fluid_decode (tp->f58, &id, type) == 0,
             "f58_decode (%s)", tp->f58);
         ok (id == tp->id,
@@ -83,8 +90,9 @@ void test_f58 (void)
 
     ok (fluid_encode (buf, 1, 1, type) < 0 && errno == EOVERFLOW,
         "fluid_encode (buf, 1, 1, F58) returns EOVERFLOW");
-    ok (fluid_encode (buf, 5, 65535, type) < 0 && errno == EOVERFLOW,
-        "fluid_encode (buf, 5, 65535, F58) returns EOVERFLOW");
+    errno = 0;
+    ok (fluid_encode (buf, 4, 65535, type) < 0 && errno == EOVERFLOW,
+        "fluid_encode (buf, 4, 65535, F58) returns EOVERFLOW: %s", strerror (errno));
 
     ok (fluid_decode ("1234", &id, type) < 0 && errno == EINVAL,
         "fluid_decode ('aaa', FLUID_STRING_F58) returns EINVAL");
@@ -318,6 +326,9 @@ int main (int argc, char *argv[])
 {
     plan (NO_PLAN);
 
+    /* Locale initialization required for fluid_f58_encode() */
+    setlocale (LC_ALL, "");
+
     test_basic ();
     test_f58 ();
     test_fluid_parse ();

src/shell/shell.c

@@ -18,6 +18,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdarg.h>
+#include <locale.h>
 #include <jansson.h>
 #include <czmq.h>
 #include <flux/core.h>
@@ -1100,6 +1101,10 @@ int main (int argc, char *argv[])
     flux_shell_t shell;
     int i;
 
+    /* Initialize locale from environment
+     */
+    setlocale (LC_ALL, "");
+
     shell_log_init (&shell, shell_name);
 
     shell_initialize (&shell);

t/python/t0010-job.py

@@ -17,6 +17,7 @@
 import unittest
 import datetime
 import signal
+import locale
 from glob import glob
 
 import yaml
@@ -42,6 +43,7 @@ class TestJob(unittest.TestCase):
     @classmethod
     def setUpClass(self):
         self.fh = flux.Flux()
+        self.use_ascii = locale.getlocale()[1] != "UTF-8"
 
         self.jobspec_dir = os.path.abspath(
             os.path.join(os.environ["FLUX_SOURCE_DIR"], "t", "jobspec")
@@ -416,6 +418,8 @@ def test_24_jobid(self):
         ]
         for test in parse_tests:
             for key in test:
+                if key == "f58" and self.use_ascii:
+                    continue
                 jobid_int = test["int"]
                 jobid = job.JobID(test[key])
                 self.assertEqual(jobid, jobid_int)

t/t2201-job-cmd.t

@@ -13,15 +13,15 @@ MAXJOBID_HEX="0xffffffffffffffff"
 MAXJOBID_KVS="job.ffff.ffff.ffff.ffff"
 MAXJOBID_DOTHEX="ffff.ffff.ffff.ffff"
 MAXJOBID_WORDS="natural-analyze-verbal--natural-analyze-verbal"
-MAXJOBID_F58="ƒjpXCZedGfVQ"
+MAXJOBID_F58="fjpXCZedGfVQ"
 MAXJOBIDS_LIST="$MAXJOBID_DEC $MAXJOBID_HEX $MAXJOBID_KVS $MAXJOBID_DOTHEX $MAXJOBID_WORDS $MAXJOBID_F58"
 
 MINJOBID_DEC=0
 MINJOBID_HEX="0x0"
 MINJOBID_KVS="job.0000.0000.0000.0000"
 MINJOBID_DOTHEX="0000.0000.0000.0000"
 MINJOBID_WORDS="academy-academy-academy--academy-academy-academy"
-MINJOBID_F58="ƒ1"
+MINJOBID_F58="f1"
 MINJOBIDS_LIST="$MINJOBID_DEC $MINJOBID_HEX $MINJOBID_KVS $MINJOBID_DOTHEX $MINJOBID_WORDS $MINJOBID_F58"
 
 test_under_flux 1 job
@@ -162,6 +162,13 @@ test_expect_success 'flux-job: id --to=f58 works' '
 	test "$jobid" = "$MINJOBID_F58"
 '
 
+UTF8_LOCALE=$(locale -a | grep UTF-8)
+test -n "$UTF8_LOCALE" && test_set_prereq UTF8_LOCALE
+test_expect_success UTF8_LOCALE 'flux-job: f58 can use multibyte prefix' '
+	jobid=$(LC_ALL=${UTF8_LOCALE} flux job id --to=f58 1) &&
+	test "$jobid" = "ƒ2"
+'
+
 test_expect_success 'flux-job: id fails on bad input' '
 	test_must_fail flux job id badstring &&
 	test_must_fail flux job id job.0000.0000 &&