Merge pull request #3394 from orestisf1993/validate-utf8

libi3: validate UTF8 strings

include/libi3.h

@@ -200,7 +200,7 @@ i3String *i3string_from_markup(const char *from_markup);
  * Returns the newly-allocated i3String.
  *
  */
-i3String *i3string_from_utf8_with_length(const char *from_utf8, size_t num_bytes);
+i3String *i3string_from_utf8_with_length(const char *from_utf8, ssize_t num_bytes);
 
 /**
  * Build an i3String from an UTF-8 encoded string in Pango markup with fixed

libi3/string.c

@@ -30,15 +30,7 @@ struct _i3String {
  *
  */
 i3String *i3string_from_utf8(const char *from_utf8) {
-    i3String *str = scalloc(1, sizeof(i3String));
+    return i3string_from_utf8_with_length(from_utf8, -1);
-
-    /* Get the text */
-    str->utf8 = sstrdup(from_utf8);
-
-    /* Compute and store the length */
-    str->num_bytes = strlen(str->utf8);
-
-    return str;
 }
 
 /*
@@ -60,16 +52,14 @@ i3String *i3string_from_markup(const char *from_markup) {
  * Returns the newly-allocated i3String.
  *
  */
-i3String *i3string_from_utf8_with_length(const char *from_utf8, size_t num_bytes) {
+i3String *i3string_from_utf8_with_length(const char *from_utf8, ssize_t num_bytes) {
     i3String *str = scalloc(1, sizeof(i3String));
 
-    /* Copy the actual text to our i3String */
+    /* g_utf8_make_valid NULL-terminates the string. */
-    str->utf8 = scalloc(num_bytes + 1, 1);
+    str->utf8 = g_utf8_make_valid(from_utf8, num_bytes);
-    strncpy(str->utf8, from_utf8, num_bytes);
-    str->utf8[num_bytes] = '\0';
 
-    /* Store the length */
+    /* num_bytes < 0 means NULL-terminated string, need to calculate length */
-    str->num_bytes = num_bytes;
+    str->num_bytes = num_bytes < 0 ? strlen(str->utf8) : (size_t)num_bytes;
 
     return str;
 }

src/load_layout.c

@@ -611,9 +611,16 @@ void tree_append_json(Con *con, const char *buf, const size_t len, char **errorm
     yajl_config(hand, yajl_allow_comments, true);
     /* Allow multiple values, i.e. multiple nodes to attach */
     yajl_config(hand, yajl_allow_multiple_values, true);
-    /* Allow strings that are not valid UTF8. Could be possible if a container
+    /* We don't need to validate that the input is valid UTF8 here.
-     * name contains such characters. If yajl stops parsing because of this, an
+     * tree_append_json is called in two cases:
-     * in-place restart could fail: see #3156. */
+     * 1. With the append_layout command. json_validate is called first and will
+     *    fail on invalid UTF8 characters so we don't need to recheck.
+     * 2. With an in-place restart. The rest of the codebase should be
+     *    responsible for producing valid UTF8 JSON output. If not,
+     *    tree_append_json will just preserve invalid UTF8 strings in the tree
+     *    instead of failing to parse the layout file which could lead to
+     *    problems like in #3156.
+     * Either way, disabling UTF8 validation slightly speeds up yajl. */
     yajl_config(hand, yajl_dont_validate_strings, true);
     json_node = con;
     to_focus = NULL;

src/tree.c

@@ -92,6 +92,10 @@ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) {
 
     DLOG("appended tree, using new root\n");
     croot = TAILQ_FIRST(&(croot->nodes_head));
+    if (!croot) {
+        /* tree_append_json failed. Continuing here would segfault. */
+        goto out;
+    }
     DLOG("new root = %p\n", croot);
     Con *out = TAILQ_FIRST(&(croot->nodes_head));
     DLOG("out = %p\n", out);

testcases/t/300-restart-non-utf8.t

@@ -0,0 +1,34 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • https://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • https://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • https://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Verify that i3 does not crash when restart is issued while a window with a
+# title that contains non-UTF8 characters is open.
+# Ticket: #3156
+# Bug still in: 4.15-241-g9dc4df81
+use i3test;
+
+my $ws = fresh_workspace;
+open_window(name => "\x{AA} <-- invalid");
+
+cmd 'restart';
+does_i3_live;
+
+# Confirm that the invalid character got replaced with U+FFFD - "REPLACEMENT
+# CHARACTER"
+cmd '[title="^' . "\x{fffd}" . ' <-- invalid$"] fullscreen enable';
+is_num_fullscreen($ws, 1, 'title based criterion works');
+
+done_testing;