diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 147eca4d..5b1427fe 100644 --- a/src/command/cmd_funcs.c +++ b/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; } diff --git a/src/common.c b/src/common.c index 66e344c5..10be280a 100644 --- a/src/common.c +++ b/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); - gchar* filename = g_file_get_basename(file); - g_object_unref(file); + GFile* file = g_file_new_for_commandline_arg(url); + char* basename = g_file_get_basename(file); - if (g_strcmp0(filename, ".") == 0 - || g_strcmp0(filename, "..") == 0 - || g_strcmp0(filename, G_DIR_SEPARATOR_S) == 0) { - g_free(filename); - return strdup(default_name); + if (_has_directory_suffix(basename)) { + g_free(basename); + basename = 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; - gchar* basename = NULL; - if (path != NULL) { - directory = g_path_get_dirname(path); - basename = g_path_get_basename(path); + // Default to './' as path when none has been provided. + if (path == NULL) { + path = "./"; } - if (!directory) { - // Explicitly use "./" as directory if no directory has been passed. - directory = "./"; - } + // Resolves paths such as './../.' for path. + GFile* target = g_file_new_for_commandline_arg(path); + gchar* filename = NULL; - if (!g_file_test(directory, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) { - return NULL; + if (_has_directory_suffix(path) || g_file_test(path, G_FILE_TEST_IS_DIR)) { + // 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; } diff --git a/src/tools/aesgcm_download.c b/src/tools/aesgcm_download.c index d75cabe3..f8d2db9f 100644 --- a/src/tools/aesgcm_download.c +++ b/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; diff --git a/src/tools/http_download.c b/src/tools/http_download.c index ef7e2906..397ff4b8 100644 --- a/src/tools/http_download.c +++ b/src/tools/http_download.c @@ -205,6 +205,7 @@ http_file_get(void* userdata) } g_strfreev(argv); + free(download->cmd_template); } free(download->url); diff --git a/tests/unittests/test_common.c b/tests/unittests/test_common.c index 462676cc..2dee8722 100644 --- a/tests/unittests/test_common.c +++ b/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 = "./", - .filename = "./image.jpeg", + .path = "./.", + .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); - assert_string_equal(filename, tests[i].filename); + got_filename = unique_filename_from_url(tests[i].url, tests[i].path); + 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