Rework url to filename

src/command/cmd_funcs.c

@@ -9141,7 +9141,6 @@ cmd_url_open(ProfWin* window, const char* const command, gchar** args)
     if (0 == g_strcmp0(scheme, "aesgcm")) {
         char* filename = unique_filename_from_url(url, files_get_data_path(DIR_DOWNLOADS));
         _url_aesgcm_method(window, cmd_template, url, filename);
-
         free(filename);
         goto out;
     }

src/common.c

@@ -602,56 +602,63 @@ _unique_filename(const char* filename)
     return unique;
 }
 
-gchar*
+bool
+_has_directory_suffix(const char* path)
+{
+    return (g_str_has_suffix(path, ".")
+            || g_str_has_suffix(path, "..")
+            || g_str_has_suffix(path, G_DIR_SEPARATOR_S));
+}
+
+char*
 _basename_from_url(const char* url)
 {
-    const char* default_name = "index.html";
+    const char* default_name = "index";
 
-    GFile* file = g_file_new_for_uri(url);
+    GFile* file = g_file_new_for_commandline_arg(url);
-    gchar* filename = g_file_get_basename(file);
+    char* basename = g_file_get_basename(file);
-    g_object_unref(file);
 
-    if (g_strcmp0(filename, ".") == 0
+    if (_has_directory_suffix(basename)) {
-        || g_strcmp0(filename, "..") == 0
+        g_free(basename);
-        || g_strcmp0(filename, G_DIR_SEPARATOR_S) == 0) {
+        basename = strdup(default_name);
-        g_free(filename);
-        return strdup(default_name);
     }
 
-    return filename;
+    g_object_unref(file);
+
+    return basename;
 }
 
 gchar*
 unique_filename_from_url(const char* url, const char* path)
 {
-    gchar* directory = NULL;
+    // Default to './' as path when none has been provided.
-    gchar* basename = NULL;
+    if (path == NULL) {
-    if (path != NULL) {
+        path = "./";
-        directory = g_path_get_dirname(path);
-        basename = g_path_get_basename(path);
     }
 
-    if (!directory) {
+    // Resolves paths such as './../.' for path.
-        // Explicitly use "./" as directory if no directory has been passed.
+    GFile* target = g_file_new_for_commandline_arg(path);
-        directory = "./";
+    gchar* filename = NULL;
-    }
 
-    if (!g_file_test(directory, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
+    if (_has_directory_suffix(path) || g_file_test(path, G_FILE_TEST_IS_DIR)) {
-        return NULL;
+        // The target should be used as a directory. Assume that the basename
+        // should be derived from the URL.
+        char* basename = _basename_from_url(url);
+        filename = g_build_filename(g_file_peek_path(target), basename, NULL);
+        g_free(basename);
+    } else {
+        // Just use the target as filename.
+        filename = g_build_filename(g_file_peek_path(target), NULL);
     }
 
-    if (g_file_test(path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
-        basename = _basename_from_url(url);
-    }
-
-    gchar* filename = g_build_filename(directory, basename, NULL);
-
     gchar* unique_filename = _unique_filename(filename);
-    if (!unique_filename) {
+    if (unique_filename == NULL) {
         g_free(filename);
         return NULL;
     }
 
+    g_object_unref(target);
     g_free(filename);
+
     return unique_filename;
 }

src/tools/aesgcm_download.c

@@ -85,11 +85,6 @@ aesgcm_file_get(void* userdata)
                                    "(%s).",
                                    https_url, g_strerror(errno));
         return NULL;
-    } else {
-        // TODO(wstrm): Maybe refactor this to use file handles so we do not
-        //              have to open a dummy file descriptor and then close it.
-        //              It's pretty ugly this way...
-        close(tmpfd);
     }
 
     FILE* outfh = fopen(aesgcm_dl->filename, "wb");
@@ -107,6 +102,7 @@ aesgcm_file_get(void* userdata)
     http_dl->worker = aesgcm_dl->worker;
     http_dl->url = strdup(https_url);
     http_dl->filename = strdup(tmpname);
+    http_dl->cmd_template = NULL;
 
     aesgcm_dl->http_dl = http_dl;
 
@@ -117,7 +113,7 @@ aesgcm_file_get(void* userdata)
         http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
                                    "Downloading '%s' failed: Unable to open "
                                    "temporary file at '%s' for reading (%s).",
-                                   aesgcm_dl->url, aesgcm_dl->filename,
+                                   aesgcm_dl->url, tmpname,
                                    g_strerror(errno));
         return NULL;
     }
@@ -130,6 +126,7 @@ aesgcm_file_get(void* userdata)
         cons_show_error(g_strerror(errno));
     }
 
+    close(tmpfd);
     remove(tmpname);
     g_free(tmpname);
 
@@ -149,7 +146,7 @@ aesgcm_file_get(void* userdata)
 
     if (aesgcm_dl->cmd_template != NULL) {
         gchar** argv = format_call_external_argv(aesgcm_dl->cmd_template,
-                                                 aesgcm_dl->url,
+                                                 aesgcm_dl->filename,
                                                  aesgcm_dl->filename);
 
         // TODO(wstrm): Log the error.
@@ -164,11 +161,11 @@ aesgcm_file_get(void* userdata)
         }
 
         g_strfreev(argv);
+        free(aesgcm_dl->cmd_template);
     }
 
     free(aesgcm_dl->filename);
     free(aesgcm_dl->url);
