From a4863c0f2656389c973c3add8c1c6de84d7819f0 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mon, 18 May 2020 14:58:52 +0200
Subject: [PATCH] lib: check consistency of rpm from openRemoteFile

Fixes: https://pagure.io/koji/issue/2130
---
 builder/kojid                |  1 -
 koji/__init__.py             | 14 +++++++++++---
 tests/test_lib/test_utils.py | 22 ++++++++++++++++++++--
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/builder/kojid b/builder/kojid
index d450e05a..acd0c99a 100755
--- a/builder/kojid
+++ b/builder/kojid
@@ -1148,7 +1148,6 @@ class BuildTask(BaseTaskHandler):
         opts = dict([(k, getattr(self.options, k)) for k in ('topurl', 'topdir')])
         opts['tempdir'] = self.workdir
         with koji.openRemoteFile(relpath, **opts) as fo:
-            koji.check_rpm_file(fo)
             h = koji.get_rpm_header(fo)
         if not koji.get_header_field(h, 'sourcepackage'):
             raise koji.BuildError("%s is not a source package" % srpm)
diff --git a/koji/__init__.py b/koji/__init__.py
index bf39ea94..6f247f53 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -1751,17 +1751,18 @@ def format_exc_plus():
 
 
 def request_with_retry(retries=3, backoff_factor=0.3,
-                        status_forcelist=(500, 502, 504, 408, 429), session=None):
+                       status_forcelist=(500, 502, 504, 408, 429), session=None):
     # stolen from https://www.peterbe.com/plog/best-practice-with-retries-with-requests
     session = session or requests.Session()
     retry = Retry(total=retries, read=retries, connect=retries,
-                backoff_factor=backoff_factor,
-                status_forcelist=status_forcelist)
+                  backoff_factor=backoff_factor,
+                  status_forcelist=status_forcelist)
     adapter = HTTPAdapter(max_retries=retry)
     session.mount('http://', adapter)
     session.mount('https://', adapter)
     return session
 
+
 def openRemoteFile(relpath, topurl=None, topdir=None, tempdir=None):
     """Open a file on the main server (read-only)
 
@@ -1780,6 +1781,13 @@ def openRemoteFile(relpath, topurl=None, topdir=None, tempdir=None):
             raise GenericError("Downloaded file %s doesn't match expected size (%s vs %s)" %
                                (url, fo.tell(), resp.headers['Content-Length']))
         fo.seek(0)
+        if relpath.endswith('.rpm'):
+            # if it is an rpm run basic checks (assume that anything ending with the suffix,
+            # but not being rpm is suspicious anyway)
+            try:
+                check_rpm_file(fo)
+            except Exception as ex:
+                raise GenericError("Downloaded rpm %s is corrupted:\n%s" % (url, str(ex)))
     elif topdir:
         fn = "%s/%s" % (topdir, relpath)
         fo = open(fn)
diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py
index 7ec1b22b..c252ee0d 100644
--- a/tests/test_lib/test_utils.py
+++ b/tests/test_lib/test_utils.py
@@ -170,7 +170,6 @@ class MiscFunctionTestCase(unittest.TestCase):
             m_requests.register_uri('GET', url, text=text,
                     headers = {'Content-Length': "3"})
             m_TemporaryFile.return_value.tell.return_value = len(text)
-            # using neither
             with self.assertRaises(koji.GenericError):
                 koji.openRemoteFile(path, topurl=topurl)
             m_TemporaryFile.assert_called_once()
@@ -186,13 +185,32 @@ class MiscFunctionTestCase(unittest.TestCase):
             m_requests.register_uri('GET', url, text=text,
                     headers = {'Content-Length': "100"})
             m_TemporaryFile.return_value.tell.return_value = len(text)
-            # using neither
             with self.assertRaises(koji.GenericError):
                 koji.openRemoteFile(path, topurl=topurl)
             m_TemporaryFile.assert_called_once()
             m_TemporaryFile.return_value.tell.assert_called()
             m_open.assert_not_called()
 
+    def test_openRemoteFile_valid_rpm(self):
+        # downloaded file is correct rpm
+        with requests_mock.Mocker() as m_requests:
+            topurl = 'http://example.com/koji'
+            path = 'tests/test_lib/data/rpms/test-src-1-1.fc24.src.rpm'
+            url = os.path.join(topurl, path)
+            m_requests.register_uri('GET', url, body=open(path, 'rb'))
+            #with self.assertRaises(koji.GenericError):
+            koji.openRemoteFile(path, topurl=topurl)
+
+    def test_openRemoteFile_invalid_rpm(self):
+        # downloaded file is correct rpm
+        with requests_mock.Mocker() as m_requests:
+            topurl = 'http://example.com/koji'
+            path = 'file.rpm'
+            url = os.path.join(topurl, path)
+            m_requests.register_uri('GET', url, text='123')
+            with self.assertRaises(koji.GenericError):
+                koji.openRemoteFile(path, topurl=topurl)
+
     def test_joinpath_bad(self):
         bad_joins = [
             ['/foo', '../bar'],