From 6a1f653508052b631afc12a16c2854cb0eefe239 Mon Sep 17 00:00:00 2001
From: Orestis Floros <orestisf1993@gmail.com>
Date: Sun, 9 Sep 2018 15:32:54 +0300
Subject: [PATCH] libi3: validate UTF8 strings

Will validate container / window titles.

Fixes #3156.
---
 include/libi3.h                    |  2 +-
 libi3/string.c                     | 22 ++++++-------------
 src/load_layout.c                  | 13 +++++++++---
 testcases/t/300-restart-non-utf8.t | 34 ++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 20 deletions(-)
 create mode 100644 testcases/t/300-restart-non-utf8.t

diff --git a/include/libi3.h b/include/libi3.h
index d3b40796..91395328 100644
--- a/include/libi3.h
+++ b/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
diff --git a/libi3/string.c b/libi3/string.c
index edd588da..f78a140f 100644
--- a/libi3/string.c
+++ b/libi3/string.c
@@ -30,15 +30,7 @@ struct _i3String {
  *
  */
 i3String *i3string_from_utf8(const char *from_utf8) {
-    i3String *str = scalloc(1, sizeof(i3String));
-
-    /* Get the text */
-    str->utf8 = sstrdup(from_utf8);
-
-    /* Compute and store the length */
-    str->num_bytes = strlen(str->utf8);
-
-    return str;
+    return i3string_from_utf8_with_length(from_utf8, -1);
 }
 
 /*
@@ -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 */
-    str->utf8 = scalloc(num_bytes + 1, 1);
-    strncpy(str->utf8, from_utf8, num_bytes);
-    str->utf8[num_bytes] = '\0';
+    /* g_utf8_make_valid NULL-terminates the string. */
+    str->utf8 = g_utf8_make_valid(from_utf8, num_bytes);
 
-    /* Store the length */
-    str->num_bytes = num_bytes;
+    /* num_bytes < 0 means NULL-terminated string, need to calculate length */
+    str->num_bytes = num_bytes < 0 ? strlen(str->utf8) : (size_t)num_bytes;
 
     return str;
 }
diff --git a/src/load_layout.c b/src/load_layout.c
index d2ad4e87..32da9c47 100644
--- a/src/load_layout.c
+++ b/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
-     * name contains such characters. If yajl stops parsing because of this, an
-     * in-place restart could fail: see #3156. */
+    /* We don't need to validate that the input is valid UTF8 here.
+     * tree_append_json is called in two cases:
+     * 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;
diff --git a/testcases/t/300-restart-non-utf8.t b/testcases/t/300-restart-non-utf8.t
new file mode 100644
index 00000000..81b56583
--- /dev/null
+++ b/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;
-- 
2.39.5