fix: Fix file autocompletion bugs and restore cycling

Fix several issues in cmd_ac_complete_filepath:
* Prevent a segfault when input is empty.
* Fix a double free where acstring was managed by both auto_gchar and GArray.
* Fix a memory leak when reassigning inpcp during quote stripping.
* Restore the ability to cycle through files on repeated TAB presses by
  caching the last input and skipping updates if the input is already a
  known completion.
* Preserve user input style (e.g. ~, ./) in autocompletion strings to
  ensure matches are correctly displayed and filtered.

Bug got introduced when "cleaning" code with new compiler flags and
sanitizers: aec8e48268.
Add unit tests so this doesn't happen again.

Before that commit we didn't use the static variable but used
autocomplete_update instead. Now we avoid redundat updates and preserve
the state across tab presses. We should look at this again later.

Fix https://github.com/profanity-im/profanity/issues/2098

Makefile.am

@@ -164,6 +164,7 @@ unittest_sources = \
 	tests/unittests/command/test_cmd_pgp.c tests/unittests/command/test_cmd_pgp.h \
 	tests/unittests/command/test_cmd_join.c tests/unittests/command/test_cmd_join.h \
 	tests/unittests/command/test_cmd_roster.c tests/unittests/command/test_cmd_roster.h \
+	tests/unittests/command/test_cmd_ac.c tests/unittests/command/test_cmd_ac.h \
 	tests/unittests/command/test_cmd_disconnect.c tests/unittests/command/test_cmd_disconnect.h \
 	tests/unittests/plugins/test_callbacks.c tests/unittests/plugins/test_callbacks.h \
 	tests/unittests/plugins/test_plugins_disco.c tests/unittests/plugins/test_plugins_disco.h \

meson.build

@@ -628,6 +628,7 @@ if get_option('tests')
       'tests/unittests/command/test_cmd_pgp.c',
       'tests/unittests/command/test_cmd_join.c',
       'tests/unittests/command/test_cmd_roster.c',
+      'tests/unittests/command/test_cmd_ac.c',
       'tests/unittests/command/test_cmd_disconnect.c',
       'tests/unittests/plugins/test_callbacks.c',
       'tests/unittests/plugins/test_plugins_disco.c',

src/command/cmd_ac.c

@@ -297,6 +297,8 @@ static Autocomplete vcard_set_param_ac;
 static Autocomplete vcard_togglable_param_ac;
 static Autocomplete vcard_address_type_ac;
 
+static char* last_filepath_input = NULL;
+
 static Autocomplete* all_acs[] = {
     &commands_ac,
     &who_room_ac,
@@ -1652,6 +1654,8 @@ cmd_ac_uninit(void)
     autocomplete_free(plugins_unload_ac);
     autocomplete_free(plugins_reload_ac);
     autocomplete_free(script_show_ac);
+    g_free(last_filepath_input);
+    last_filepath_input = NULL;
     g_hash_table_destroy(ac_funcs);
     ac_funcs = NULL;
 }
@@ -1684,6 +1688,7 @@ cmd_ac_complete_filepath(const char* const input, char* const startstr, gboolean
             *last_quote = '\0';
         }
         gchar* unquoted = g_strdup(inpcp + 1);
+        g_free(inpcp);
         inpcp = unquoted;
     }
 
