From b2b281e4857a4d05a07e90cd3c66887845ea4557 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 12 Sep 2016 23:25:54 -0400 Subject: [PATCH] Make flatpak_is_valid_name set a GError Return detailed information about the problem with the name in the GError. Update all callers. --- app/flatpak-builtins-build-bundle.c | 5 +-- app/flatpak-builtins-build-init.c | 13 +++---- app/flatpak-builtins-build-sign.c | 5 +-- app/flatpak-builtins-override.c | 4 +-- common/flatpak-dir.c | 5 +-- common/flatpak-utils.c | 55 +++++++++++++++++++++-------- common/flatpak-utils.h | 3 +- document-portal/xdp-fuse.c | 2 +- document-portal/xdp-main.c | 10 +++--- 9 files changed, 68 insertions(+), 34 deletions(-) diff --git a/app/flatpak-builtins-build-bundle.c b/app/flatpak-builtins-build-bundle.c index 78c6b8d73..639e5fb3b 100644 --- a/app/flatpak-builtins-build-bundle.c +++ b/app/flatpak-builtins-build-bundle.c @@ -888,6 +888,7 @@ flatpak_builtin_build_bundle (int argc, char **argv, GCancellable *cancellable, g_autoptr(GFile) file = NULL; g_autoptr(GFile) repofile = NULL; g_autoptr(OstreeRepo) repo = NULL; + g_autoptr(GError) my_error = NULL; const char *location; const char *filename; const char *name; @@ -920,8 +921,8 @@ flatpak_builtin_build_bundle (int argc, char **argv, GCancellable *cancellable, file = g_file_new_for_commandline_arg (filename); - if (!flatpak_is_valid_name (name)) - return flatpak_fail (error, _("'%s' is not a valid name"), name); + if (!flatpak_is_valid_name (name, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid name: %s"), name, my_error->message); if (!flatpak_is_valid_branch (branch)) return flatpak_fail (error, _("'%s' is not a valid branch name"), branch); diff --git a/app/flatpak-builtins-build-init.c b/app/flatpak-builtins-build-init.c index 742c4c9a5..ca0115a1c 100644 --- a/app/flatpak-builtins-build-init.c +++ b/app/flatpak-builtins-build-init.c @@ -64,6 +64,7 @@ flatpak_builtin_build_init (int argc, char **argv, GCancellable *cancellable, GE g_autoptr(GFile) var_run_dir = NULL; g_autoptr(GFile) metadata_file = NULL; g_autoptr(GString) metadata_contents = NULL; + g_autoptr(GError) my_error = NULL; const char *app_id; const char *directory; const char *sdk; @@ -90,14 +91,14 @@ flatpak_builtin_build_init (int argc, char **argv, GCancellable *cancellable, GE if (argc >= 6) branch = argv[5]; - if (!flatpak_is_valid_name (app_id)) - return flatpak_fail (error, _("'%s' is not a valid application name"), app_id); + if (!flatpak_is_valid_name (app_id, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid application name: %s"), app_id, my_error->message); - if (!flatpak_is_valid_name (runtime)) - return flatpak_fail (error, _("'%s' is not a valid runtime name"), runtime); + if (!flatpak_is_valid_name (runtime, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid runtime name: %s"), runtime, my_error->message); - if (!flatpak_is_valid_name (sdk)) - return flatpak_fail (error, _("'%s' is not a valid sdk name"), sdk); + if (!flatpak_is_valid_name (sdk, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid sdk name: %s"), sdk, my_error->message); if (!flatpak_is_valid_branch (branch)) return flatpak_fail (error, _("'%s' is not a valid branch name"), branch); diff --git a/app/flatpak-builtins-build-sign.c b/app/flatpak-builtins-build-sign.c index da3c0a6d0..c61aa51e2 100644 --- a/app/flatpak-builtins-build-sign.c +++ b/app/flatpak-builtins-build-sign.c @@ -52,6 +52,7 @@ flatpak_builtin_build_sign (int argc, char **argv, GCancellable *cancellable, GE g_autoptr(GOptionContext) context = NULL; g_autoptr(GFile) repofile = NULL; g_autoptr(OstreeRepo) repo = NULL; + g_autoptr(GError) my_error = NULL; const char *location; const char *branch; const char *id; @@ -79,8 +80,8 @@ flatpak_builtin_build_sign (int argc, char **argv, GCancellable *cancellable, GE else branch = "master"; - if (!flatpak_is_valid_name (id)) - return flatpak_fail (error, _("'%s' is not a valid name"), id); + if (!flatpak_is_valid_name (id, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid name: %s"), id, my_error->message); if (!flatpak_is_valid_branch (branch)) return flatpak_fail (error, _("'%s' is not a valid branch name"), branch); diff --git a/app/flatpak-builtins-override.c b/app/flatpak-builtins-override.c index 1139dec98..846be016d 100644 --- a/app/flatpak-builtins-override.c +++ b/app/flatpak-builtins-override.c @@ -67,8 +67,8 @@ flatpak_builtin_override (int argc, char **argv, GCancellable *cancellable, GErr app = argv[1]; - if (!flatpak_is_valid_name (app)) - return flatpak_fail (error, _("'%s' is not a valid application name"), app); + if (!flatpak_is_valid_name (app, &my_error)) + return flatpak_fail (error, _("'%s' is not a valid application name: %s"), app, my_error->message); metakey = flatpak_load_override_keyfile (app, flatpak_dir_is_user (dir), &my_error); if (metakey == NULL) diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index e30493cde..c0d93bd84 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -4300,13 +4300,14 @@ find_matching_refs (GHashTable *refs, const char *opt_arches[] = {opt_arch, NULL}; GHashTableIter hash_iter; gpointer key; + g_autoptr(GError) local_error = NULL; if (opt_arch != NULL) arches = opt_arches; - if (opt_name && !flatpak_is_valid_name (opt_name)) + if (opt_name && !flatpak_is_valid_name (opt_name, &local_error)) { - flatpak_fail (error, "'%s' is not a valid name", opt_name); + flatpak_fail (error, "'%s' is not a valid name: %s", opt_name, local_error->message); return NULL; } diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c index 5784295f0..c589661d5 100644 --- a/common/flatpak-utils.c +++ b/common/flatpak-utils.c @@ -402,7 +402,8 @@ is_valid_name_character (gint c) * Since: 2.26 */ gboolean -flatpak_is_valid_name (const char *string) +flatpak_is_valid_name (const char *string, + GError **error) { guint len; gboolean ret; @@ -422,10 +423,17 @@ flatpak_is_valid_name (const char *string) s = string; if (G_UNLIKELY (*s == '.')) - /* can't start with a . */ - goto out; + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Name can't start with a period"); + goto out; + } else if (G_UNLIKELY (!is_valid_initial_name_character (*s))) - goto out; + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Name can't start with %c", *s); + goto out; + } s += 1; dot_count = 0; @@ -434,19 +442,35 @@ flatpak_is_valid_name (const char *string) if (*s == '.') { s += 1; - if (G_UNLIKELY (s == end || !is_valid_initial_name_character (*s))) - goto out; + if (G_UNLIKELY (s == end)) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Name can't end with a period"); + goto out; + } + if (!is_valid_initial_name_character (*s)) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Name segment can't start with %c", *s); + goto out; + } dot_count++; } else if (G_UNLIKELY (!is_valid_name_character (*s))) { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Name can't contain %c", *s); goto out; } s += 1; } if (G_UNLIKELY (dot_count < 2)) - goto out; + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Names must contain at least 2 periods"); + goto out; + } ret = TRUE; @@ -470,7 +494,6 @@ flatpak_has_name_prefix (const char *string, !is_valid_name_character (*rest); } - static gboolean is_valid_initial_branch_character (gint c) { @@ -545,6 +568,7 @@ flatpak_decompose_ref (const char *full_ref, GError **error) { g_auto(GStrv) parts = NULL; + g_autoptr(GError) local_error = NULL; parts = g_strsplit (full_ref, "/", 0); if (g_strv_length (parts) != 4) @@ -559,9 +583,9 @@ flatpak_decompose_ref (const char *full_ref, return NULL; } - if (!flatpak_is_valid_name (parts[1])) + if (!flatpak_is_valid_name (parts[1], &local_error)) { - flatpak_fail (error, "Invalid name %s", parts[1]); + flatpak_fail (error, "Invalid name %s: %s", parts[1], local_error->message); return NULL; } @@ -589,6 +613,7 @@ flatpak_split_partial_ref_arg (char *partial_ref, char *slash; char *arch = NULL; char *branch = NULL; + g_autoptr(GError) local_error = NULL; if (partial_ref == NULL) return TRUE; @@ -597,8 +622,8 @@ flatpak_split_partial_ref_arg (char *partial_ref, if (slash != NULL) *slash = 0; - if (!flatpak_is_valid_name (partial_ref)) - return flatpak_fail (error, "Invalid name %s", partial_ref); + if (!flatpak_is_valid_name (partial_ref, &local_error)) + return flatpak_fail (error, "Invalid name %s: %s", partial_ref, local_error->message); if (slash == NULL) goto out; @@ -641,9 +666,11 @@ flatpak_compose_ref (gboolean app, const char *arch, GError **error) { - if (!flatpak_is_valid_name (name)) + g_autoptr(GError) local_error = NULL; + + if (!flatpak_is_valid_name (name, &local_error)) { - flatpak_fail (error, "'%s' is not a valid name", name); + flatpak_fail (error, "'%s' is not a valid name: %s", name, local_error->message); return NULL; } diff --git a/common/flatpak-utils.h b/common/flatpak-utils.h index d598120fb..a06153be3 100644 --- a/common/flatpak-utils.h +++ b/common/flatpak-utils.h @@ -78,7 +78,8 @@ gboolean flatpak_summary_lookup_ref (GVariant *summary, gboolean flatpak_has_name_prefix (const char *string, const char *name); -gboolean flatpak_is_valid_name (const char *string); +gboolean flatpak_is_valid_name (const char *string, + GError **error); gboolean flatpak_is_valid_branch (const char *string); char **flatpak_decompose_ref (const char *ref, diff --git a/document-portal/xdp-fuse.c b/document-portal/xdp-fuse.c index 7abf2f163..aa7d7324c 100644 --- a/document-portal/xdp-fuse.c +++ b/document-portal/xdp-fuse.c @@ -976,7 +976,7 @@ xdp_fuse_lookup (fuse_req_t req, case XDP_INODE_BY_APP: /* This lazily creates the app dir */ - if (flatpak_is_valid_name (name)) + if (flatpak_is_valid_name (name, NULL)) child_inode = xdp_inode_get_dir (name, NULL, NULL); break; diff --git a/document-portal/xdp-main.c b/document-portal/xdp-main.c index 5e346b88d..d8db1604c 100644 --- a/document-portal/xdp-main.c +++ b/document-portal/xdp-main.c @@ -112,6 +112,7 @@ portal_grant_permissions (GDBusMethodInvocation *invocation, GVariant *parameters, const char *app_id) { + g_autoptr(GError) my_error = NULL; const char *target_app_id; const char *id; g_autofree const char **permissions = NULL; @@ -133,11 +134,11 @@ portal_grant_permissions (GDBusMethodInvocation *invocation, return; } - if (!flatpak_is_valid_name (target_app_id)) + if (!flatpak_is_valid_name (target_app_id, &my_error)) { g_dbus_method_invocation_return_error (invocation, FLATPAK_PORTAL_ERROR, FLATPAK_PORTAL_ERROR_INVALID_ARGUMENT, - "Invalid app name: %s", target_app_id); + "'%s' is not a valid app name: %s", target_app_id, my_error->message); return; } @@ -173,6 +174,7 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation, g_autofree const char **permissions = NULL; g_autoptr(FlatpakDbEntry) entry = NULL; + g_autoptr(GError) my_error = NULL; XdpPermissionFlags perms; g_variant_get (parameters, "(&s&s^a&s)", &id, &target_app_id, &permissions); @@ -189,11 +191,11 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation, return; } - if (!flatpak_is_valid_name (target_app_id)) + if (!flatpak_is_valid_name (target_app_id, &my_error)) { g_dbus_method_invocation_return_error (invocation, FLATPAK_PORTAL_ERROR, FLATPAK_PORTAL_ERROR_INVALID_ARGUMENT, - "Invalid app name: %s", target_app_id); + "'%s' is not a valid app name: %s", target_app_id, my_error->message); return; }