run: Sort environment before serializing it into bwrap arguments

This has no practical effect (assuming environment variables are unique),
but it makes it easier to find an environment variable of interest
in a very long bwrap command-line.

Signed-off-by: Simon McVittie <smcv@collabora.com>

6 files changed, 153 insertions, 0 deletions

diff --git a/common/flatpak-bwrap-private.h b/common/flatpak-bwrap-private.h
index 6c31b789..801ef74b 100644
--- a/common/flatpak-bwrap-private.h
+++ b/common/flatpak-bwrap-private.h
@@ -75,6 +75,7 @@ void          flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
                                           const char   *type,
                                           const char   *src,
                                           const char   *dest);
+void          flatpak_bwrap_sort_envp (FlatpakBwrap *bwrap);
 void          flatpak_bwrap_envp_to_args (FlatpakBwrap *bwrap);
 gboolean      flatpak_bwrap_bundle_args (FlatpakBwrap *bwrap,
                                          int           start,
diff --git a/common/flatpak-bwrap.c b/common/flatpak-bwrap.c
index 6d8dc5b5..c55d6d7e 100644
--- a/common/flatpak-bwrap.c
+++ b/common/flatpak-bwrap.c
@@ -287,6 +287,21 @@ flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
 }
 
 /*
+ * Sort bwrap->envp. This has no practical effect, but it's easier to
+ * see what is going on in a large environment block if the variables
+ * are sorted.
+ */
+void
+flatpak_bwrap_sort_envp (FlatpakBwrap *bwrap)
+{
+  if (bwrap->envp != NULL)
+    {
+      qsort (bwrap->envp, g_strv_length (bwrap->envp), sizeof (char *),
+             flatpak_envp_cmp);
+    }
+}
+
+/*
  * Convert bwrap->envp into a series of --setenv arguments for bwrap(1),
  * assumed to be applied to an empty environment. Reset envp to be an
  * empty environment.
diff --git a/common/flatpak-run.c b/common/flatpak-run.c
index 13da04b1..b64310f5 100644
--- a/common/flatpak-run.c
+++ b/common/flatpak-run.c
@@ -3403,6 +3403,7 @@ regenerate_ld_cache (GPtrArray    *base_argv_array,
                           "--dev", "/dev",
                           "--bind", flatpak_file_get_path_cached (ld_so_dir), "/run/ld-so-cache-dir",
                           NULL);
+  flatpak_bwrap_sort_envp (bwrap);
   flatpak_bwrap_envp_to_args (bwrap);
 
   if (!flatpak_bwrap_bundle_args (bwrap, 1, -1, FALSE, error))
@@ -4016,6 +4017,7 @@ flatpak_run_app (FlatpakDecomposed *app_ref,
       command = default_command;
     }
 
+  flatpak_bwrap_sort_envp (bwrap);
   flatpak_bwrap_envp_to_args (bwrap);
 
   if (!flatpak_bwrap_bundle_args (bwrap, 1, -1, FALSE, error))
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index 97b77196..eb32c28c 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -907,6 +907,9 @@ null_safe_g_ptr_array_unref (gpointer data)
   g_clear_pointer (&data, g_ptr_array_unref);
 }
 
+int flatpak_envp_cmp (const void *p1,
+                      const void *p2);
+
 #define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
 
 #endif /* __FLATPAK_UTILS_H__ */
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 6e5bd6b3..3b8ab521 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -8611,4 +8611,71 @@ flatpak_dconf_path_is_similar (const char *path1,
   return (path1[i1] == '\0');
 }
 