@@ -1695,13 +1700,27 @@ cmd_ac_complete_filepath(const char* const input, char* const startstr, gboolean
     auto_gchar gchar* foofile = g_path_get_basename(expanded);
     auto_gchar gchar* directory = g_path_get_dirname(expanded);
 
+    // If the input is already a known completion, don't update to allow cycling
+    if (last_filepath_input && (g_strcmp0(inpcp, last_filepath_input) == 0 || autocomplete_contains(filepath_ac, inpcp))) {
+        return autocomplete_param_with_ac(input, startstr, filepath_ac, TRUE, previous);
+    }
+
     // If the input ends with a slash, the basename will be "." or "/"
     // In that case, we are looking for all files in that directory
     gboolean find_all = FALSE;
-    if (inpcp[strlen(inpcp) - 1] == '/') {
+    size_t inpcp_len = strlen(inpcp);
+    if (inpcp_len == 0 || inpcp[inpcp_len - 1] == '/') {
         find_all = TRUE;
     }
 
+    char* last_slash = strrchr(inpcp, '/');
+    auto_gchar gchar* user_dir_part = NULL;
+    if (last_slash) {
+        user_dir_part = g_strndup(inpcp, last_slash - inpcp + 1);
+    } else {
+        user_dir_part = g_strdup("");
+    }
+
     GArray* files = g_array_new(TRUE, FALSE, sizeof(gchar*));
     g_array_set_clear_func(files, (GDestroyNotify)_filepath_item_free);
 
@@ -1724,12 +1743,7 @@ cmd_ac_complete_filepath(const char* const input, char* const startstr, gboolean
                 continue;
             }
 
-            auto_gchar gchar* acstring = NULL;
-            if (strcmp(directory, "/") == 0) {
-                acstring = g_strdup_printf("/%s", dir->d_name);
-            } else {
-                acstring = g_strdup_printf("%s/%s", directory, dir->d_name);
-            }
+            gchar* acstring = g_strdup_printf("%s%s", user_dir_part, dir->d_name);
 
             if (acstring) {
                 g_array_append_val(files, acstring);
@@ -1741,6 +1755,9 @@ cmd_ac_complete_filepath(const char* const input, char* const startstr, gboolean
     autocomplete_update(filepath_ac, (char**)files->data);
     g_array_free(files, TRUE);
 
+    g_free(last_filepath_input);
+    last_filepath_input = g_strdup(inpcp);
+
     return autocomplete_param_with_ac(input, startstr, filepath_ac, TRUE, previous);
 }
 

tests/unittests/command/test_cmd_ac.c

@@ -0,0 +1,94 @@
+#include <stdlib.h>
+#include <glib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include "prof_cmocka.h"
+#include "command/cmd_ac.h"
+
+static void
+_create_test_file(const char* path)
+{
+    FILE* f = fopen(path, "w");
+    if (f) {
+        fprintf(f, "test");
+        fclose(f);
+    }
+}
+
+void
+cmd_ac_complete_filepath__segfaults_when_empty(void** state)
+{
+    char* result = cmd_ac_complete_filepath("/sendfile ", "/sendfile", FALSE);
+
+    free(result);
+}
+
+void
+cmd_ac_complete_filepath__finds_files_in_current_dir(void** state)
+{
+    mkdir("test_dir", 0755);
+    _create_test_file("test_dir/file1.txt");
+    _create_test_file("test_dir/file2.txt");
+
+    char* result = cmd_ac_complete_filepath("/sendfile test_dir/file", "/sendfile", FALSE);
+    assert_non_null(result);
+    assert_string_equal(result, "/sendfile test_dir/file1.txt");
+    free(result);
+
+    remove("test_dir/file1.txt");
+    remove("test_dir/file2.txt");
+    rmdir("test_dir");
+}
+
+void
+cmd_ac_complete_filepath__finds_files_with_dot_slash(void** state)
+{
+    mkdir("test_dir", 0755);
+    _create_test_file("test_dir/file1.txt");
+
+    char* result = cmd_ac_complete_filepath("/sendfile ./test_dir/file", "/sendfile", FALSE);
+    assert_non_null(result);
+    assert_string_equal(result, "/sendfile ./test_dir/file1.txt");
+    free(result);
+
+    remove("test_dir/file1.txt");
+    rmdir("test_dir");
+}
+
+void
+cmd_ac_complete_filepath__cycles_through_files(void** state)
+{
+    mkdir("test_dir", 0755);
+    _create_test_file("test_dir/file1.txt");
+    _create_test_file("test_dir/file2.txt");
+
+    // 1st TAB
+    char* res1 = cmd_ac_complete_filepath("/sendfile test_dir/file", "/sendfile", FALSE);
+    assert_non_null(res1);
+    assert_string_equal(res1, "/sendfile test_dir/file1.txt");
+
+    // 2nd TAB
+    char* res2 = cmd_ac_complete_filepath(res1, "/sendfile", FALSE);
+    assert_non_null(res2);
+    assert_string_equal(res2, "/sendfile test_dir/file2.txt");
+
+    // 3rd TAB
+    char* res3 = cmd_ac_complete_filepath(res2, "/sendfile", FALSE);
+    assert_non_null(res3);
+    assert_string_equal(res3, "/sendfile test_dir/file1.txt");
+
+    // SHIFT-TAB
+    char* res4 = cmd_ac_complete_filepath(res3, "/sendfile", TRUE);
+    assert_non_null(res4);
+    assert_string_equal(res4, "/sendfile test_dir/file2.txt");
+
+    free(res1);
+    free(res2);
+    free(res3);
+    free(res4);
+
+    remove("test_dir/file1.txt");
+    remove("test_dir/file2.txt");
+    rmdir("test_dir");
+}

tests/unittests/command/test_cmd_ac.h

@@ -0,0 +1,9 @@
+#ifndef TESTS_TEST_CMD_AC_H
+#define TESTS_TEST_CMD_AC_H
+
+void cmd_ac_complete_filepath__segfaults_when_empty(void** state);
+void cmd_ac_complete_filepath__finds_files_in_current_dir(void** state);
+void cmd_ac_complete_filepath__finds_files_with_dot_slash(void** state);
+void cmd_ac_complete_filepath__cycles_through_files(void** state);
+
+#endif

tests/unittests/unittests.c

@@ -30,6 +30,7 @@
 #include "command/test_cmd_bookmark.h"
 #include "command/test_cmd_join.h"
 #include "xmpp/test_muc.h"
+#include "command/test_cmd_ac.h"
 #include "command/test_cmd_roster.h"
 #include "command/test_cmd_disconnect.h"
 #include "xmpp/test_form.h"
@@ -96,6 +97,10 @@ main(int argc, char* argv[])
         cmocka_unit_test(unique_filename_from_url__tests__table_driven),
         cmocka_unit_test(string_to_verbosity__returns__correct_values),
         cmocka_unit_test(get_expanded_path__returns__expanded),
+        cmocka_unit_test_setup_teardown(cmd_ac_complete_filepath__segfaults_when_empty, load_preferences, close_preferences),
+        cmocka_unit_test_setup_teardown(cmd_ac_complete_filepath__finds_files_in_current_dir, load_preferences, close_preferences),
+        cmocka_unit_test_setup_teardown(cmd_ac_complete_filepath__finds_files_with_dot_slash, load_preferences, close_preferences),
+        cmocka_unit_test_setup_teardown(cmd_ac_complete_filepath__cycles_through_files, load_preferences, close_preferences),
         cmocka_unit_test(strtoi_range__returns__true_for_valid_input),
         cmocka_unit_test(strtoi_range__returns__false_for_out_of_range),
         cmocka_unit_test(strtoi_range__returns__false_for_invalid_input),