dir: Ignore trailing slash in remote URIs

Currently if you have a remote configured with the URL
"https://dl.flathub.org/repo/" (as you would if you use the flatpakrepo
file) and you use a flatpakref file which specifies the URL
"https://dl.flathub.org/repo", Flatpak tries to add a duplicate remote
because it doesn't see those URLs as equal. So ignore the trailing slash
when comparing remote URLs. OSTree works equally well with both kinds
(it uses g_build_filename()).

Flathub served flatpakref files with URLs missing a trailing slash until
this commit:
https://github.com/flathub/ansible-playbook/commit/b20694f09

Also, add a unit test that fails without this patch.

Fixes https://github.com/flatpak/flatpak/issues/2979

Closes: #3065
Approved by: alexlarsson

(cherry picked from commit af4504c8d3)

common/flatpak-dir.c

@@ -18,6 +18,7 @@
  * Authors:
  *       Alexander Larsson <alexl@redhat.com>
  *       Philip Withnall <withnall@endlessm.com>
+ *       Matthew Leeds <matthew.leeds@endlessm.com>
  */
 
 #include "config.h"
@@ -12405,6 +12406,34 @@ flatpak_dir_create_remote_for_ref_file (FlatpakDir *self,
   return TRUE;
 }
 
+static gboolean
+_flatpak_uri_equal (const char *uri1,
+                    const char *uri2)
+{
+  g_autofree char *uri1_norm = NULL;
+  g_autofree char *uri2_norm = NULL;
+  gsize uri1_len = strlen (uri1);
+  gsize uri2_len = strlen (uri2);
+
+  /* URIs handled by libostree are equivalent with or without a trailing slash,
+   * but this isn't otherwise guaranteed to be the case.
+   */
+  if (g_str_has_prefix (uri1, "oci+") || g_str_has_prefix (uri2, "oci+"))
+    return g_strcmp0 (uri1, uri2) == 0;
+
+  if (g_str_has_suffix (uri1, "/"))
+    uri1_norm = g_strndup (uri1, uri1_len - 1);
+  else
+    uri1_norm = g_strdup (uri1);
+
+  if (g_str_has_suffix (uri2, "/"))
+    uri2_norm = g_strndup (uri2, uri2_len - 1);
+  else
+    uri2_norm = g_strdup (uri2);
+
+  return g_strcmp0 (uri1_norm, uri2_norm) == 0;
+}
+
 /* This tries to find a pre-configured remote for the specified uri
  * and (optionally) collection id. This is a bit more complex than it
  * sounds, because a local remote could be configured in different
@@ -12419,6 +12448,8 @@ flatpak_dir_create_remote_for_ref_file (FlatpakDir *self,
  *  If the collection id is the same (and specified), its going to be
  *  the same remote, even if the url is different (because it could be
  *  some other mirror of the same repo).
+ *
+ *  We also consider non-OCI URLs equal even if one lacks a trailing slash.
  */
 char *
 flatpak_dir_find_remote_by_uri (FlatpakDir *self,
@@ -12427,6 +12458,9 @@ flatpak_dir_find_remote_by_uri (FlatpakDir *self,
 {
   g_auto(GStrv) remotes = NULL;
 
+  g_return_val_if_fail (self != NULL, NULL);
+  g_return_val_if_fail (uri != NULL, NULL);
+
   if (!flatpak_dir_ensure_repo (self, NULL, NULL))
     return NULL;
 
@@ -12455,9 +12489,9 @@ flatpak_dir_find_remote_by_uri (FlatpakDir *self,
               strcmp (collection_id, remote_collection_id) == 0)
             return g_strdup (remote);
 
-          /* Same repo if uris matches, unless both have collection-id
+          /* Same repo if uris match, unless both have collection-id
              specified but different */
-          if (strcmp (uri, remote_uri) == 0 &&
+          if (_flatpak_uri_equal (uri, remote_uri) &&
               !(collection_id != NULL &&
                 remote_collection_id != NULL &&
                 strcmp (collection_id, remote_collection_id) != 0))

tests/test-repo.sh

@@ -24,7 +24,7 @@ set -euo pipefail
 skip_without_bwrap
 skip_revokefs_without_fuse
 
-echo "1..32"
+echo "1..33"
 
 #Regular repo
 setup_repo
@@ -178,6 +178,44 @@ assert_file_has_content install-error-log "Server returned status 404: Not Found
 
 echo "ok install fails gracefully for 404 URLs"
 
+# Use a new remote so we can be sure it doesn't match any existing one's URL
+setup_repo_no_add flatpakref org.test.Collection.Flatpakref
+
+cat << EOF > repos/flatpakref/flatpakref-repo.flatpakrepo
+[Flatpak Repo]
+Version=1
+Url=http://127.0.0.1:$(cat httpd-port-main)/flatpakref/
+Title=The Title
+GPGKey=${FL_GPG_BASE64}
+EOF
+
+if [ x${USE_COLLECTIONS_IN_CLIENT-} == xyes ]; then
+    echo "DeployCollectionID=org.test.Collection.Flatpakref" >> repos/flatpakref/flatpakref-repo.flatpakrepo
+fi
+
+cat << EOF > org.test.Hello.flatpakref
+[Flatpak Ref]
+Name=org.test.Hello
+Branch=master
+Url=http://127.0.0.1:$(cat httpd-port-main)/flatpakref
+GPGKey=${FL_GPG_BASE64}
+RuntimeRepo=http://127.0.0.1:$(cat httpd-port-main)/flatpakref/flatpakref-repo.flatpakrepo
+EOF
+
+${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello
+
+# Ensure that only one remote is added even though the URL in the flatpakref
+# does not have a trailing slash and the URL in the flatpakrepo file does
+NUM_REMOTES_BEFORE=$(flatpak remotes | wc -l)
+${FLATPAK} ${U} install -y org.test.Hello.flatpakref
+NUM_REMOTES_AFTER=$(flatpak remotes | wc -l)
+
+if [ $NUM_REMOTES_AFTER -ne $((NUM_REMOTES_BEFORE + 1)) ]; then
+    assert_not_reached "install of flatpakref should only add one remote"
+fi
+
+echo "ok install flatpakref normalizes remote URL trailing slash"
+
 ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello
 
 if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform 2> install-error-log; then