[epiphany/mcatanzaro/web-app-id] Fix web applications that use non-Latin characters in the name

[Marge Bot <marge-bot src gnome org>]

commit f060a06f4e1e6f8c87c115fe6f53a6e45a5ec174
Author: Michael Catanzaro <mcatanzaro redhat com>
Date:   Wed Nov 24 13:24:18 2021 -0600

    Fix web applications that use non-Latin characters in the name
    
    Users can input whatever they want for the name of the web app, as long
    as it is valid UTF-8. Since a72d21d758a3ac8c3dce8d4373941dfbf6f517aa we
    have passed this along straight to GApplication without sanitizing it to
    ensure it can safely be used in a GApplication ID. If the user decides
    to use any character in the app name besides Latin alphanumeric
    characters, we are doomed. This has never worked properly, but
    previously all we saw were criticals from GApplication as it dropped our
    bogus app ID. But since WebKitGTK 2.32, WebKit actually refuses to start
    a sandboxed subprocess without a valid app ID, as it should.
    
    The complication here is that we cannot simply decide to create better
    app IDs for existing web apps: the app ID must exactly match the desktop
    file, and we want it to match the profile directory name too. So let's
    assume that non-Latin web apps were previously broken, hope that users
    delete any such broken web apps rather than leak them on disk, and
    handwave away migration issues by altering the app ID only if it cannot
    safely be used in a GApplication. Existing non-broken web apps continue
    to use the same app IDs as before. Newly-created web apps with only
    Latin characters follow the old format for IDs, while newly-created web
    apps with non-approved characters will follow the new format. Old broken
    web apps remain broken forever, until the user decides to delete them.
    
    The old app ID format is:
    `org.gnome.Epiphany.WebApp-<normalized-name>-<checksum>`
    
    The new format is:
    `org.gnome.Epiphany.WebApp-<checksum>`
    
    The ideal format would be:
    `org.gnome.Epiphany.WebApp_<checksum>`
    
    because hyphens in app IDs are deprecated, but let's not take on that
    battle today.
    
    Note that in this commit message, "app ID" refers to the GApplication
    ID, which is actually confusingly different from the EphyWebApplication
    "app ID."
    
    This commit also replaces the concept of "program name" with the
    GApplication ID. It was very confusing to have "name" and "program name"
    be two completely different things. Now we can have "ID" and
    "GApplication ID" be two different things instead. Yay.
    
    Fixes #1627
    
    See also: #1626
    
    Part-of: <https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/1032>

 lib/ephy-file-helpers.c  |   2 +-
 lib/ephy-settings.c      |   2 +-
 lib/ephy-web-app-utils.c | 161 +++++++++++++++++++++++++++++++----------------
 lib/ephy-web-app-utils.h |   2 +-
 src/ephy-shell.c         |   5 +-
 5 files changed, 114 insertions(+), 58 deletions(-)
