Do not use any parts of image URL in temporary file names

We currently use the last component, which is:
a) a potential information exposure,
b) can hit file name length limits [1]

Use the master path file name instead, which is always a UUID.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2014630

Change-Id: I2aa1230468132c2b7a8585d691ec180947f89c1e
(cherry picked from commit 7d85694fdf)
This commit is contained in:
Dmitry Tantsur 2021-10-15 18:57:46 +02:00
parent eeb0a6d76b
commit 2e9d2b5ec4
3 changed files with 28 additions and 1 deletions

View File

@ -161,7 +161,7 @@ class ImageCache(object):
# TODO(ghe): timeout and retry for downloads
# TODO(ghe): logging when image cannot be created
tmp_dir = tempfile.mkdtemp(dir=self.master_dir)
tmp_path = os.path.join(tmp_dir, href.split('/')[-1])
tmp_path = os.path.join(tmp_dir, os.path.basename(master_path))
try:
with _concurrency_semaphore:

View File

@ -226,6 +226,28 @@ class TestImageCacheFetch(base.TestCase):
with open(self.dest_path) as fp:
self.assertEqual("TEST", fp.read())
@mock.patch.object(image_cache, '_fetch', autospec=True)
def test__download_image_large_url(self, mock_fetch):
# A long enough URL may exceed the file name limits of the file system.
# Make sure we don't use any parts of the URL anywhere.
url = "http://example.com/image.iso?secret=%s" % ("x" * 1000)
def _fake_fetch(ctx, href, tmp_path, *args):
self.assertEqual(url, href)
self.assertNotEqual(self.dest_path, tmp_path)
self.assertNotEqual(os.path.dirname(tmp_path), self.master_dir)
with open(tmp_path, 'w') as fp:
fp.write("TEST")
mock_fetch.side_effect = _fake_fetch
self.cache._download_image(url, self.master_path, self.dest_path)
self.assertTrue(os.path.isfile(self.dest_path))
self.assertTrue(os.path.isfile(self.master_path))
self.assertEqual(os.stat(self.dest_path).st_ino,
os.stat(self.master_path).st_ino)
with open(self.dest_path) as fp:
self.assertEqual("TEST", fp.read())
@mock.patch.object(image_cache, '_fetch', autospec=True)
@mock.patch.object(image_cache, 'LOG', autospec=True)
@mock.patch.object(os, 'link', autospec=True)

View File

@ -0,0 +1,5 @@
---
fixes:
- |
Fixes ``File name too long`` in the image caching code when a URL contains
a long query string.