+/**
+ * flatpak_envp_cmp:
+ * @p1: a `const char * const *`
+ * @p2: a `const char * const *`
+ *
+ * Compare two environment variables, given as pointers to pointers
+ * to the actual `KEY=value` string.
+ *
+ * In particular this is suitable for sorting a #GStrv using `qsort`.
+ *
+ * Returns: negative, 0 or positive if `*p1` compares before, equal to
+ *  or after `*p2`
+ */
+int
+flatpak_envp_cmp (const void *p1,
+                  const void *p2)
+{
+  const char * const * s1 = p1;
+  const char * const * s2 = p2;
+  size_t l1 = strlen (*s1);
+  size_t l2 = strlen (*s2);
+  size_t min;
+  const char *tmp;
+  int ret;
+
+  tmp = strchr (*s1, '=');
+
+  if (tmp != NULL)
+    l1 = tmp - *s1;
+
+  tmp = strchr (*s2, '=');
+
+  if (tmp != NULL)
+    l2 = tmp - *s2;
 
+  min = MIN (l1, l2);
+  ret = strncmp (*s1, *s2, min);
+
+  /* If they differ before the first '=' (if any) in either s1 or s2,
+   * then they are certainly different */
+  if (ret != 0)
+    return ret;
+
+  ret = strcmp (*s1, *s2);
+
+  /* If they do not differ at all, then they are equal */
+  if (ret == 0)
+    return ret;
+
+  /* FOO < FOO=..., and FOO < FOOBAR */
+  if ((*s1)[min] == '\0')
+    return -1;
+
+  /* FOO=... > FOO, and FOOBAR > FOO */
+  if ((*s2)[min] == '\0')
+    return 1;
+
+  /* FOO= < FOOBAR */
+  if ((*s1)[min] == '=' && (*s2)[min] != '=')
+    return -1;
+
+  /* FOOBAR > FOO= */
+  if ((*s2)[min] == '=' && (*s1)[min] != '=')
+    return 1;
+
+  /* Fall back to plain string comparison */
+  return ret;
+}
diff --git a/tests/testcommon.c b/tests/testcommon.c
index 1d217c07..8e33eb25 100644
--- a/tests/testcommon.c
+++ b/tests/testcommon.c
@@ -1557,6 +1557,70 @@ test_dconf_paths (void)
     }
 }
 
+static void
+test_envp_cmp (void)
+{
+  static const char * const unsorted[] =
+  {
+    "SAME_NAME=2",
+    "EARLY_NAME=a",
+    "SAME_NAME=222",
+    "Z_LATE_NAME=b",
+    "SUFFIX_ADDED=23",
+    "SAME_NAME=1",
+    "SAME_NAME=",
+    "SUFFIX=42",
+    "SAME_NAME=3",
+    "SAME_NAME",
+  };
+  static const char * const sorted[] =
+  {
+    "EARLY_NAME=a",
+    "SAME_NAME",
+    "SAME_NAME=",
+    "SAME_NAME=1",
+    "SAME_NAME=2",
+    "SAME_NAME=222",
+    "SAME_NAME=3",
+    "SUFFIX=42",
+    "SUFFIX_ADDED=23",
+    "Z_LATE_NAME=b",
+  };
+  const char **sort_this = NULL;
+  gsize i, j;
+
+  G_STATIC_ASSERT (G_N_ELEMENTS (sorted) == G_N_ELEMENTS (unsorted));
+
+  for (i = 0; i < G_N_ELEMENTS (sorted); i++)
+    {
+      g_autofree gchar *copy = g_strdup (sorted[i]);
+
+      g_test_message ("%s == %s", copy, sorted[i]);
+      g_assert_cmpint (flatpak_envp_cmp (&copy, &sorted[i]), ==, 0);
+      g_assert_cmpint (flatpak_envp_cmp (&sorted[i], &copy), ==, 0);
+
+      for (j = i + 1; j < G_N_ELEMENTS (sorted); j++)
+        {
+          g_test_message ("%s < %s", sorted[i], sorted[j]);
+          g_assert_cmpint (flatpak_envp_cmp (&sorted[i], &sorted[j]), <, 0);
+          g_assert_cmpint (flatpak_envp_cmp (&sorted[j], &sorted[i]), >, 0);
+        }
+    }
+
+  sort_this = g_new0 (const char *, G_N_ELEMENTS (unsorted));
+
+  for (i = 0; i < G_N_ELEMENTS (unsorted); i++)
+    sort_this[i] = unsorted[i];
+
+  qsort (sort_this, G_N_ELEMENTS (unsorted), sizeof (char *),
+         flatpak_envp_cmp);
+
+  for (i = 0; i < G_N_ELEMENTS (sorted); i++)
+    g_assert_cmpstr (sorted[i], ==, sort_this[i]);
+
+  g_free (sort_this);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -1585,6 +1649,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/common/dconf-app-id", test_dconf_app_id);
   g_test_add_func ("/common/dconf-paths", test_dconf_paths);
   g_test_add_func ("/common/decompose-ref", test_decompose);
+  g_test_add_func ("/common/envp-cmp", test_envp_cmp);
 
   g_test_add_func ("/app/looks-like-branch", test_looks_like_branch);
   g_test_add_func ("/app/columns", test_columns);