[systemd-devel] [PATCH v2] localed: validate set-x11-keymap input

Jan Synacek jsynacek at redhat.com
Fri Nov 14 03:42:42 PST 2014


Try to validate the input similarly to how setxkbmap does it. Multiple
layouts and variants can be specified, separated by a comma. Variants
can also be left out, meaning that the user doesn't want any particular
variant for the respective layout.

Variants are validated respectively to their layouts. First variant
validates against first layout, second variant to second layout, etc. If
there are more entries of either layouts or variants, only their
respective counterparts are validated, the rest is ignored.

Examples:
$ set-x11-keymap us,cz  pc105 ,qwerty
"us" is not validated, because no respective variant was specified. "cz"
is checked for an existing "qwerty" variant, the check succeeds.

$ set-x11-keymap us pc105 ,qwerty
"us" is not validated as in the above example. The rest of the variants
is ignored, because there are no respective layouts.

$ set-x11-keymap us,cz pc105 qwerty
"us" is validated against the "qwerty" variant, check fails, because
there is no "qwerty" variant for the "us" layout.

$ set-x11-keymap us,cz pc105 euro,qwerty
Validation succeeds, there is the "euro" variant for the "us" layout,
and "qwerty" variant for the "cz" layout.

http://lists.freedesktop.org/archives/systemd-devel/2014-October/024411.html
---
v2:
- rewrite to use the X11Keymap struct
- use greedy allocation where it makes sense
- rename enum keymap_state to KeymapComponent
- on the server side, propagate error to the client and don't log it
- on the server side, change SD_BUS_ERROR_FAILED to SD_BUS_ERROR_INVALID_ARGS

 Makefile.am            |   2 +
 src/locale/localectl.c |  85 ++-----------
 src/locale/localed.c   |   6 +
 src/shared/xkb-util.c  | 319 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/xkb-util.h  |  50 ++++++++
 5 files changed, 385 insertions(+), 77 deletions(-)
 create mode 100644 src/shared/xkb-util.c
 create mode 100644 src/shared/xkb-util.h

diff --git a/Makefile.am b/Makefile.am
index 701666c..224582c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -772,6 +772,8 @@ libsystemd_shared_la_SOURCES = \
 	src/shared/time-util.h \
 	src/shared/locale-util.c \
 	src/shared/locale-util.h \
+	src/shared/xkb-util.c \
+	src/shared/xkb-util.h \
 	src/shared/mempool.c \
 	src/shared/mempool.h \
 	src/shared/hashmap.c \
diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index d4a2d29..08a1011 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -46,6 +46,7 @@
 #include "virt.h"
 #include "fileio.h"
 #include "locale-util.h"
+#include "xkb-util.h"
 
 static bool arg_no_pager = false;
 static bool arg_ask_password = true;
@@ -388,15 +389,8 @@ static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
 
 static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
         _cleanup_fclose_ FILE *f = NULL;
-        _cleanup_strv_free_ char **list = NULL;
-        char line[LINE_MAX];
-        enum {
-                NONE,
-                MODELS,
-                LAYOUTS,
-                VARIANTS,
-                OPTIONS
-        } state = NONE, look_for;
+        _cleanup_(xkb_keymap_free_components) X11Keymap keymap = {};
+        enum KeymapComponent look_for;
         int r;
 
         if (n > 2) {
@@ -404,12 +398,6 @@ static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
                 return -EINVAL;
         }
 
-        f = fopen("/usr/share/X11/xkb/rules/base.lst", "re");
-        if (!f) {
-                log_error("Failed to open keyboard mapping list. %m");
-                return -errno;
-        }
-
         if (streq(args[0], "list-x11-keymap-models"))
                 look_for = MODELS;
         else if (streq(args[0], "list-x11-keymap-layouts"))
@@ -421,71 +409,14 @@ static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
         else
                 assert_not_reached("Wrong parameter");
 
