From 4c1f71ec9e171dcf29079ff80bab68d86d7d1856 Mon Sep 17 00:00:00 2001
From: Florian Festi <ffesti@redhat.com>
Date: Thu, 11 Apr 2024 14:58:53 +0200
Subject: [PATCH] Add error messages for url helper calls

Rpm allows URLs as cli parameters. The files are then automatically
downloaded with %_urlhelper which defaults to curl(1). So far failures
have been ignored right away and error messages are generated later when
the file was not found on disk.

Issue a meaningful error message when the help program is failing or missing
completely. This allows not to ship curl with rpm while still giving the
user a chance to find out what is going on. This is not quite ideal as
the operation continues and creates a second error message later on, but
as good as it gets without redesigning the whole code.

Related: rhbz#2216754
Resolves: #2683
---
 rpmio/url.c         | 15 ++++++++++++---
 tests/rpmgeneral.at | 27 +++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/rpmio/url.c b/rpmio/url.c
index ac29933c9..6f56fb592 100644
--- a/rpmio/url.c
+++ b/rpmio/url.c
@@ -99,6 +99,7 @@ int urlGetFile(const char * url, const char * dest)
     char *urlhelper = NULL;
     int status;
     pid_t pid;
+    int res = -1;
 
     urlhelper = rpmExpand("%{?_urlhelper}", NULL);
 
@@ -118,9 +119,17 @@ int urlGetFile(const char * url, const char * dest)
         execvp(argv[0], argv);
         exit(127); /* exit with 127 for compatibility with bash(1) */
     }
+
+    if ((waitpid(pid, &status, 0) != -1) && WIFEXITED(status)) {
+	if (WEXITSTATUS(status) == 127)
+	    rpmlog(RPMLOG_ERR, _("Could not find url helper: \"%s\"\n"), urlhelper);
+	else if (WEXITSTATUS(status) == 0)
+	    res = 0;
+	else
+	    rpmlog(RPMLOG_ERR, _("Executing url helper \"%s\" failed with status %i\n"), cmd, WEXITSTATUS(status));
+    }
+
     free(cmd);
     free(urlhelper);
-
-    return ((waitpid(pid, &status, 0) != -1) &&
-	    WIFEXITED(status) && (WEXITSTATUS(status) == 0)) ? 0 : -1;
+    return res;
 }
diff --git a/tests/rpmgeneral.at b/tests/rpmgeneral.at
index 0ab5ed6d4..d1b71084e 100644
--- a/tests/rpmgeneral.at
+++ b/tests/rpmgeneral.at
@@ -383,3 +383,30 @@ runroot_other rpm2archive "${RPMTEST}"/data/SRPMS/hello-1.0-1.src.rpm | tar tzf
 ])
 
 RPMTEST_CLEANUP
+
+AT_SETUP([urlhelper missing])
+AT_KEYWORDS([urlhelper])
+RPMDB_INIT
+RPMTEST_CHECK([
+# runroot rpm --define "_urlhelper /not/there" --root /srv/test -qp https://example.com/foo-0.1-1.noarch.rpm
+runroot rpm --define "_urlhelper /not/there" -qp https://www.example.com/foo-1.0-1.x86_64.rpm
+],
+[1],
+[],
+[error: Could not find url helper: "/not/there"
+error: open of https://www.example.com/foo-1.0-1.x86_64.rpm failed: No such file or directory
+])
+RPMTEST_CLEANUP
+
+AT_SETUP([urlhelper fails])
+AT_KEYWORDS([urlhelper])
+RPMDB_INIT
+RPMTEST_CHECK([
+runroot rpm --define "_urlhelper /bin/false" --root /srv/test -qp https://example.com/foo-0.1-1.noarch.rpm 2> >(sed 's|rpm-tmp.* https|rpm-tmp https|' >&2)
+],
+[1],
+[],
+[error: Executing url helper "/bin/false /usr/local/var/tmp/rpm-tmp https://example.com/foo-0.1-1.noarch.rpm" failed with status 1
+error: open of https://example.com/foo-0.1-1.noarch.rpm failed: No such file or directory
+])
+RPMTEST_CLEANUP