---
diff --git a/lib/ephy-file-helpers.c b/lib/ephy-file-helpers.c
index 8bc2a494d..a2e3ab645 100644
--- a/lib/ephy-file-helpers.c
+++ b/lib/ephy-file-helpers.c
@@ -376,7 +376,7 @@ ephy_file_helpers_init (const char            *profile_dir,
 
     app_file = g_build_filename (profile_dir, ".app", NULL);
     if (g_file_test (app_file, G_FILE_TEST_EXISTS)) {
-      const char *app_name = ephy_web_application_get_program_name_from_profile_directory 
(profile_dir_global);
+      const char *app_name = ephy_web_application_get_gapplication_id_from_profile_directory 
(profile_dir_global);
       cache_dir = g_build_filename (g_get_user_cache_dir (), app_name, NULL);
       config_dir = g_build_filename (g_get_user_config_dir (), app_name, NULL);
       profile_dir_type = EPHY_PROFILE_DIR_WEB_APP;
diff --git a/lib/ephy-settings.c b/lib/ephy-settings.c
index 7e86340e0..bcd96515a 100644
--- a/lib/ephy-settings.c
+++ b/lib/ephy-settings.c
@@ -58,7 +58,7 @@ ephy_settings_init (void)
                                     g_object_unref);
 
   if (ephy_profile_dir_is_web_application ()) {
-    const char *web_app_name = ephy_web_application_get_program_name_from_profile_directory 
(profile_directory);
+    const char *web_app_name = ephy_web_application_get_gapplication_id_from_profile_directory 
(profile_directory);
     base_path = g_build_path ("/", "/org/gnome/epiphany/web-apps/", web_app_name, NULL);
   } else {
     base_path = g_strdup ("/org/gnome/epiphany/");
diff --git a/lib/ephy-web-app-utils.c b/lib/ephy-web-app-utils.c
index 1ba24a64f..9c8f26c51 100644
--- a/lib/ephy-web-app-utils.c
+++ b/lib/ephy-web-app-utils.c
@@ -33,20 +33,31 @@
 #include <string.h>
 #include <fcntl.h>
 
-/* Web Apps are installed in the default config dir of the user.
- * Every app has its own profile directory. To create a web app
- * an id needs to be generated using the given name as
- * <normalized-name>-checksum.
+/* Web apps are installed in the default data dir of the user. Every
+ * app has its own profile directory. To create a web app, an ID needs
+ * to be generated using the given name as <normalized-name>-checksum.
+ * The ID is used to uniquely identify the app.
  *
- * The id is used to uniquely identify the app.
- *  - Program name: org.gnome.Epiphany.WebApp-<id>
- *  - Profile directory: <program-name>
- *  - Desktop file: <profile-dir>/<program-name>.desktop
+ *  - Name: the user-visible pretty app name
+ *  - Normalized name: lowercase name
+ *  - Checksum: SHA-1 of name
+ *  - ID: <normalized-name>-<checksum>
+ *  - GApplication ID: see below
+ *  - Profile directory: <gapplication-id>
+ *  - Desktop file: <profile-dir>/<gapplication-id>.desktop
+ *
+ * Note that our ID and GApplication ID are different. Yes, this is confusing.
+ *
+ * For GApplication ID, there are two cases:
+ *
+ *  - If <id> is safe to use in a D-Bus identifier,
+ *    then: GApplication ID is org.gnome.Epiphany.WebApp-<id>
+ *  - otherwise: GAppliation ID is org.gnome.Epiphany.WebApp-<checksum>
  *
  * System web applications have a profile dir without a desktop file.
  */
 
-#define EPHY_WEB_APP_PROGRAM_NAME_PREFIX "org.gnome.Epiphany.WebApp-"
+#define EPHY_WEB_APP_GAPPLICATION_ID_PREFIX "org.gnome.Epiphany.WebApp-"
 
 char *
 ephy_web_application_get_app_id_from_name (const char *name)
@@ -84,34 +95,90 @@ get_encoded_path (const char *path)
   return encoded;
 }
 
+static const char *
+get_app_id_from_gapplication_id (const char *name)
+{
+  if (!g_str_has_prefix (name, EPHY_WEB_APP_GAPPLICATION_ID_PREFIX)) {
+    g_warning ("GApplication ID %s does not begin with required prefix %s", name, 
EPHY_WEB_APP_GAPPLICATION_ID_PREFIX);
+    return NULL;
+  }
+
+  return name + strlen (EPHY_WEB_APP_GAPPLICATION_ID_PREFIX);
+}
+
 static char *
-get_app_profile_directory_name (const char *id)
+get_gapplication_id_from_id (const char *id)
 {
-  char *profile_dir;
-  char *encoded;
+  g_auto (GStrv) split = NULL;
+  g_autofree char *gapplication_id = NULL;
+
+  /* Ideally we would convert hyphens to underscores here, because
+   * hyphens are not very friendly to D-Bus. However, changing this
+   * would require a new profile migration, because the GApplication ID
+   * must exactly match the desktop file basename.
+   *
+   * If we're willing to do another migration in the future, then we
+   * should drop this path and always return just the prefix plus hash,
+   * using underscores rather than hyphens.
+   */
+  gapplication_id = g_strconcat (EPHY_WEB_APP_GAPPLICATION_ID_PREFIX, id, NULL);
+  if (g_application_id_is_valid (gapplication_id))
+    return g_steal_pointer (&gapplication_id);
+
+  /* Split ID into: <normalized-name>-<checksum> */
+  split = g_strsplit (id, "-", -1);
+  if (g_strv_length (split) != 2) {
+    g_warning ("Web app ID %s is broken: must have two hyphens", id);
+    return NULL;
+  }
 
-  profile_dir = g_strconcat (EPHY_WEB_APP_PROGRAM_NAME_PREFIX, id, NULL);
-  encoded = get_encoded_path (profile_dir);
-  g_free (profile_dir);
+  /* We'll simply omit the <normalized-name> from the app ID, in order
+   * to avoid problematic characters. Ideally we would use an underscore
+   * here too, rather than a hyphen, but let's be consistent with
+   * existing web apps.
+   */
+  g_clear_pointer (&gapplication_id, g_free);
+  gapplication_id = g_strconcat (EPHY_WEB_APP_GAPPLICATION_ID_PREFIX, split[1], NULL);
 
-  return encoded;
+  if (!g_application_id_is_valid (gapplication_id)) {
+    g_warning ("Web app ID %s is broken: derived GApplication ID %s is not a valid app ID (is the final 
component alphanumeric?)", id, gapplication_id);
+    return NULL;
+  }
+
+  return g_steal_pointer (&gapplication_id);
 }
 
 static char *
-get_app_desktop_filename (const char *id)
+get_app_profile_directory_name (const char *id)
 {
-  char *filename;
-  char *encoded;
+  g_autofree char *gapplication_id = NULL;
 
-  filename = g_strconcat (EPHY_WEB_APP_PROGRAM_NAME_PREFIX, id, ".desktop", NULL);
-  encoded = get_encoded_path (filename);
-  g_free (filename);
+  gapplication_id = get_gapplication_id_from_id (id);
+  if (!gapplication_id)
+    g_error ("Failed to get GApplication ID from app ID %s", id);
 
-  return encoded;
+  return get_encoded_path (gapplication_id);
+}
+
+static char *
+get_app_desktop_filename (const char *id)
+{
+  g_autofree char *gapplication_id = NULL;
+  g_autofree char *filename = NULL;
+
+  /* Warning: the GApplication ID must exactly match the desktop file's
+   * basename. Don't overthink this or stuff will break, e.g. GNotification.
+   */
+  gapplication_id = get_gapplication_id_from_id (id);
+  if (!gapplication_id)
+    g_error ("Failed to get GApplication ID from app ID %s", id);
+
+  filename = g_strconcat (gapplication_id, ".desktop", NULL);
+  return get_encoded_path (filename);
 }
 
 const char *
-ephy_web_application_get_program_name_from_profile_directory (const char *profile_dir)
+ephy_web_application_get_gapplication_id_from_profile_directory (const char *profile_dir)
 {
   const char *name;
 
@@ -128,32 +195,21 @@ ephy_web_application_get_program_name_from_profile_directory (const char *profil
   if (g_str_has_prefix (name, "app-"))
     name += strlen ("app-");
 
-  if (!g_str_has_prefix (name, EPHY_WEB_APP_PROGRAM_NAME_PREFIX)) {
-    g_warning ("Profile directory %s does not begin with required web app prefix %s", profile_dir, 
EPHY_WEB_APP_PROGRAM_NAME_PREFIX);
+  if (!g_str_has_prefix (name, EPHY_WEB_APP_GAPPLICATION_ID_PREFIX)) {
+    g_warning ("Profile directory %s does not begin with required web app prefix %s", profile_dir, 
EPHY_WEB_APP_GAPPLICATION_ID_PREFIX);
     return NULL;
   }
 
   return name;
 }
 
-static const char *
-get_app_id_from_program_name (const char *name)
-{
-  if (!g_str_has_prefix (name, EPHY_WEB_APP_PROGRAM_NAME_PREFIX)) {
-    g_warning ("Program name %s does not begin with required prefix %s", name, 
EPHY_WEB_APP_PROGRAM_NAME_PREFIX);
-    return NULL;
-  }
-
-  return name + strlen (EPHY_WEB_APP_PROGRAM_NAME_PREFIX);
-}
-
 static const char *
 get_app_id_from_profile_directory (const char *profile_dir)
 {
-  const char *program_name;
+  const char *gapplication_id;
 
-  program_name = ephy_web_application_get_program_name_from_profile_directory (profile_dir);
-  return program_name ? get_app_id_from_program_name (program_name) : NULL;
+  gapplication_id = ephy_web_application_get_gapplication_id_from_profile_directory (profile_dir);
+  return gapplication_id ? get_app_id_from_gapplication_id (gapplication_id) : NULL;
 }
 
 static char *
@@ -322,7 +378,7 @@ create_desktop_file (const char *id,
     g_free (path);
   }
 
-  wm_class = g_strconcat (EPHY_WEB_APP_PROGRAM_NAME_PREFIX, id, NULL);
+  wm_class = g_strconcat (EPHY_WEB_APP_GAPPLICATION_ID_PREFIX, id, NULL);
   g_key_file_set_value (file, "Desktop Entry", "StartupWMClass", wm_class);
   g_free (wm_class);
 
@@ -478,7 +534,7 @@ ephy_web_application_ensure_for_app_info (GAppInfo *app_info)
 void
 ephy_web_application_setup_from_profile_directory (const char *profile_directory)
 {
-  const char *program_name;
+  const char *gapplication_id;
   const char *id;
   char *desktop_basename;
   char *desktop_filename;
@@ -486,24 +542,23 @@ ephy_web_application_setup_from_profile_directory (const char *profile_directory
 
   g_assert (profile_directory != NULL);
 
-  program_name = ephy_web_application_get_program_name_from_profile_directory (profile_directory);
-  if (!program_name)
-    exit (1);
+  gapplication_id = ephy_web_application_get_gapplication_id_from_profile_directory (profile_directory);
+  if (!gapplication_id)
+    g_error ("Failed to get GApplication ID from profile directory %s", profile_directory);
 
-  g_set_prgname (program_name);
+  /* FIXME: is this actually necessary? Probably not? */
+  g_set_prgname (gapplication_id);
 
-  id = get_app_id_from_program_name (program_name);
+  id = get_app_id_from_gapplication_id (gapplication_id);
   if (!id)
-    exit (1);
+    g_error ("Failed to get app ID from GApplication ID %s", gapplication_id);
 
   /* Get display name from desktop file */
   desktop_basename = get_app_desktop_filename (id);
   desktop_filename = g_build_filename (profile_directory, desktop_basename, NULL);
   desktop_info = g_desktop_app_info_new_from_filename (desktop_filename);
-  if (!desktop_info) {
-    g_warning ("Required desktop file not present at %s", desktop_filename);
-    exit (1);
-  }
+  if (!desktop_info)
+    g_error ("Required desktop file not present at %s", desktop_filename);
   g_set_application_name (g_app_info_get_name (G_APP_INFO (desktop_info)));
 
   g_free (desktop_basename);
@@ -621,7 +676,7 @@ ephy_web_application_get_application_list_internal (gboolean only_legacy)
 
     name = g_file_info_get_name (info);
     if ((only_legacy && g_str_has_prefix (name, "app-")) ||
-        (!only_legacy && g_str_has_prefix (name, EPHY_WEB_APP_PROGRAM_NAME_PREFIX))) {
+        (!only_legacy && g_str_has_prefix (name, EPHY_WEB_APP_GAPPLICATION_ID_PREFIX))) {
       EphyWebApplication *app;
       char *profile_dir;
 
diff --git a/lib/ephy-web-app-utils.h b/lib/ephy-web-app-utils.h
index 3d68263e6..fb9546ffd 100644
--- a/lib/ephy-web-app-utils.h
+++ b/lib/ephy-web-app-utils.h
@@ -45,7 +45,7 @@ typedef enum {
 
 char               *ephy_web_application_get_app_id_from_name (const char *name);
 
-const char         *ephy_web_application_get_program_name_from_profile_directory (const char *profile_dir);
+const char         *ephy_web_application_get_gapplication_id_from_profile_directory (const char 
*profile_dir);
 
 char               *ephy_web_application_create (const char *id, const char *address, const char *name, 
GdkPixbuf *icon, EphyWebApplicationOptions options);
 
diff --git a/src/ephy-shell.c b/src/ephy-shell.c
index ef4d84392..0ace095aa 100644
--- a/src/ephy-shell.c
+++ b/src/ephy-shell.c
@@ -1293,9 +1293,10 @@ _ephy_shell_create_instance (EphyEmbedShellMode mode)
 
   if (mode == EPHY_EMBED_SHELL_MODE_APPLICATION) {
     const char *profile_dir = ephy_profile_dir ();
-    const char *web_id = ephy_web_application_get_program_name_from_profile_directory (profile_dir);
 
-    id = web_id;
+    id = ephy_web_application_get_gapplication_id_from_profile_directory (profile_dir);
+    if (id == NULL)
+      g_error ("Cannot start web app instance: %s is not a valid profile directory", profile_dir);
   } else {
     id = APPLICATION_ID;
   }