-        FOREACH_LINE(line, f, break) {
-                char *l, *w;
-
-                l = strstrip(line);
-
-                if (isempty(l))
-                        continue;
-
-                if (l[0] == '!') {
-                        if (startswith(l, "! model"))
-                                state = MODELS;
-                        else if (startswith(l, "! layout"))
-                                state = LAYOUTS;
-                        else if (startswith(l, "! variant"))
-                                state = VARIANTS;
-                        else if (startswith(l, "! option"))
-                                state = OPTIONS;
-                        else
-                                state = NONE;
-
-                        continue;
-                }
-
-                if (state != look_for)
-                        continue;
-
-                w = l + strcspn(l, WHITESPACE);
-
-                if (n > 1) {
-                        char *e;
-
-                        if (*w == 0)
-                                continue;
-
-                        *w = 0;
-                        w++;
-                        w += strspn(w, WHITESPACE);
-
-                        e = strchr(w, ':');
-                        if (!e)
-                                continue;
-
-                        *e = 0;
-
-                        if (!streq(w, args[1]))
-                                continue;
-                } else
-                        *w = 0;
-
-                 r = strv_extend(&list, l);
-                 if (r < 0)
-                         return log_oom();
-        }
-
-        if (strv_isempty(list)) {
-                log_error("Couldn't find any entries.");
-                return -ENOENT;
-        }
-
-        strv_sort(list);
-        strv_uniq(list);
+        r = xkb_keymap_get_components(&keymap);
+        if (r < 0)
+                return r;
 
         pager_open_if_enabled();
 
-        strv_print(list);
+        xkb_keymap_print_components(&keymap, look_for, (n > 1) ? args[1] : NULL);
+
         return 0;
 }
 
diff --git a/src/locale/localed.c b/src/locale/localed.c
index 9377ce5..e0f862c 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -40,6 +40,7 @@
 #include "bus-message.h"
 #include "event-util.h"
 #include "locale-util.h"