-    free(aesgcm_dl->cmd_template);
     free(aesgcm_dl);
 
     return NULL;

src/tools/http_download.c

@@ -205,6 +205,7 @@ http_file_get(void* userdata)
         }
 
         g_strfreev(argv);
+        free(download->cmd_template);
     }
 
     free(download->url);

tests/unittests/test_common.c

@@ -334,77 +334,123 @@ typedef struct
 {
     char* url;
     char* path;
-    char* filename;
+    char* target;
+    char* basename;
 } unique_filename_from_url_t;
 
 void
 unique_filename_from_url_td(void** state)
 {
-    enum table { num_tests = 11 };
+
+    enum table { num_tests = 15 };
+    char* pwd = g_get_current_dir();
 
     unique_filename_from_url_t tests[num_tests] = {
         (unique_filename_from_url_t){
             .url = "https://host.test/image.jpeg",
-            .path = "./",
+            .path = "./.",
-            .filename = "./image.jpeg",
+            .target = pwd,
+            .basename = "image.jpeg",
+        },
+        (unique_filename_from_url_t){
+            .url = "https://host.test/image.jpeg",
+            .path = NULL,
+            .target = pwd,
+            .basename = "image.jpeg",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/image.jpeg#somefragment",
             .path = "./",
-            .filename = "./image.jpeg",
+            .target = pwd,
+            .basename = "image.jpeg",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/image.jpeg?query=param",
             .path = "./",
-            .filename = "./image.jpeg",
+            .target = pwd,
+            .basename = "image.jpeg",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/image.jpeg?query=param&another=one",
             .path = "./",
-            .filename = "./image.jpeg",
+            .target = pwd,
+            .basename = "image.jpeg",
+        },
+        (unique_filename_from_url_t){
+            .url = "https://host.test/image.jpeg?query=param&another=one",
+            .path = "/tmp/",
+            .target = "/tmp/",
+            .basename = "image.jpeg",
+        },
+        (unique_filename_from_url_t){
+            .url = "https://host.test/image.jpeg?query=param&another=one",
+            .path = "/tmp/hopefully/this/file/does/not/exist",
+            .target = "/tmp/hopefully/this/file/does/not/",
+            .basename = "exist",
+        },
+        (unique_filename_from_url_t){
+            .url = "https://host.test/image.jpeg?query=param&another=one",
+            .path = "/tmp/hopefully/this/file/does/not/exist/",
+            .target = "/tmp/hopefully/this/file/does/not/exist/",
+            .basename = "image.jpeg",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/images/",
             .path = "./",
-            .filename = "./images",
+            .target = pwd,
+            .basename = "images",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/images/../../file",
             .path = "./",
-            .filename = "./file",
+            .target = pwd,
+            .basename = "file",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/images/../../file/..",
             .path = "./",
-            .filename = "./index.html",
+            .target = pwd,
+            .basename = "index",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/images/..//",
             .path = "./",
-            .filename = "./index.html",
+            .target = pwd,
+            .basename = "index",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test/",
             .path = "./",
-            .filename = "./index.html",
+            .target = pwd,
+            .basename = "index",
         },
         (unique_filename_from_url_t){
             .url = "https://host.test",
             .path = "./",
-            .filename = "./index.html",
+            .target = pwd,
+            .basename = "index",
         },
         (unique_filename_from_url_t){
             .url = "aesgcm://host.test",
             .path = "./",
-            .filename = "./index.html",
+            .target = pwd,
+            .basename = "index",
         },
     };
 
-    char* filename;
+    char* got_filename;
+    char* exp_filename;
     for (int i = 0; i < num_tests; i++) {
-        filename = unique_filename_from_url(tests[i].url, tests[i].path);
+        got_filename = unique_filename_from_url(tests[i].url, tests[i].path);
-        assert_string_equal(filename, tests[i].filename);
+        exp_filename = g_build_filename(tests[i].target, tests[i].basename, NULL);
+
+        assert_string_equal(got_filename, exp_filename);
+
+        free(got_filename);
+        free(exp_filename);
     }
+
+    g_free(pwd);
 }
 
 gboolean