Merge pull request #305 from matthiasclasen/name-validation

More validation error reporting improvements

app/flatpak-builtins-build-bundle.c

@@ -924,8 +924,8 @@ flatpak_builtin_build_bundle (int argc, char **argv, GCancellable *cancellable,
   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);
+  if (!flatpak_is_valid_branch (branch, &my_error))
+    return flatpak_fail (error, _("'%s' is not a valid branch name: %s"), branch, &my_error);
 
   if (opt_runtime)
     full_branch = flatpak_build_runtime_ref (name, branch, opt_arch);

app/flatpak-builtins-build-export.c

@@ -499,6 +499,7 @@ flatpak_builtin_build_export (int argc, char **argv, GCancellable *cancellable,
   g_autoptr(OstreeMutableTree) files_mtree = NULL;
   g_autoptr(OstreeMutableTree) export_mtree = NULL;
   g_autoptr(GKeyFile) metakey = NULL;
+  g_autoptr(GError) my_error = NULL;
   gsize metadata_size;
   g_autofree char *subject = NULL;
   g_autofree char *body = NULL;
@@ -526,9 +527,9 @@ flatpak_builtin_build_export (int argc, char **argv, GCancellable *cancellable,
   else
     branch = "master";
 
-  if (!flatpak_is_valid_branch (branch))
+  if (!flatpak_is_valid_branch (branch, &my_error))
     {
-      flatpak_fail (error, _("'%s' is not a valid branch name"), branch);
+      flatpak_fail (error, _("'%s' is not a valid branch name: %s"), branch, my_error->message);
       goto out;
     }
 

app/flatpak-builtins-build-init.c

@@ -100,8 +100,8 @@ flatpak_builtin_build_init (int argc, char **argv, GCancellable *cancellable, GE
   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);
+  if (!flatpak_is_valid_branch (branch, &my_error))
+    return flatpak_fail (error, _("'%s' is not a valid branch name: %s"), branch, my_error->message);
 
   runtime_ref = flatpak_build_untyped_ref (runtime, branch, opt_arch);
   sdk_ref = flatpak_build_untyped_ref (sdk, branch, opt_arch);

app/flatpak-builtins-build-sign.c

@@ -83,8 +83,8 @@ flatpak_builtin_build_sign (int argc, char **argv, GCancellable *cancellable, GE
   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);
+  if (!flatpak_is_valid_branch (branch, &my_error))
+    return flatpak_fail (error, _("'%s' is not a valid branch name: %s"), branch, my_error->message);
 
   if (opt_gpg_key_ids == NULL)
     return flatpak_fail (error, _("No gpg key ids specified"));

common/flatpak-dir.c

@@ -4311,9 +4311,9 @@ find_matching_refs (GHashTable *refs,
       return NULL;
     }
 
-  if (opt_branch && !flatpak_is_valid_branch (opt_branch))
+  if (opt_branch && !flatpak_is_valid_branch (opt_branch, &local_error))
     {
-      flatpak_fail (error, "'%s' is not a valid branch name", opt_branch);
+      flatpak_fail (error, "'%s' is not a valid branch name: %s", opt_branch, local_error->message);
       return NULL;
     }
 

common/flatpak-utils.c

@@ -377,8 +377,9 @@ is_valid_name_character (gint c)
     (c >= '0' && c <= '9');
 }
 
-/** flatpak_is_name:
+/** flatpak_is_valid_name:
  * @string: The string to check
+ * @error: Return location for an error
  *
  * Checks if @string is a valid application name.
  *
@@ -416,8 +417,19 @@ flatpak_is_valid_name (const char *string,
   ret = FALSE;
 
   len = strlen (string);
-  if (G_UNLIKELY (len == 0 || len > 255))
-    goto out;
+  if (G_UNLIKELY (len == 0))
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Name can't be empty");
+      goto out;
+    }
+
+  if (G_UNLIKELY (len > 255))
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Name can't be longer than 255 characters");
+      goto out;
+    }
 
   end = string + len;
 
@@ -515,6 +527,7 @@ is_valid_branch_character (gint c)
 
 /** flatpak_is_valid_branch:
  * @string: The string to check
+ * @error: return location for an error
  *
  * Checks if @string is a valid branch name.
  *
@@ -528,7 +541,8 @@ is_valid_branch_character (gint c)
  * Since: 2.26
  */
 gboolean
-flatpak_is_valid_branch (const char *string)
+flatpak_is_valid_branch (const char *string,
+                         GError **error)
 {
   guint len;
   gboolean ret;
@@ -541,19 +555,31 @@ flatpak_is_valid_branch (const char *string)
 
   len = strlen (string);
   if (G_UNLIKELY (len == 0))
-    goto out;
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Branch can't be empty");
+      goto out;
+    }
 
   end = string + len;
 
   s = string;
   if (G_UNLIKELY (!is_valid_initial_branch_character (*s)))
-    goto out;
+    {
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                   "Branch can't start with %c", *s);
+      goto out;
+    }
 
   s += 1;
   while (s != end)
     {
       if (G_UNLIKELY (!is_valid_branch_character (*s)))
-        goto out;
+        {
+          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                       "Branch can't contain %c", *s);
+          goto out;
+        }
       s += 1;
     }
 
@@ -595,9 +621,9 @@ flatpak_decompose_ref (const char *full_ref,
       return NULL;
     }
 
-  if (!flatpak_is_valid_branch (parts[3]))
+  if (!flatpak_is_valid_branch (parts[3], &local_error))
     {
-      flatpak_fail (error, "Invalid branch %s", parts[3]);
+      flatpak_fail (error, "Invalid branch %s: %s", parts[3], local_error->message);
       return NULL;
     }
 
@@ -642,8 +668,8 @@ flatpak_split_partial_ref_arg (char *partial_ref,
   branch = slash + 1;
   if (strlen (branch) > 0)
     {
-      if (!flatpak_is_valid_branch (branch))
-        return flatpak_fail (error, "Invalid branch %s", branch);
+      if (!flatpak_is_valid_branch (branch, &local_error))
+        return flatpak_fail (error, "Invalid branch %s: %s", branch, local_error->message);
     }
   else
     branch = NULL;
@@ -674,9 +700,9 @@ flatpak_compose_ref (gboolean    app,
       return NULL;
     }
 
-  if (branch && !flatpak_is_valid_branch (branch))
+  if (branch && !flatpak_is_valid_branch (branch, &local_error))
     {
-      flatpak_fail (error, "'%s' is not a valid branch name", branch);
+      flatpak_fail (error, "'%s' is not a valid branch name: %s", branch, local_error->message);
       return NULL;
     }
 

common/flatpak-utils.h

@@ -80,7 +80,8 @@ gboolean flatpak_has_name_prefix (const char *string,
                                   const char *name);
 gboolean flatpak_is_valid_name (const char *string,
                                 GError **error);
-gboolean flatpak_is_valid_branch (const char *string);
+gboolean flatpak_is_valid_branch (const char *string,
+                                  GError **error);
 
 char **flatpak_decompose_ref (const char *ref,
                               GError    **error);