From 866ba643d2d80cd1c8983aab47b774fc6de1920a Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 22 Jul 2019 18:31:11 -0700 Subject: [PATCH] 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 af4504c8d3a2e00b2d968644517ef8a9180afcd6) --- common/flatpak-dir.c | 38 ++++++++++++++++++++++++++++++++++++-- tests/test-repo.sh | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index 92462091..2f1c379d 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -18,6 +18,7 @@ * Authors: * Alexander Larsson * Philip Withnall + * Matthew Leeds */ #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)) diff --git a/tests/test-repo.sh b/tests/test-repo.sh index 01ca6a94..c5f54621 100644 --- a/tests/test-repo.sh +++ b/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