+#include "xkb-util.h"
 
 enum {
         /* We don't list LC_ALL here on purpose. People should be
@@ -1031,6 +1032,7 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
             !streq_ptr(model, c->x11_model) ||
             !streq_ptr(variant, c->x11_variant) ||
             !streq_ptr(options, c->x11_options)) {
+                _cleanup_free_ char *msg = NULL;
 
                 if ((layout && !string_is_safe(layout)) ||
                     (model && !string_is_safe(model)) ||
@@ -1050,6 +1052,10 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
                     free_and_strdup(&c->x11_options, options) < 0)
                         return -ENOMEM;
 
+                r = xkb_validate_keymaps(model, layout, variant, options, &msg);
+                if (r < 0)
+                        return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, msg);
+
                 r = x11_write_data(c);
                 if (r < 0) {
                         log_error("Failed to set X11 keyboard layout: %s", strerror(-r));
diff --git a/src/shared/xkb-util.c b/src/shared/xkb-util.c
new file mode 100644
index 0000000..89c9922
--- /dev/null
+++ b/src/shared/xkb-util.c
@@ -0,0 +1,319 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd 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
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "xkb-util.h"
+
+static char **xkb_parse_argument(const char *arg) {
+        _cleanup_free_ char *input = NULL;
+        char *token;
+        char **result = NULL;
+        int r;
+
+        if (!arg)
+                return NULL;
+
+        input = strdup(arg);
+        if (!input)
+                return NULL;
+
+        token = strsep(&input, ",");
+        while(token) {
+                r = strv_extend(&result, token);
+                if (r < 0)
+                        return NULL;
+                token = strsep(&input, ",");
+        }
+
+        return result;
+}
+
+int xkb_keymap_get_components(X11Keymap *keymap) {
+        _cleanup_strv_free_ char **models = NULL, **options = NULL;
+        _cleanup_fclose_ FILE *f;
+        char line[LINE_MAX];
+        enum KeymapComponent state = NONE;
+        size_t m = 0, o = 0, allocm = 0, alloco = 0;
+
+        Hashmap *x11_layouts;
+        int r;
+
+        x11_layouts = hashmap_new(&string_hash_ops);
+        if (!x11_layouts)
+                return log_oom();
+
+        f = fopen("/usr/share/X11/xkb/rules/base.lst", "re");
+        if (!f) {
+                log_error("Failed to open keyboard mapping list. %m");
+                return -errno;
+        }
+
+        FOREACH_LINE(line, f, break) {
+                char *l, *w;
+                _cleanup_free_ char *layout = NULL;
+
+                l = strstrip(line);
+
+                if (isempty(l))
+                        continue;
+
+                if (l[0] == '!') {
+                        if (startswith(l, "! model"))
+                                state = MODELS;
+                        else if (startswith(l, "! layout"))
+                                state = LAYOUTS;
+                        else if (startswith(l, "! variant"))
+                                state = VARIANTS;
+                        else if (startswith(l, "! option"))
+                                state = OPTIONS;
+                        else
+                                state = NONE;
+
+                        continue;
+                }
+
+                w = l + strcspn(l, WHITESPACE);
+
+                if (state == VARIANTS && *w != 0) {
+                        char *e;
+
+                        *w = 0;
+                        w++;
+                        w += strspn(w, WHITESPACE);
+
+                        e = strchr(w, ':');
+                        if (!e)
+                                continue;
+
+                        *e = 0;
+                        layout = strdup(w);
+                } else {
+                        *w = 0;
+                        layout = strdup(l);
+                }
+
+                if (!layout)
+                        return log_oom();
+
+                if (state == LAYOUTS) {
+                        _cleanup_set_free_ Set *item = NULL;
+
+                        item = set_new(&string_hash_ops);
+                        if (!item)
+                                return log_oom();
+
+                        if (!hashmap_contains(x11_layouts, layout)) {
+                                r = hashmap_put(x11_layouts, layout, item);
+                                if (r < 0)
+                                        return log_oom();
+                        }
+
+                        item = NULL;
+                        layout = NULL;
+
+                } else if (state == VARIANTS) {
+                        _cleanup_set_free_ Set *item = NULL;
+
+                        if (!hashmap_contains(x11_layouts, layout)) {
+                                item = set_new(&string_hash_ops);
+                                if (!item)
+                                        return log_oom();
+
+                        } else {
+                                item = hashmap_get(x11_layouts, layout);
+                                assert(item);
+                        }
+
+                        r = set_put_strdup(item, l);
+                        if (r < 0)
+                                return log_oom();
+
+                        r = hashmap_update(x11_layouts, layout, item);
+                        if (r < 0)
+                                return log_oom();
+
+                        item = NULL;
+
+                } else if (state == MODELS) {
+                        if (!GREEDY_REALLOC(models, allocm, m + 2))
+                                return log_oom();
+
+                        models[m] = strdup(l);
+                        if (!models[m])
+                                return log_oom();
+
+                        m++;
+                } else if (state == OPTIONS) {
+                        if (!GREEDY_REALLOC(options, alloco, o + 2))
+                                return log_oom();
+
+                        options[o] = strdup(l);
+                        if (!options[o])
+                                return log_oom();
+
+                        o++;
+                }
+        }
+
+        if (m == 0)
+                log_warning("Couldn't find any xkb models.");
+        if (o == 0)
+                log_warning("Couldn't find any xkb options.");
+
+        if (hashmap_isempty(x11_layouts))
+                log_warning("Couldn't find any xkb layouts or variants.");
+        else
+                keymap->x11_layouts = x11_layouts;
+
+        models[m] = NULL;
+        keymap->models = models;
+        models = NULL;
+        options[o] = NULL;
+        keymap->options = options;
+        options = NULL;
+
+        return 0;
+}
+
+int xkb_validate_keymaps(const char *model,
+                         const char *layouts_arg,
+                         const char *variants_arg,
+                         const char *options_arg,
+                         char **error)
+{
+        _cleanup_(xkb_keymap_free_components) X11Keymap keymap = {};
+        _cleanup_strv_free_ char **layouts = NULL, **variants = NULL;
+        char **it;
+        size_t i = 0, n;
+        int r;
+
+        r = xkb_keymap_get_components(&keymap);
+        if (r < 0)
+                return r;
+
+        layouts = xkb_parse_argument(layouts_arg);
+        variants = xkb_parse_argument(variants_arg);
+        n = strv_length(variants);
+
+        STRV_FOREACH(it, layouts) {
+                Set *x11_layout;
+
+                /* Empty layout, skip the variant check. */
+                if (streq(*it, ""))
+                        goto next;
+
+                x11_layout = hashmap_get(keymap.x11_layouts, *it);
+                if (!x11_layout) {
+                        r = asprintf(error, "Requested layout '%s' not available.\n", *it);
+                        if (r < 0)
+                                return log_oom();
+                        return -EINVAL;
+                }
+
+                /* No variants, nothing to check. */
+                if (n == 0)
+                        goto next;
+
+                /* More layouts than variants, no need to look at variants anymore. */
+                if (i >= n)
+                        goto next;
+
+                /* Variant left out, skip the check. */
+                if (streq(variants[i], ""))
+                        goto next;
+
+                if (!set_contains(x11_layout, variants[i])) {
+                        r = asprintf(error, "Requested variant '%s' for layout '%s' not available.\n", variants[i], *it);
+                        if (r < 0)
+                                return log_oom();
+                        return -EINVAL;
+                }
+        next:
+                i++;
+        }
+
+        /* Since setxkbmap doesn't validate model and options, we
+           don't either. It might be good to add the check, though. */
+        return 0;
+}
+
+void xkb_keymap_print_components(X11Keymap *keymap, enum KeymapComponent look_for, const char *layout_prefix) {
+        if (look_for == LAYOUTS) {
+                Set *s;
+                char *k;
+                Iterator i;
+                /* XXX: Is there a better way to sort Hashmap keys? */
+                _cleanup_strv_free_ char **tmp = NULL;
+
+                HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i)
+                        if (strv_extend(&tmp, k) < 0)
+                                (void) log_oom();
+
+                strv_sort(tmp);
+                strv_print(tmp);
+
+        } else if (look_for == VARIANTS) {
+                Set *s;
+                char *k;
+                Iterator i, j;
+                /* XXX: Is there a better way to sort Set keys? */
+                _cleanup_strv_free_ char **tmp = NULL;
+
+                if (layout_prefix) {
+                        s = hashmap_get(keymap->x11_layouts, layout_prefix);
+                        SET_FOREACH(k, s, j)
+                                if (strv_extend(&tmp, k) < 0)
+                                        (void) log_oom();
+                } else {
+                        HASHMAP_FOREACH(s, keymap->x11_layouts, i)
+                                SET_FOREACH(k, s, j)
+                                        strv_extend(&tmp, k);
+                }
+
+                strv_sort(tmp);
+                strv_print(tmp);
+
+        } else if (look_for == MODELS) {
+                strv_sort(keymap->models);
+                strv_print(keymap->models);
+
+        } else if (look_for == OPTIONS) {
+                strv_sort(keymap->options);
+                strv_print(keymap->options);
+
+        } else
+                assert_not_reached("Wrong parameter.");
+}
+
+void xkb_keymap_free_components(X11Keymap *keymap) {
+        if (keymap->x11_layouts) {
+                Set *s;
+                char *k;
+                Iterator i;
+
+                HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i) {
+                        free(k);
+                        set_free_free(s);
+                }
+                hashmap_free(keymap->x11_layouts);
+        }
+        strv_free(keymap->models);
+        strv_free(keymap->options);
+}
diff --git a/src/shared/xkb-util.h b/src/shared/xkb-util.h
new file mode 100644
index 0000000..548dd29
--- /dev/null
+++ b/src/shared/xkb-util.h
@@ -0,0 +1,50 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd 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
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <stdbool.h>
+
+#include "macro.h"
+#include "strv.h"
+#include "util.h"
+#include "set.h"
+
+enum KeymapComponent {
+        NONE,
+        MODELS,
+        LAYOUTS,
+        VARIANTS,
+        OPTIONS
+};
+
+typedef struct {
+        Hashmap *x11_layouts; /* char* -> Set* */
+        char **models;
+        char **options;
+} X11Keymap;
+
+
+int xkb_keymap_get_components(X11Keymap *keymap);
+int xkb_validate_keymaps(const char *model, const char *layouts_arg, const char *variants_arg, const char *options_arg, char **error);
+
+void xkb_keymap_print_components(X11Keymap *keymap, enum KeymapComponent, const char *layout_prefix);
+void xkb_keymap_free_components(X11Keymap *keymap);
-- 
1.9.3



More information about the systemd-devel mailing list