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
This commit is contained in:
parent
34bd426768
commit
7d85694fdf
@ -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:
|
||||
|
@ -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)
|
||||
|
@ -0,0 +1,5 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes ``File name too long`` in the image caching code when a URL contains
|
||||
a long query string.
|
Loading…
Reference in New Issue
Block a user