validate JSON before loading

This commit also introduces slurp() which reads a file in its entirety. Using
this function instead of doing IO in the functions in load_layout.c again and
again makes the code cleaner (fixing at least two memory leaks) and avoids
re-reading the same file 3 times.

related to #2755

include/load_layout.h

@@ -31,6 +31,12 @@ typedef enum {
  * determine whether the file contains workspaces or regular containers, which
  * is important to know when deciding where (and how) to append the contents.
  * */
-json_content_t json_determine_content(const char *filename);
+json_content_t json_determine_content(const char *buf, const size_t len);
 
-void tree_append_json(Con *con, const char *filename, char **errormsg);
+/**
+ * Returns true if the provided JSON could be parsed by yajl.
+ *
+ */
+bool json_validate(const char *buf, const size_t len);
+
+void tree_append_json(Con *con, const char *buf, const size_t len, char **errormsg);

include/util.h

@@ -164,3 +164,11 @@ void kill_nagbar(pid_t *nagbar_pid, bool wait_for_it);
  * if the number could be parsed.
  */
 bool parse_long(const char *str, long *out, int base);
+
+/**
+ * Slurp reads path in its entirety into buf, returning the length of the file
+ * or -1 if the file could not be read. buf is set to a buffer of appropriate
+ * size, or NULL if -1 is returned.
+ *
+ */
+ssize_t slurp(const char *path, char **buf);

src/commands.c

@@ -773,13 +773,25 @@ void cmd_append_layout(I3_CMD, const char *cpath) {
     /* Make sure we allow paths like '~/.i3/layout.json' */
     path = resolve_tilde(path);
 
-    json_content_t content = json_determine_content(path);
+    char *buf = NULL;
+    ssize_t len;
+    if ((len = slurp(path, &buf)) < 0) {
+        /* slurp already logged an error. */
+        goto out;
+    }
+
+    if (!json_validate(buf, len)) {
+        ELOG("Could not parse \"%s\" as JSON, not loading.\n", path);
+        yerror("Could not parse \"%s\" as JSON.", path);
+        goto out;
+    }
+
+    json_content_t content = json_determine_content(buf, len);
     LOG("JSON content = %d\n", content);
     if (content == JSON_CONTENT_UNKNOWN) {
         ELOG("Could not determine the contents of \"%s\", not loading.\n", path);
         yerror("Could not determine the contents of \"%s\".", path);
-        free(path);
+        goto out;
-        return;
     }
 
     Con *parent = focused;
@@ -795,7 +807,7 @@ void cmd_append_layout(I3_CMD, const char *cpath) {
     }
     DLOG("Appending to parent=%p instead of focused=%p\n", parent, focused);
     char *errormsg = NULL;
-    tree_append_json(parent, path, &errormsg);
+    tree_append_json(parent, buf, len, &errormsg);
     if (errormsg != NULL) {
         yerror(errormsg);
         free(errormsg);
@@ -820,8 +832,10 @@ void cmd_append_layout(I3_CMD, const char *cpath) {
     if (content == JSON_CONTENT_WORKSPACE)
         ipc_send_workspace_event("restored", parent, NULL);
 
-    free(path);
     cmd_output->needs_tree_render = true;
+out:
+    free(path);
+    free(buf);
 }
 
 /*

src/load_layout.c

@@ -538,36 +538,42 @@ static int json_determine_content_string(void *ctx, const unsigned char *val, si
     return 0;
 }
 
+/*
+ * Returns true if the provided JSON could be parsed by yajl.
+ *
+ */
+bool json_validate(const char *buf, const size_t len) {
+    bool valid = true;
+    yajl_handle hand = yajl_alloc(NULL, NULL, NULL);
+    /* Allowing comments allows for more user-friendly layout files. */
+    yajl_config(hand, yajl_allow_comments, true);
+    /* Allow multiple values, i.e. multiple nodes to attach */
+    yajl_config(hand, yajl_allow_multiple_values, true);
+
+    setlocale(LC_NUMERIC, "C");
+    if (yajl_parse(hand, (const unsigned char *)buf, len) != yajl_status_ok) {
+        unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len);
+        ELOG("JSON parsing error: %s\n", str);
+        yajl_free_error(hand, str);
+        valid = false;
+    }
+    setlocale(LC_NUMERIC, "");
+
+    yajl_complete_parse(hand);
+    yajl_free(hand);
+
+    return valid;
+}
+
 /* Parses the given JSON file until it encounters the first “type” property to
  * determine whether the file contains workspaces or regular containers, which
  * is important to know when deciding where (and how) to append the contents.
  * */
-json_content_t json_determine_content(const char *filename) {
+json_content_t json_determine_content(const char *buf, const size_t len) {
-    FILE *f;
-    if ((f = fopen(filename, "r")) == NULL) {
-        ELOG("Cannot open file \"%s\"\n", filename);
-        return JSON_CONTENT_UNKNOWN;
-    }
-    struct stat stbuf;
-    if (fstat(fileno(f), &stbuf) != 0) {
-        ELOG("Cannot fstat() \"%s\"\n", filename);
-        fclose(f);
-        return JSON_CONTENT_UNKNOWN;
-    }
-    char *buf = smalloc(stbuf.st_size);
-    int n = fread(buf, 1, stbuf.st_size, f);
-    if (n != stbuf.st_size) {
-        ELOG("File \"%s\" could not be read entirely, not loading.\n", filename);
-        fclose(f);
-        return JSON_CONTENT_UNKNOWN;
-    }
-    DLOG("read %d bytes\n", n);
     // We default to JSON_CONTENT_CON because it is legal to not include
     // “"type": "con"” in the JSON files for better readability.
     content_result = JSON_CONTENT_CON;
     content_level = 0;
-    yajl_gen g;
-    yajl_handle hand;
     static yajl_callbacks callbacks = {
         .yajl_string = json_determine_content_string,
         .yajl_map_key = json_key,
@@ -576,51 +582,27 @@ json_content_t json_determine_content(const char *filename) {
         .yajl_end_map = json_determine_content_shallower,
         .yajl_end_array = json_determine_content_shallower,
     };
-    g = yajl_gen_alloc(NULL);
+    yajl_handle hand = yajl_alloc(&callbacks, NULL, NULL);
-    hand = yajl_alloc(&callbacks, NULL, (void *)g);
     /* Allowing comments allows for more user-friendly layout files. */
     yajl_config(hand, yajl_allow_comments, true);
     /* Allow multiple values, i.e. multiple nodes to attach */
     yajl_config(hand, yajl_allow_multiple_values, true);
-    yajl_status stat;
     setlocale(LC_NUMERIC, "C");
-    stat = yajl_parse(hand, (const unsigned char *)buf, n);
+    const yajl_status stat = yajl_parse(hand, (const unsigned char *)buf, len);
     if (stat != yajl_status_ok && stat != yajl_status_client_canceled) {
-        unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, n);
+        unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len);
         ELOG("JSON parsing error: %s\n", str);
         yajl_free_error(hand, str);
     }
 
     setlocale(LC_NUMERIC, "");
     yajl_complete_parse(hand);
-
+    yajl_free(hand);
-    fclose(f);
 
     return content_result;
 }
 
-void tree_append_json(Con *con, const char *filename, char **errormsg) {
+void tree_append_json(Con *con, const char *buf, const size_t len, char **errormsg) {
-    FILE *f;
-    if ((f = fopen(filename, "r")) == NULL) {
-        ELOG("Cannot open file \"%s\"\n", filename);
-        return;
-    }
-    struct stat stbuf;
-    if (fstat(fileno(f), &stbuf) != 0) {
-        ELOG("Cannot fstat() \"%s\"\n", filename);
-        fclose(f);
-        return;
-    }
-    char *buf = smalloc(stbuf.st_size);
-    int n = fread(buf, 1, stbuf.st_size, f);
-    if (n != stbuf.st_size) {
-        ELOG("File \"%s\" could not be read entirely, not loading.\n", filename);
-        fclose(f);
-        return;
-    }
-    DLOG("read %d bytes\n", n);
-    yajl_gen g;
-    yajl_handle hand;
     static yajl_callbacks callbacks = {
         .yajl_boolean = json_bool,
         .yajl_integer = json_int,
@@ -631,13 +613,11 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) {
         .yajl_end_map = json_end_map,
         .yajl_end_array = json_end_array,
     };
-    g = yajl_gen_alloc(NULL);
+    yajl_handle hand = yajl_alloc(&callbacks, NULL, NULL);
-    hand = yajl_alloc(&callbacks, NULL, (void *)g);
     /* Allowing comments allows for more user-friendly layout files. */
     yajl_config(hand, yajl_allow_comments, true);
     /* Allow multiple values, i.e. multiple nodes to attach */
     yajl_config(hand, yajl_allow_multiple_values, true);
-    yajl_status stat;
     json_node = con;
     to_focus = NULL;
     incomplete = 0;
@@ -649,9 +629,9 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) {
     parsing_focus = false;
     parsing_marks = false;
     setlocale(LC_NUMERIC, "C");
-    stat = yajl_parse(hand, (const unsigned char *)buf, n);
+    const yajl_status stat = yajl_parse(hand, (const unsigned char *)buf, len);
     if (stat != yajl_status_ok) {
-        unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, n);
+        unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len);
         ELOG("JSON parsing error: %s\n", str);
         if (errormsg != NULL)
             *errormsg = sstrdup((const char *)str);
@@ -672,10 +652,8 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) {
     setlocale(LC_NUMERIC, "");
     yajl_complete_parse(hand);
     yajl_free(hand);
-    yajl_gen_free(g);
 
-    fclose(f);
+    if (to_focus) {
-    free(buf);
-    if (to_focus)
         con_focus(to_focus);
+    }
 }

src/tree.c

@@ -64,12 +64,19 @@ static Con *_create___i3(void) {
  *
  */
 bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) {
+    bool result = false;
     char *globbed = resolve_tilde(path);
 
     if (!path_exists(globbed)) {
         LOG("%s does not exist, not restoring tree\n", globbed);
-        free(globbed);
+        goto out;
-        return false;
+    }
+
+    char *buf = NULL;
+    ssize_t len;
+    if ((len = slurp(globbed, &buf)) < 0) {
+        /* slurp already logged an error. */
+        goto out;
     }
 
     /* TODO: refactor the following */
@@ -81,8 +88,7 @@ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) {
         geometry->height};
     focused = croot;
 
-    tree_append_json(focused, globbed, NULL);
+    tree_append_json(focused, buf, len, NULL);
-    free(globbed);
 
     DLOG("appended tree, using new root\n");
     croot = TAILQ_FIRST(&(croot->nodes_head));
@@ -104,8 +110,12 @@ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) {
     }
 
     restore_open_placeholder_windows(croot);
+    result = true;
 
-    return true;
+out:
+    free(globbed);
+    free(buf);
+    return result;
 }
 
 /*

src/util.c

@@ -475,3 +475,35 @@ bool parse_long(const char *str, long *out, int base) {
     *out = result;
     return true;
 }
+
+/*
+ * Slurp reads path in its entirety into buf, returning the length of the file
+ * or -1 if the file could not be read. buf is set to a buffer of appropriate
+ * size, or NULL if -1 is returned.
+ *
+ */
+ssize_t slurp(const char *path, char **buf) {
+    FILE *f;
+    if ((f = fopen(path, "r")) == NULL) {
+        ELOG("Cannot open file \"%s\": %s\n", path, strerror(errno));
+        return -1;
+    }
+    struct stat stbuf;
+    if (fstat(fileno(f), &stbuf) != 0) {
+        ELOG("Cannot fstat() \"%s\": %s\n", path, strerror(errno));
+        fclose(f);
+        return -1;
+    }
+    /* Allocate one extra NUL byte to make the buffer usable with C string
+     * functions. yajl doesn’t need this, but this makes slurp safer. */
+    *buf = scalloc(stbuf.st_size + 1, 1);
+    size_t n = fread(*buf, 1, stbuf.st_size, f);
+    fclose(f);
+    if ((ssize_t)n != stbuf.st_size) {
+        ELOG("File \"%s\" could not be read entirely: got %zd, want %zd\n", path, n, stbuf.st_size);
+        free(buf);
+        *buf = NULL;
+        return -1;
+    }
+    return (ssize_t)n;
+}