summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/opener.c65
-rw-r--r--tests/test_opener.c219
2 files changed, 237 insertions, 47 deletions
diff --git a/src/opener.c b/src/opener.c
index f9afb1b..822e332 100644
--- a/src/opener.c
+++ b/src/opener.c
@@ -51,9 +51,6 @@ opener_get_progs(Opener *o) {
return o ? o->p_progs : NULL;
}
-/* Expand %f → shell-escaped path. Returns a newly-allocated argv vector
- * (NULL-terminated). Caller frees with g_strfreev. */
-
/* Replace all occurrences of c_old with c_new in c_str. Caller frees. */
static char *
_str_replace(const char *c_str, const char *c_old, const char *c_new) {
@@ -74,25 +71,51 @@ _str_replace(const char *c_str, const char *c_old, const char *c_new) {
return g_string_free(p_out, FALSE);
}
-static char **
-_expand_command(const char *c_cmd, GFile *p_file) {
+/* Expand %f → the file path inside an already-parsed argv.
+ *
+ * The command template is parsed with g_shell_parse_argv FIRST, so quotes,
+ * escapes, and option flags are resolved correctly; %f is then substituted
+ * into the parsed argv elements. This guarantees %f is always exactly one
+ * argv value no matter what spaces or shell metacharacters the path contains
+ * (substitution happens after shell parsing, never before).
+ *
+ * Returns a newly-allocated, NULL-terminated argv vector; caller frees with
+ * g_strfreev. On a parse error returns NULL and sets p_err — never launches
+ * malformed argv. Non-static (with the `_` prefix retained to mark it as
+ * non-public) so tests/test_opener.c can verify the argv shape directly via
+ * an extern prototype, without polluting the public opener.h. */
+char **
+_expand_command(const char *c_cmd, GFile *p_file, GError **p_err) {
+ g_return_val_if_fail(c_cmd != NULL, NULL);
+ g_return_val_if_fail(G_IS_FILE(p_file), NULL);
char *c_path = g_file_get_path(p_file);
- /* Simple tokenisation: split on spaces, replace %f with path. */
- char **parts = g_strsplit(c_cmd, " ", -1);
- GPtrArray *argv = g_ptr_array_new();
- for (guint i = 0; parts[i]; i++) {
- if (g_str_equal(parts[i], "%f"))
- g_ptr_array_add(argv, g_strdup(c_path));
- else if (strstr(parts[i], "%f")) {
- char *r = _str_replace(parts[i], "%f", c_path);
- g_ptr_array_add(argv, r ? r : g_strdup(parts[i]));
- } else
- g_ptr_array_add(argv, g_strdup(parts[i]));
+ if (c_path == NULL) {
+ g_set_error(p_err, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+ "opener: file has no local path");
+ return NULL;
+ }
+ /* Parse the raw template first: resolves quotes/escapes/options. */
+ char **argv = NULL;
+ if (!g_shell_parse_argv(c_cmd, NULL, &argv, p_err)) {
+ g_free(c_path);
+ return NULL;
+ }
+ /* Substitute %f into each parsed element so the path is always one argv
+ * value, regardless of spaces or shell metacharacters it contains. */
+ for (guint i = 0; argv[i]; i++) {
+ if (g_str_equal(argv[i], "%f")) {
+ g_free(argv[i]);
+ argv[i] = g_strdup(c_path);
+ } else if (strstr(argv[i], "%f")) {
+ char *r = _str_replace(argv[i], "%f", c_path);
+ if (r) {
+ g_free(argv[i]);
+ argv[i] = r;
+ }
+ }
}
- g_ptr_array_add(argv, NULL);
- g_strfreev(parts);
g_free(c_path);
- return (char **)g_ptr_array_free(argv, FALSE);
+ return argv;
}
gboolean
@@ -101,7 +124,9 @@ opener_launch(Opener *o, GFile *p_file, const OpenerProg *p_prog,
(void)o;
g_return_val_if_fail(G_IS_FILE(p_file), FALSE);
g_return_val_if_fail(p_prog, FALSE);
- char **argv = _expand_command(p_prog->c_command, p_file);
+ char **argv = _expand_command(p_prog->c_command, p_file, p_err);
+ if (argv == NULL)
+ return FALSE;
GSubprocess *p_sub = g_subprocess_newv((const char *const *)argv,
G_SUBPROCESS_FLAGS_NONE, p_err);
g_strfreev(argv);
diff --git a/tests/test_opener.c b/tests/test_opener.c
index a364bdb..6e3cd1d 100644
--- a/tests/test_opener.c
+++ b/tests/test_opener.c
@@ -1,8 +1,18 @@
-/* test_opener.c — %f expansion + launch with true/false. */
+/* test_opener.c — %f expansion + launch with true/false.
+ *
+ * The argv-shape regression test (test_expand_weird_one_argv) calls
+ * _expand_command directly. That function is non-static in opener.c (with the
+ * `_` prefix retained to mark it as non-public) exactly so this test can
+ * inspect the parsed argv; it is declared here via an extern prototype rather
+ * than exposed in the public opener.h, to avoid public-header pollution. */
#include "opener.h"
#include <gio/gio.h>
#include <glib.h>
+/* Non-static internal from opener.c (test-only accessor); see top-of-file
+ * comment. */
+extern char **_expand_command(const char *c_cmd, GFile *p_file, GError **p_err);
+
static GFile *
make_tmp_file(void) {
GError *e = NULL;
@@ -17,6 +27,49 @@ make_tmp_file(void) {
return f;
}
+/* Build a temp dir containing a file whose name has spaces, $, and backticks
+ * — the same "weird" pattern the original test_weird_filename used. */
+static GFile *
+make_weird_file(void) {
+ GError *e = NULL;
+ char *d = g_dir_make_tmp("ggaze-opener-XXXXXX", &e);
+ g_assert_no_error(e);
+ char *p = g_build_filename(d, "file with spaces $HOME `whoami`.jpg", NULL);
+ GFile *f = g_file_new_for_path(p);
+ g_file_replace_contents(f, "x", 1, NULL, FALSE,
+ G_FILE_CREATE_REPLACE_DESTINATION, NULL, NULL, NULL);
+ g_free(p);
+ g_free(d);
+ return f;
+}
+
+/* Delete p_file and its temp parent directory (recursing any leftover
+ * children so the dir removal succeeds). */
+static void
+cleanup_file(GFile *f) {
+ if (!f)
+ return;
+ GFile *d = g_file_get_parent(f);
+ g_file_delete(f, NULL, NULL);
+ if (d) {
+ GFileEnumerator *en = g_file_enumerate_children(
+ d, "standard::name", G_FILE_QUERY_INFO_NONE, NULL, NULL);
+ if (en) {
+ GFileInfo *i;
+ while ((i = g_file_enumerator_next_file(en, NULL, NULL))) {
+ GFile *c = g_file_get_child(d, g_file_info_get_name(i));
+ g_file_delete(c, NULL, NULL);
+ g_object_unref(c);
+ g_object_unref(i);
+ }
+ g_object_unref(en);
+ }
+ g_file_delete(d, NULL, NULL);
+ g_object_unref(d);
+ }
+ g_object_unref(f);
+}
+
static void
test_launch_true(void) {
GFile *f = make_tmp_file();
@@ -26,7 +79,7 @@ test_launch_true(void) {
g_assert_true(opener_launch(o, f, &prog, &e));
g_assert_no_error(e);
opener_delete(o);
- g_object_unref(f);
+ cleanup_file(f);
}
static void
@@ -38,41 +91,144 @@ test_launch_false(void) {
g_assert_true(opener_launch(o, f, &prog, &e));
g_assert_no_error(e);
opener_delete(o);
- g_object_unref(f);
+ cleanup_file(f);
}
static void
test_weird_filename(void) {
- GError *e = NULL;
- char *d = g_dir_make_tmp("ggaze-opener-XXXXXX", &e);
- g_assert_no_error(e);
- char *p = g_build_filename(d, "file with spaces $HOME `whoami`.jpg", NULL);
- GFile *f = g_file_new_for_path(p);
- g_file_replace_contents(f, "x", 1, NULL, FALSE,
- G_FILE_CREATE_REPLACE_DESTINATION, NULL, NULL, NULL);
+ GFile *f = make_weird_file();
Opener *o = opener_new();
OpenerProg prog = {"true", "true %f"};
+ GError *e = NULL;
g_assert_true(opener_launch(o, f, &prog, &e));
g_assert_no_error(e);
opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_quoted(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"sh -c 'true'", "sh -c 'true'"};
+ GError *e = NULL;
+ g_assert_true(opener_launch(o, f, &prog, &e));
+ g_assert_no_error(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_quoted_with_pctf(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"sh -c 'true %f'", "sh -c 'true %f'"};
+ GError *e = NULL;
+ g_assert_true(opener_launch(o, f, &prog, &e));
+ g_assert_no_error(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_escaped_spaces(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"escaped", "true a\\ b"};
+ GError *e = NULL;
+ g_assert_true(opener_launch(o, f, &prog, &e));
+ g_assert_no_error(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_option_flags(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"opts", "true -verbose %f"};
+ GError *e = NULL;
+ g_assert_true(opener_launch(o, f, &prog, &e));
+ g_assert_no_error(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_weird_filename_quoted(void) {
+ GFile *f = make_weird_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"sh -c 'true %f'", "sh -c 'true %f'"};
+ GError *e = NULL;
+ /* %f is substituted inside the already-parsed single argv element
+ * `true %f` → `true /weird/path`, passed as ONE arg to `sh -c`. */
+ g_assert_true(opener_launch(o, f, &prog, &e));
+ g_assert_no_error(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_malformed(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"bad", "sh -c 'echo hello"};
+ GError *e = NULL;
+ g_assert_false(opener_launch(o, f, &prog, &e));
+ g_assert_nonnull(e);
+ g_error_free(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+static void
+test_empty(void) {
+ GFile *f = make_tmp_file();
+ Opener *o = opener_new();
+ OpenerProg prog = {"empty", ""};
+ GError *e = NULL;
+ g_assert_false(opener_launch(o, f, &prog, &e));
+ g_assert_nonnull(e);
+ g_error_free(e);
+ opener_delete(o);
+ cleanup_file(f);
+}
+
+/* The key regression test: with a weird filename (spaces + shell
+ * metacharacters) and an UNQUOTED %f template, the path must land in the
+ * parsed argv as exactly ONE element — not split on the path's spaces. `true`
+ * would ignore extra args, so a launch-only check cannot prove this; we
+ * inspect the argv shape directly via _expand_command. */
+static void
+test_expand_weird_one_argv(void) {
+ GFile *f = make_weird_file();
+ GError *e = NULL;
+ char **argv = _expand_command("true %f", f, &e);
+ g_assert_no_error(e);
+ g_assert_nonnull(argv);
+ /* argv = {"true", "<weird path>", NULL} — exactly three slots. */
+ g_assert_cmpstr(argv[0], ==, "true");
+ g_assert_nonnull(argv[1]);
+ g_assert_cmpstr(argv[2], ==, NULL);
+ /* The path element must equal the file's path verbatim, spaces and all. */
+ char *c_path = g_file_get_path(f);
+ g_assert_cmpstr(argv[1], ==, c_path);
+ g_free(c_path);
+ g_strfreev(argv);
+ cleanup_file(f);
+}
+
+/* A non-local GFile (no path) must be rejected with a GError rather than
+ * producing an argv with an embedded NULL that silently drops trailing args. */
+static void
+test_expand_no_local_path(void) {
+ GFile *f = g_file_new_for_uri("https://example.com/img.jpg");
+ GError *e = NULL;
+ char **argv = _expand_command("editor %f", f, &e);
+ g_assert_null(argv);
+ g_assert_nonnull(e);
+ g_error_free(e);
g_object_unref(f);
- g_free(p);
- GFile *dd = g_file_new_for_path(d);
- GFileEnumerator *en = g_file_enumerate_children(
- dd, "standard::name", G_FILE_QUERY_INFO_NONE, NULL, NULL);
- if (en) {
- GFileInfo *i;
- while ((i = g_file_enumerator_next_file(en, NULL, NULL))) {
- GFile *c = g_file_get_child(dd, g_file_info_get_name(i));
- g_file_delete(c, NULL, NULL);
- g_object_unref(c);
- g_object_unref(i);
- }
- g_object_unref(en);
- }
- g_file_delete(dd, NULL, NULL);
- g_object_unref(dd);
- g_free(d);
}
int
@@ -81,5 +237,14 @@ main(int argc, char **argv) {
g_test_add_func("/opener/true", test_launch_true);
g_test_add_func("/opener/false", test_launch_false);
g_test_add_func("/opener/weird_filename", test_weird_filename);
+ g_test_add_func("/opener/quoted", test_quoted);
+ g_test_add_func("/opener/quoted_with_pctf", test_quoted_with_pctf);
+ g_test_add_func("/opener/escaped_spaces", test_escaped_spaces);
+ g_test_add_func("/opener/option_flags", test_option_flags);
+ g_test_add_func("/opener/weird_filename_quoted", test_weird_filename_quoted);
+ g_test_add_func("/opener/malformed", test_malformed);
+ g_test_add_func("/opener/empty", test_empty);
+ g_test_add_func("/opener/expand_weird_one_argv", test_expand_weird_one_argv);
+ g_test_add_func("/opener/expand_no_local_path", test_expand_no_local_path);
return g_test_run();
} \ No newline at end of file
generated by cgit v1.2.3 (git 2.25.1) at 2026-08-07 03:35:40 +0000